-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "fix(otelcol): remove the v2 metrics exporter configuration (#… …7105)" #7118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 2a5c9de in 2 minutes and 3 seconds
More details
- Looked at
160
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
12
drafted comments based on config settings.
1. deploy/docker-swarm/docker-compose.ha.yaml:227
-
Draft comment:
Updated otel-collector image version from 0.111.27 to 0.111.28. Confirm that the version bump doesn’t require further config adjustments. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. deploy/docker-swarm/docker-compose.ha.yaml:250
-
Draft comment:
Updated schema-migrator image version from 0.111.24 to 0.111.28. Ensure consistency with other deployment files. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. deploy/docker-swarm/otel-collector-config.yaml:90
-
Draft comment:
Added 'signozclickhousemetrics' exporter to both metrics and metrics/prometheus pipelines. Verify this is the intended configuration to revert the previous change. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. deploy/docker/docker-compose.ha.yaml:237
-
Draft comment:
Updated otel-collector image version from ${OTELCOL_TAG:-0.111.27} to ${OTELCOL_TAG:-0.111.28}. Confirm consistency with related services. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
5. deploy/docker/docker-compose.ha.yaml:271
-
Draft comment:
Updated schema-migrator image version from ${OTELCOL_TAG:-0.111.24} to ${OTELCOL_TAG:-0.111.28}. Verify that downstream dependencies align with this change. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
6. deploy/docker/docker-compose.yaml:166
-
Draft comment:
Updated otel-collector image version to ${OTELCOL_TAG:-0.111.28}. Ensure that other service dependencies remain compatible. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
7. deploy/docker/otel-collector-config.yaml:69
-
Draft comment:
Added 'signozclickhousemetrics' exporter with DSN configuration. Confirm that its inclusion in pipelines is consistent with system expectations. -
Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
8. deploy/docker-swarm/docker-compose.ha.yaml:227
-
Draft comment:
Updated otel-collector image tag to 0.111.28 (and similarly schema-migrator version updated) for consistency across docker-swarm. Ensure all dependent services are compatible with this version. -
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is about updating a dependency version and asks to ensure compatibility with dependent services. According to the rules, comments on dependency changes and asking to ensure compatibility are not allowed.
9. deploy/docker-swarm/otel-collector-config.yaml:67
-
Draft comment:
Reintroduced the signozclickhousemetrics exporter and appended it to the metrics pipelines. Confirm this duplicate exporter (with clickhousemetricswrite) is intentional and does not cause duplicate metric data. -
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The comment asks for confirmation about intention, which violates our rules about not asking authors to confirm things. However, it does point out a potential technical issue with duplicate metric data being exported. But without understanding the full system architecture and the purpose of each exporter, we can't be certain this is actually a problem.
The duplicate exporters might serve different purposes or handle different metric formats that we can't see from this config alone. The comment might be highlighting a real issue.
While there could be a real issue, the comment violates our rules by asking for confirmation rather than making a clear statement about what needs to be changed.
Delete the comment because it asks for confirmation of intention rather than making a clear, actionable statement about what needs to be changed.
10. deploy/docker/docker-compose.ha.yaml:237
-
Draft comment:
Updated otel-collector and schema-migrator image tags to 0.111.28 in the docker configuration. Verify that this update is consistent with the swarm setup. -
Reason this comment was not posted:
Marked as duplicate.
11. deploy/docker/docker-compose.yaml:166
-
Draft comment:
Updated otel-collector and schema-migrator image tags to 0.111.28 in the docker-compose file. Confirm that volume mounts and dependencies remain correct. -
Reason this comment was not posted:
Marked as duplicate.
12. deploy/docker/otel-collector-config.yaml:69
-
Draft comment:
Re-added the signozclickhousemetrics exporter and included it in both the 'metrics' and 'metrics/prometheus' pipelines. Double-check that merging two exporters for ClickHouse metrics is the desired configuration. -
Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_hPsoWAM5jtYcxO2y
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
prashant-shahi
approved these changes
Feb 15, 2025
grandwizard28
approved these changes
Feb 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This reverts commit 846b6a9 and updates the collector version.
Important
Reverts a previous commit, updates OpenTelemetry Collector and schema migrator versions, and adds a new metrics exporter configuration.
846b6a9
which removed the v2 metrics exporter configuration.otel-collector
image tosignoz/signoz-otel-collector:0.111.28
indocker-compose.ha.yaml
anddocker-compose.yaml
.schema-migrator
image tosignoz/signoz-schema-migrator:0.111.28
indocker-compose.ha.yaml
anddocker-compose.yaml
.signozclickhousemetrics
exporter inotel-collector-config.yaml
.signozclickhousemetrics
inotel-collector-config.yaml
.This description was created by
for 2a5c9de. It will automatically update as commits are pushed.