feat(helm v5): Make k8sattributes processor as default and deprecate k8s_tagger for k8s metadata enrichment#4093
feat(helm v5): Make k8sattributes processor as default and deprecate k8s_tagger for k8s metadata enrichment#4093
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
jagan2221
changed the title
jagan2221
changed the title
…logic-kubernetes-collection into helmv5/k8s_tagger
| @@ -0,0 +1 @@ | |||
| feat(helm v5): Make k8sattributes processor as default and deprecate k8s_tagger for k8s metadata enrichment | |||
There was a problem hiding this comment.
nit: if this change is being pushed before helm v5, then I would suggest we remove v5 references from here.
There was a problem hiding this comment.
This will be pushed in v5 only. We'll keep this reviewed and ready and then merge all helm v5 PR's at once.
| traces: | ||
| receivers: [jaeger, otlp, otlp/deprecated, zipkin] | ||
| processors: [memory_limiter, k8s_tagger, source, resource, batch] | ||
| processors: [memory_limiter, {{ if .Values.otelcolInstrumentation.useSumoK8sProcessor }}k8s_tagger{{ else }}k8sattributes{{ end }}, source, resource, batch] |
There was a problem hiding this comment.
Will it be a valid config if useSumoK8sProcessor is not enabled. Just wanted to make sure that we don't end up with "memory_limiter,,source" and end up erroring out at the collector start.
There was a problem hiding this comment.
yes, if useSumoK8sProcessor is disabled by default in our values, so k8sattributes will be chosen. There will always be useSumoK8sProcessor either true/false.
There was a problem hiding this comment.
Pull request overview
This PR makes k8sattributes (the upstream OpenTelemetry k8s metadata enrichment processor) the default instead of the deprecated SumoLogic-specific k8s_tagger processor for all three telemetry pipelines: logs, metrics, and traces. The k8s_tagger can still be opted into via the *.useSumoK8sProcessor: true flag and is planned for complete removal in helm chart v6.
Changes:
- Set
useSumoK8sProcessor: falseas the new default formetadata.logs,metadata.metrics, andotelcolInstrumentation, adding the newk8sattributesprocessor configuration block for each pipeline - Added two new flags (
extractPodLabels,extractNodeLabels) undermetadata.metricsto control metrics cardinality when usingk8sattributes - Updated all Helm golden file tests and integration tests to reflect the new default processor and the new labels it produces (notably
service.namespace)
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
deploy/helm/sumologic/values.yaml |
Changes defaults of useSumoK8sProcessor to false and adds extractPodLabels/extractNodeLabels flags; contains stale/incorrect comments |
deploy/helm/sumologic/conf/instrumentation/otelcol.instrumentation.conf.yaml |
Adds k8sattributes processor configuration for traces/metrics pipelines when useSumoK8sProcessor is false |
deploy/helm/sumologic/conf/metrics/otelcol/processors.yaml |
Adds k8sattributes configuration for metrics pipeline with conditional pod/node label extraction |
deploy/helm/sumologic/conf/logs/otelcol/config.yaml |
Adds secondary pod_association rule (namespace+name compound key) for the logs k8sattributes config |
deploy/helm/sumologic/README.md |
Adds documentation for new useSumoK8sProcessor, waitForMetadata*, extractPodLabels, and extractNodeLabels fields |
tests/helm/testdata/goldenfile/metadata_*_otc/*.output.yaml |
Updates golden files to reflect k8sattributes as new default processor |
tests/helm/testdata/goldenfile/otelcol-instrumentation-config/*.output.yaml |
Updates golden files for instrumentation config to use k8sattributes |
tests/helm/metrics_test.go |
Updates pipeline processor name assertion from k8s_tagger to k8sattributes |
tests/integration/features.go |
Adds service.namespace to expected metric labels; removes node labels (now disabled by default) |
tests/integration/internal/constants.go |
Moves node-related OTel metrics from dedicated list to FlakyMetrics |
tests/integration/helm_k8s_processor_test.go |
Removes NodeLabelsMetrics from expected metrics (now disabled by default) |
tests/integration/values/values_helm_k8s_processor.yaml |
Adds explicit extractPodLabels/extractNodeLabels flags (now redundant since they're defaults) |
tests/helm/testdata/goldenfile/metadata_logs_otc/k8sattributes.output.yaml |
Adds secondary pod_association rule |
tests/helm/testdata/goldenfile/metrics-server/basic.output.yaml |
Unrelated reordering of resource and dnsPolicy fields |
.changelog/4093.changed.txt |
Adds changelog entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
LGTM, Check co-pilot comments regarding empty labels scenario and add the test case if needed. |
What changed?
Made k8sattributes processor as default and deprecate k8s_tagger for logs,metrics and traces pipelines. It can be enabled again via *.useSumok8sprocessor flag
We will remove k8s_tagger completely in v6 helm chart.
Changes to note:
pod labels and node labels are the fields which increases high cardinality.
k8s_tagger was already adding pod labels, but nodelabels are newly added in k8sattributes processor and we don't want to add nodelabels by default.
So, kept the flag values as for k8sattributes,
extractPodLabels = true (Retain podlabels as like k8s_tagger)
extractNodeLabels = false (Disable nodelabels since k8s_tagger didn't added that earlier)
Comparison of enrichment done by k8s_tagger vs k8s_attributes processor. Nodelabels included in this for test, but in our change has nodelabels disabled.