Skip to content

Kubernetes: How to populate resource attributes based on attributes, labels and transformation#1756

Merged
lmolkova merged 22 commits intoopen-telemetry:mainfrom
zeitlinger:k8s-label-recommendations
Feb 27, 2025
Merged

Kubernetes: How to populate resource attributes based on attributes, labels and transformation#1756
lmolkova merged 22 commits intoopen-telemetry:mainfrom
zeitlinger:k8s-label-recommendations

Conversation

@zeitlinger
Copy link
Copy Markdown
Member

@zeitlinger zeitlinger commented Jan 17, 2025

Fixes #236

This is a new take on the issue. #349 has been created before and not completed.

So what's the difference now?

  1. The proposed rules are already implemented in the OTel operator and work well: https://github.com/open-telemetry/opentelemetry-operator#configure-resource-attributes - the PR (at least in it's initial form) makes the operator implementation fully compatible with this spec PR.
  2. The specification work on service.instance.id has been completed - which was effectively a blocker before.

Merge requirement checklist

Copy link
Copy Markdown
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

Love the overall direction. This makes a lot of sense

Comment thread docs/resource/k8s.md Outdated
Comment thread docs/resource/k8s.md Outdated
Comment thread docs/resource/k8s.md Outdated
@ChrsMark ChrsMark requested a review from a team January 20, 2025 10:59
Comment thread docs/resource/k8s.md Outdated
@dmitryax
Copy link
Copy Markdown
Member

We briefly discussed this during the k8s SemConv WG meeting. There is no consensus on the document yet, but we decided to recommend moving it from the k8s resource section of the semantic conventions repo to service because it prescribes a way of populating values of service attributes, not attributes in k8s namespace.

Comment thread docs/resource/k8s.md Outdated
Comment thread docs/resource/k8s.md Outdated
Comment thread docs/resource/k8s.md Outdated
@zeitlinger
Copy link
Copy Markdown
Member Author

We briefly discussed this during the k8s SemConv WG meeting. There is no consensus on the document yet, but we decided to recommend moving it from the k8s resource section of the semantic conventions repo to service because it prescribes a way of populating values of service attributes, not attributes in k8s namespace.

@open-telemetry/specs-semconv-approvers the service page is automatically generated - how can I move the content of this PR to the service page?

@github-actions github-actions Bot added the enhancement New feature or request label Jan 28, 2025
@trask
Copy link
Copy Markdown
Member

trask commented Jan 28, 2025

@open-telemetry/specs-semconv-approvers the service page is automatically generated - how can I move the content of this PR to the service page?

maybe as subsection(s) under https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/README.md#service

Comment thread docs/resource/README.md Outdated
@zeitlinger zeitlinger force-pushed the k8s-label-recommendations branch from 604cdc9 to d850c85 Compare February 3, 2025 13:53
@zeitlinger
Copy link
Copy Markdown
Member Author

@jsuereth @tigrannajaryan thanks for the positive feedback in the entities SIG call. A added the notes about the object hierarchy for service.name and value stability for service.instance.id. Please let me know if this addresses everything.

@zeitlinger
Copy link
Copy Markdown
Member Author

@jinja2 thanks for the detailed review - everything should be addressed now

@zeitlinger
Copy link
Copy Markdown
Member Author

@open-telemetry/specs-semconv-approvers please take a look

Approval from the k8s semconv and entities SIG has been granted now

Comment thread docs/resource/k8s.md Outdated
Comment thread docs/resource/README.md Outdated
Comment thread docs/resource/README.md Outdated
Comment thread docs/resource/README.md Outdated
@zeitlinger zeitlinger force-pushed the k8s-label-recommendations branch from 2a21f8c to a5ba77b Compare February 12, 2025 10:04
@joaopgrassi
Copy link
Copy Markdown
Member

@open-telemetry/specs-semconv-maintainers this seems like a good PR to include in https://changelog.opentelemetry.io. Should we add the label to it? 🤔

@trask
Copy link
Copy Markdown
Member

trask commented Feb 12, 2025

@open-telemetry/specs-semconv-maintainers this seems like a good PR to include in https://changelog.opentelemetry.io. Should we add the label to it? 🤔

sure! I don't think it strictly falls under "New semantic convention areas or stability changes, as well as releases", but I think it's always ok for @open-telemetry/specs-semconv-maintainers to call out particularly interesting or broadly relevant changes on https://changelog.opentelemetry.io

cc @austinlparker

@zeitlinger
Copy link
Copy Markdown
Member Author

@open-telemetry/specs-semconv-maintainers this seems like a good PR to include in https://changelog.opentelemetry.io. Should we add the label to it? 🤔

now that the content is moved to non-normative, it should automatically get published under https://opentelemetry.io/docs/specs/semconv/non-normative/ - IIUC

@trask
Copy link
Copy Markdown
Member

trask commented Feb 20, 2025

@zeitlinger can you change this line in CODEOWNERS to cover this new file also (I think /docs/non-normative/k8s-*)?

/docs/non-normative/k8s-migration.md @open-telemetry/specs-semconv-approvers @open-telemetry/semconv-k8s-approvers

@open-telemetry/semconv-k8s-approvers does anyone else want to review before we merge? thanks!

@zeitlinger
Copy link
Copy Markdown
Member Author

@zeitlinger can you change this line in CODEOWNERS to cover this new file also (I think /docs/non-normative/k8s-*)?

thanks - added

@lmolkova
Copy link
Copy Markdown
Member

@zeitlinger could you please resolve discussions (that are resolved)? Thanks!

@zeitlinger
Copy link
Copy Markdown
Member Author

@zeitlinger could you please resolve discussions (that are resolved)? Thanks!

@lmolkova all are resolved now 😄

Comment thread .chloggen/k8s-label-recommendations.yaml Outdated
Comment thread .chloggen/k8s-label-recommendations.yaml Outdated
@trask
Copy link
Copy Markdown
Member

trask commented Feb 26, 2025

need #1939 to be merged first to fix CI

@lmolkova lmolkova merged commit 647de87 into open-telemetry:main Feb 27, 2025
@github-project-automation github-project-automation Bot moved this from In Review to Done in K8s SemConv SIG Feb 27, 2025
@zeitlinger zeitlinger deleted the k8s-label-recommendations branch February 27, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done
Archived in project

Development

Successfully merging this pull request may close these issues.

Proposal: Define mapping from k8s well-known labels to semconv