Add recommendation for well-known k8s labels #349
Conversation
| * The Well-Known Label `app.kubernetes.io/name` SHOULD align to the OpenTelemetry resource attribute `service.name`. | ||
| * The Well-Known Label `app.kubernetes.io/version` SHOULD align to the OpenTelemetry resource attribute `service.version`. |
There was a problem hiding this comment.
In #236 you link to a potential collector config that would implement this but if we already standardize a mapping here, I think the k8sattributesprocessor in the collector should be adapted to provide this mapping out of the box (potentially with a simple opt-in flag).
Did you discuss this addition with the collector folks owning the processor? I think it would make sense to align this with them.
There was a problem hiding this comment.
Seems like a nice thing to offer to get this more easily adopted, opened an issue @ open-telemetry/opentelemetry-collector-contrib#27261
There was a problem hiding this comment.
For now, think this also makes sense in the default helm charts. Issue tracked here: open-telemetry/opentelemetry-helm-charts#905
| * The Well-Known Label `app.kubernetes.io/name` SHOULD align to the OpenTelemetry resource attribute `service.name`. | ||
| * The Well-Known Label `app.kubernetes.io/version` SHOULD align to the OpenTelemetry resource attribute `service.version`. |
There was a problem hiding this comment.
This is not a technical nit, but more about organization of semantic conventions in general.
In this PR we K8s resource conventions requiring a certain value for service.name, in #312 we have an attempt to extend the the service.instance.id definition with K8s specific data.
It would be great to have a consistent way to specify such mappings. I'm slightly in favor of the approach this PR takes (defining values for general attributes in the domain specific conventions).
There was a problem hiding this comment.
Elaborating a bit on this point, after chatting with @jpkrohling:
Given that both #312 and this PR are merged: if I'd want to instrument a K8s service, the conventions would be spread out across places:
- setting
service.nameis specified inresource/k8s.md - setting
service.versionis specified inresource/k8s.md - setting
service.instance.idis specified in an algorithm inresource/README.md
I think we should think about consistency before merging any of those PRs. Like:
- Should we consistently define algorithms for
service.*attributes, like proposed in Define a common algorithm for service.instance.id #312 forservice.instance.id? - Should we have consistently have area-specific overrides for
service.*attributes, like what's proposed in this PR forservice.name? - Or are we fine with having a mix?
We needn't come to a final conclusion before merging anything experimental, but we'd need to before declaring anything stable.
|
|
||
| There are certain Kubernetes [Well-Known Labels and Annotations](https://kubernetes.io/docs/reference/labels-annotations-taints/) that SHOULD align to OpenTelemetry resource attributes. Labels are typically converted to existing OpenTelemetry attributes automatically using an OpenTelemetry Collector component like the [Kubernetes Attribute Processor](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/k8sattributesprocessor). | ||
|
|
||
| * The Well-Known Label `app.kubernetes.io/name` SHOULD align to the OpenTelemetry resource attribute `service.name`. |
There was a problem hiding this comment.
This label is usually used to express the generic application name, whereas app.kubernetes.io/instance is the recommended label key to indicate a specific installation of an application in k8s. The service.name attr is defined as the logical name of a service. I assume this is a unique name differentiating a service installation from another; could actually do with more clarification on this.
Let's say a k8s environment has multiple installations of the application kafka, named kafka-a, kafka-b and kafka-c deployed using a kafka helm chart (standard label mapping for helm follows the same recommendation). In this case, all pods from the 3 different statefulset are likely to have app.kubernetes.io/name = kafka and the value of label app.kubernetes.io/instance will be the specific installation/usecase of the app kafka. Depending on internal conventions, users might want service.name to reflect the specific instance name instead of just kafka in this case. All this to say, there is some ambiguity here about which label maps to service.name and imo the convention should just suggest the possible well-known labels k8s users can consider.
There was a problem hiding this comment.
From a language perspective: how about replacing SHOULD with RECOMMENDED + add the note around ambiguity around app.kubernetes.io/instance?
|
|
||
| There are certain Kubernetes [Well-Known Labels and Annotations](https://kubernetes.io/docs/reference/labels-annotations-taints/) that SHOULD align to OpenTelemetry resource attributes. Labels are typically converted to existing OpenTelemetry attributes automatically using an OpenTelemetry Collector component like the [Kubernetes Attribute Processor](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/k8sattributesprocessor). | ||
|
|
||
| * The Well-Known Label `app.kubernetes.io/name` SHOULD align to the OpenTelemetry resource attribute `service.name`. |
There was a problem hiding this comment.
There's a convention about service.name which is declared stable:
MUST be the same for all instances of horizontally scaled services. If the value was not specified, SDKs MUST fallback to
unknown_service:concatenated withprocess.executable.name, e.g.unknown_service:bash. Ifprocess.executable.nameis not available, the value MUST be set tounknown_service.
I'm not sure whether we can "alter" or "amend" a stable attribute definition like proposed here.
There was a problem hiding this comment.
Perhaps we can say this is allowed as it doesn't modify or amend the convention itself, but is related to something something external (k8s labels) and is an implementation (in k8s) recommendation?
Alternately, could this be organized differently or put somewhere else?
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Changes
Adds recommendations for mapping Kubernetes well-known labels to existing OpenTelemetry resource attributes.
See #236 for original issue.
Merge requirement checklist