Skip to content

[opentelemetry-kube-stack] Use chart's top-level resourceAttributes for k8s.cluster.name and deployment.environment.name - #361

Open
kangyili wants to merge 1 commit into
mainfrom
kangyi/resourceAttributes
Open

[opentelemetry-kube-stack] Use chart's top-level resourceAttributes for k8s.cluster.name and deployment.environment.name#361
kangyili wants to merge 1 commit into
mainfrom
kangyi/resourceAttributes

Conversation

@kangyili

Copy link
Copy Markdown
Contributor

What does this PR do?

Upstream chart PR open-telemetry/opentelemetry-helm-charts#2393 adds a top-level resourceAttributes map that renders as a resource/global upsert processor on every collector pipeline and is merged into the Instrumentation CR's resource.resourceAttributes.

This lets us delete our homegrown workaround for contrib#49970 and unify how k8s.cluster.name and deployment.environment.name get set.

@kangyili
kangyili requested a review from a team as a code owner September 11, 2026 09:31
@kangyili
kangyili force-pushed the kangyi/resourceAttributes branch from d967501 to 67db02f Compare September 11, 2026 09:34

@cyrille-leclerc cyrille-leclerc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully tested

name: k8s.pod.uid
resource/global:
attributes:
- action: upsert

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be upsert, overwriting? or just inserting if not defined? Probably a question on the upstream feature rather than on our usage of it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m okay with upsert
For supported cloud providers, the cluster name is auto-detected, and the installation script doesn’t add k8s.cluster.name to the resourceAttributes.

If a user explicitly sets the cluster name in resourceAttributes, we can assume they want to override the value, so upsert makes sense.

@mx-psi mx-psi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, could we avoid the newline changes in all rendered examples?

@kangyili

Copy link
Copy Markdown
Contributor Author

Changes look good, could we avoid the newline changes in all rendered examples?

@mx-psi Somehow the file formatting in the main branch is broken. I think the new line change is generated by make generate-examples. I didn’t modify the Makefile except for bumping the Helm chart version.
Feel free to test it on your side too, just in case it’s an issue with my local setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants