Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove label mapping for service.instance.id #3497

Merged
merged 14 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/service-instanc-id-mapping.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: auto-instrumentation

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`

# One or more tracking issues related to the change
issues: [3495]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
Technically, this is a breaking change, but we regard it as a bug fix because the previous behavior was incorrect.

if you did have multiple container instrumentation and use `app.kubernetes.io/instance` to set the `service.instance.id`,
you will now see multiple instances in the UI - which is the correct behavior.

You can still use the attribute `resource.opentelemetry.io/service.instance.id` to set the `service.instance.id`,
which will be shared across all containers in the pod - but this is not recommended for multiple container instrumentation instances.

Refer to the [semantic conventions](https://opentelemetry.io/docs/specs/semconv/resource/#service-experimental)
for more information.

35 changes: 32 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -725,14 +725,16 @@ EOF

### Configure resource attributes with annotations

This example shows a pod configuration with OpenTelemetry annotations using the `resource.opentelemetry.io/` prefix. These annotations can be used to add resource attributes to data produced by OpenTelemetry instrumentation.
This example shows a pod configuration with OpenTelemetry annotations using the `resource.opentelemetry.io/` prefix.
These annotations can be used to add resource attributes to data produced by OpenTelemetry instrumentation.

```yaml
apiVersion: v1
kind: Pod
metadata:
name: example-pod
annotations:
# this is just an example, you can create any resource attributes you need
resource.opentelemetry.io/service.name: "my-service"
resource.opentelemetry.io/service.version: "1.0.0"
resource.opentelemetry.io/environment: "production"
Expand All @@ -750,7 +752,6 @@ The following labels are supported:
- `app.kubernetes.io/name` becomes `service.name`
- `app.kubernetes.io/version` becomes `service.version`
- `app.kubernetes.io/part-of` becomes `service.namespace`
- `app.kubernetes.io/instance` becomes `service.instance.id`

```yaml
apiVersion: v1
Expand All @@ -761,7 +762,6 @@ metadata:
app.kubernetes.io/name: "my-service"
app.kubernetes.io/version: "1.0.0"
app.kubernetes.io/part-of: "shop"
app.kubernetes.io/instance: "my-service-123"
spec:
containers:
- name: main-container
Expand Down Expand Up @@ -794,6 +794,35 @@ The priority for setting resource attributes is as follows (first found wins):
This priority is applied for each resource attribute separately, so it is possible to set some attributes via
annotations and others via labels.

### How resource attributes are calculated from the pod's metadata

The following resource attributes are calculated from the pod's metadata.

#### How service.name is calculated

Choose the first value found:

- `k8s.depleyment.name`
- `k8s.replicaset.name`
- `k8s.statefulset.name`
- `k8s.daemonset.name`
- `k8s.cronjob.name`
- `k8s.job.name`
- `k8s.pod.name`
- `k8s.container.name`

#### How service.version is calculated

Take the tag from the docker image name of the container.
If the tag contains a `/`, the tag is ignored (this can happen if the image name contains a port number).

#### How service.instance.id is calculated

Create a unique identifier for the application running in the container.
The identifier is created by concatenating the following values:

`<k8s.namespace.name>.<k8s.pod.name>.<k8s.container.name>`

## Contributing and Developing

Please see [CONTRIBUTING.md](CONTRIBUTING.md).
Expand Down
1 change: 0 additions & 1 deletion apis/v1alpha1/instrumentation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ type Defaults struct {
// - `app.kubernetes.io/name` becomes `service.name`
// - `app.kubernetes.io/version` becomes `service.version`
// - `app.kubernetes.io/part-of` becomes `service.namespace`
// - `app.kubernetes.io/instance` becomes `service.instance.id`
UseLabelsForResourceAttributes bool `json:"useLabelsForResourceAttributes,omitempty"`
}

Expand Down
3 changes: 1 addition & 2 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1389,8 +1389,7 @@ Defaults defines default values for the instrumentation.
UseLabelsForResourceAttributes defines whether to use common labels for resource attributes:
- `app.kubernetes.io/name` becomes `service.name`
- `app.kubernetes.io/version` becomes `service.version`
- `app.kubernetes.io/part-of` becomes `service.namespace`
- `app.kubernetes.io/instance` becomes `service.instance.id`<br/>
- `app.kubernetes.io/part-of` becomes `service.namespace`<br/>
</td>
<td>false</td>
</tr></tbody>
Expand Down
7 changes: 3 additions & 4 deletions pkg/constants/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ const (
AnnotationDefaultAutoInstrumentationApacheHttpd = InstrumentationPrefix + "default-auto-instrumentation-apache-httpd-image"
AnnotationDefaultAutoInstrumentationNginx = InstrumentationPrefix + "default-auto-instrumentation-nginx-image"

LabelAppName = "app.kubernetes.io/name"
LabelAppInstance = "app.kubernetes.io/instance"
LabelAppVersion = "app.kubernetes.io/version"
LabelAppPartOf = "app.kubernetes.io/part-of"
LabelAppName = "app.kubernetes.io/name"
LabelAppVersion = "app.kubernetes.io/version"
LabelAppPartOf = "app.kubernetes.io/part-of"

LabelTargetAllocator = "opentelemetry.io/target-allocator"
ResourceAttributeAnnotationPrefix = "resource.opentelemetry.io/"
Expand Down
13 changes: 9 additions & 4 deletions pkg/instrumentation/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,11 +469,16 @@ func chooseServiceVersion(pod corev1.Pod, useLabelsForResourceAttributes bool, i

// chooseServiceInstanceId returns the service.instance.id to be used in the instrumentation.
// The precedence is as follows:
// 1. annotation with key "service.instance.id" or "app.kubernetes.io/instance"
// 1. annotation with key "service.instance.id"
// 2. namespace name + pod name + container name
// (as defined by https://opentelemetry.io/docs/specs/semconv/resource/#service-experimental)
func createServiceInstanceId(pod corev1.Pod, useLabelsForResourceAttributes bool, namespaceName, podName, containerName string) string {
serviceInstanceId := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceInstanceIDKey, constants.LabelAppInstance)
func createServiceInstanceId(pod corev1.Pod, namespaceName, podName, containerName string) string {
// Do not use labels for service instance id,
// because multiple containers in the same pod would get the same service instance id,
// which violates the uniqueness requirement of service instance id -
// see https://opentelemetry.io/docs/specs/semconv/resource/#service-experimental.
// We still allow the user to set the service instance id via annotation, because this is explicitly set by the user.
serviceInstanceId := chooseLabelOrAnnotation(pod, false, semconv.ServiceInstanceIDKey, "")
if serviceInstanceId != "" {
return serviceInstanceId
}
Expand Down Expand Up @@ -522,7 +527,7 @@ func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.I
k8sResources[semconv.K8SPodNameKey] = pod.Name
k8sResources[semconv.K8SPodUIDKey] = string(pod.UID)
k8sResources[semconv.K8SNodeNameKey] = pod.Spec.NodeName
k8sResources[semconv.ServiceInstanceIDKey] = createServiceInstanceId(pod, useLabelsForResourceAttributes, ns.Name, fmt.Sprintf("$(%s)", constants.EnvPodName), pod.Spec.Containers[index].Name)
k8sResources[semconv.ServiceInstanceIDKey] = createServiceInstanceId(pod, ns.Name, fmt.Sprintf("$(%s)", constants.EnvPodName), pod.Spec.Containers[index].Name)
i.addParentResourceLabels(ctx, otelinst.Spec.Resource.AddK8sUIDAttributes, ns, pod.ObjectMeta, k8sResources)

for k, v := range k8sResources {
Expand Down
44 changes: 19 additions & 25 deletions pkg/instrumentation/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,9 @@ func TestSDKInjection(t *testing.T) {
},
},
Labels: map[string]string{
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/instance": "app-id",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
},
Annotations: map[string]string{
"resource.opentelemetry.io/foo": "bar",
Expand All @@ -180,10 +179,9 @@ func TestSDKInjection(t *testing.T) {
Name: "app",
UID: "pod-uid",
Labels: map[string]string{
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/instance": "app-id",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
},
Annotations: map[string]string{
"resource.opentelemetry.io/foo": "bar",
Expand Down Expand Up @@ -396,10 +394,9 @@ func TestSDKInjection(t *testing.T) {
},
},
Labels: map[string]string{
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/instance": "app-id",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
},
Annotations: map[string]string{
"resource.opentelemetry.io/foo": "bar",
Expand All @@ -420,10 +417,9 @@ func TestSDKInjection(t *testing.T) {
Name: "app",
UID: "pod-uid",
Labels: map[string]string{
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/instance": "app-id",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
"app.kubernetes.io/name": "app-name",
"app.kubernetes.io/version": "v1",
"app.kubernetes.io/part-of": "shop",
},
Annotations: map[string]string{
"resource.opentelemetry.io/foo": "bar",
Expand Down Expand Up @@ -481,7 +477,7 @@ func TestSDKInjection(t *testing.T) {
},
{
Name: "OTEL_RESOURCE_ATTRIBUTES",
Value: "foo=bar,k8s.container.name=application-name,k8s.deployment.name=my-deployment,k8s.deployment.uid=depuid,k8s.namespace.name=project1,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),k8s.pod.uid=pod-uid,k8s.replicaset.name=my-replicaset,k8s.replicaset.uid=rsuid,service.instance.id=app-id,service.namespace=shop,service.version=v1",
Value: "foo=bar,k8s.container.name=application-name,k8s.deployment.name=my-deployment,k8s.deployment.uid=depuid,k8s.namespace.name=project1,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),k8s.pod.uid=pod-uid,k8s.replicaset.name=my-replicaset,k8s.replicaset.uid=rsuid,service.instance.id=project1.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).application-name,service.namespace=shop,service.version=v1",
},
},
},
Expand Down Expand Up @@ -516,10 +512,9 @@ func TestSDKInjection(t *testing.T) {
Namespace: "project1",
Name: "app",
Labels: map[string]string{
"app.kubernetes.io/name": "not-used",
"app.kubernetes.io/instance": "not-used",
"app.kubernetes.io/version": "not-used",
"app.kubernetes.io/part-of": "not-used",
"app.kubernetes.io/name": "not-used",
"app.kubernetes.io/version": "not-used",
"app.kubernetes.io/part-of": "not-used",
},
},
Spec: corev1.PodSpec{
Expand Down Expand Up @@ -557,10 +552,9 @@ func TestSDKInjection(t *testing.T) {
Namespace: "project1",
Name: "app",
Labels: map[string]string{
"app.kubernetes.io/name": "not-used",
"app.kubernetes.io/instance": "not-used",
"app.kubernetes.io/version": "not-used",
"app.kubernetes.io/part-of": "not-used",
"app.kubernetes.io/name": "not-used",
"app.kubernetes.io/version": "not-used",
"app.kubernetes.io/part-of": "not-used",
},
},
Spec: corev1.PodSpec{
Expand Down
Loading