feat(routing): Migrate routing processor to connector for metrics collection #4025
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
dhruv-shah-sumo
changed the title
dhruv-shah-sumo
force-pushed
the
dhruv/metric-routingc-connector
branch
from
23298c4 to
c88ce92
Compare
dhruv-shah-sumo
changed the title
dhruv-shah-sumo
force-pushed
the
dhruv/metric-routingc-connector
branch
4 times, most recently
from
dca7728 to
cbaa141
Compare
There was a problem hiding this comment.
@dhruv-shah-sumo its a good idea for this to be its own PR - lets add a changelog for this. I think this migration should be called out as a feature instead of chore, because it makes non-trivial changes to the otel configuration. This is also currently a breaking change, but it doesn't need to be.
| - metrics/sumologic/default | ||
| table: | ||
| - pipelines: | ||
| - metrics/apiserver |
There was a problem hiding this comment.
Could we add an additional exporter in the input table and verify that it shows up in the table here?
There was a problem hiding this comment.
could also put the output in the description ^^ or a jira, so in the future, someone has more visual context
There was a problem hiding this comment.
Could we add an additional exporter in the input table and verify that it shows up in the table here?
metrics routing does not expose options to add custom exporters but logs do. To increase the testing, I have enabled debug exporter so that you can see the metrics/debug pipeline being created & added to the expected output routing.
There was a problem hiding this comment.
could also put the output in the description ^^ or a jira, so in the future, someone has more visual context
@chan-tim-sumo Do you mean the after/before config changes for this migration?
There was a problem hiding this comment.
after config changes (i assume before and after SHOULD be the same anyways)
There was a problem hiding this comment.
@chan-tim-sumo No, they will be different. I'll put both.
| ## ## see routing processor documentation for more details: | ||
| ## ## https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/aee4b75100530bce7edbf736fbcf76ac4f6ced6d/processor/routingprocessor/README.md#tech-preview-opentelemetry-transformation-language-statements-as-routing-conditions | ||
| ## ## exporters is an array of the exporter | ||
| ## exporters: |
There was a problem hiding this comment.
this is a breaking change and is not necessary see - https://github.com/SumoLogic/sumologic-kubernetes-collection/pull/4012/files#r2481804958
rnishtala-sumo
changed the title
dhruv-shah-sumo
force-pushed
the
dhruv/metric-routingc-connector
branch
3 times, most recently
from
35485f2 to
a329478
Compare
dhruv-shah-sumo
force-pushed
the
dhruv/metric-routingc-connector
branch
4 times, most recently
from
bad9697 to
90e9288
Compare
…cs collection Usage of routing connector is feature flagged and will be kept off/false by default. After sufficient notices and release notes, the feature flag to use routing connector in metrics collection will be turned on. Routing processor is currently removed from the otelcol-contrib version upstream, but to handle the migration better, sumo is still keeping routing processor in its version of otel col. Signed-off-by: Dhruv Shah <dhruv.shah@sumologic.com>
dhruv-shah-sumo
force-pushed
the
dhruv/metric-routingc-connector
branch
from
90e9288 to
83f3b0a
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the OpenTelemetry Collector routing configuration from using a routing processor to a routing connector for metrics collection, necessitated by version 0.136.* of the OTel collector removing support for the routing processor. The migration is controlled by a new useRoutingConnectors flag that defaults to false for backward compatibility.
Key Changes:
- Introduces
sumologic.metrics.useRoutingConnectorsconfiguration flag to enable the new routing connector approach - Adds routing connector configuration that replaces the processor-based routing when enabled
- Updates pipeline definitions to use connectors instead of processors when the flag is enabled
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
deploy/helm/sumologic/values.yaml |
Adds useRoutingConnectors flag and updates documentation comments from processor to connector terminology |
deploy/helm/sumologic/conf/metrics/otelcol/processors.yaml |
Conditionally disables routing processor when useRoutingConnectors is enabled |
deploy/helm/sumologic/conf/metrics/otelcol/pipeline.yaml |
Updates exporter configuration to use routing connector when flag is enabled |
deploy/helm/sumologic/conf/metrics/otelcol/connectors.yaml |
New file defining routing connector configuration with job-based routing rules |
deploy/helm/sumologic/conf/metrics/otelcol/config.yaml |
Adds connector section and defines individual metric pipelines when routing connectors are enabled |
deploy/helm/sumologic/README.md |
Documents the new useRoutingConnectors configuration parameter |
tests/helm/metrics_test.go |
Adds test coverage for routing connector behavior and validates both connector and processor modes |
tests/helm/testdata/goldenfile/metadata_metrics_otc/*.yaml |
Golden test files validating the generated configurations |
ci/check_configuration_keys.py |
Adds new configuration key to allowlist |
.changelog/4025.added.txt |
Changelog entry for the migration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## routing connector expects you to define a pipeline, but here the user can just mention the exporter. | ||
| ## The helm deployment would convert exporters into correct pipelines and configure the connectors. |
There was a problem hiding this comment.
The comment on line 471 refers to 'exporters' but the example above it shows a 'pipelines' array (line 467). This is confusing since the comment suggests users specify exporters but the example shows pipelines. The comment should clarify the relationship between what users specify and what gets generated, or the example should be updated to match the comment.
| @@ -0,0 +1 @@ | |||
| chore(deps): Migrate routing processor to routing connector for metrics collection No newline at end of file | |||
There was a problem hiding this comment.
The changelog entry uses 'chore(deps)' prefix but this is a feature addition ('feat'), not a dependency update. Based on the PR title 'feat(routing): Migrate routing processor to connector for metrics collection', the changelog should use 'feat(routing)' instead.
| chore(deps): Migrate routing processor to routing connector for metrics collection | |
| feat(routing): Migrate routing processor to routing connector for metrics collection |
There was a problem hiding this comment.
@dhruv-shah-sumo Please make the changelog text same as PR title
|
Looks good! |
0.136.* version of otel collector does not support routing processor, the routing processor is being moved to routing connector. This PR changes the routing config for metrics.
OSC-1217
Checklist