Remove label mapping for service.instance.id (#3497)

* Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`, because multiple containers should never share the same `service.instance.id` * Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`, because multiple containers should never share the same `service.instance.id` * Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`, because multiple containers should never share the same `service.instance.id` * Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`, because multiple containers should never share the same `service.instance.id` * Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`, because multiple containers should never share the same `service.instance.id` * Remove the mapping of `app.kubernetes.io/instance` to `service.instance.id`, because multiple containers should never share the same `service.instance.id` * Update .chloggen/service-instanc-id-mapping.yaml Co-authored-by: Jacob Aronoff <[email protected]> * Update .chloggen/service-instanc-id-mapping.yaml Co-authored-by: Jacob Aronoff <[email protected]> * add docs * Update README.md Co-authored-by: Cyrille Le Clerc <[email protected]> * Apply suggestions from code review Co-authored-by: Cyrille Le Clerc <[email protected]> * add docs * add docs --------- Co-authored-by: Jacob Aronoff <[email protected]> Co-authored-by: Cyrille Le Clerc <[email protected]>
open-telemetry · Dec 10, 2024 · 0590057 · 0590057
1 parent 95ca52c
commit 0590057
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 39 deletions.
diff --git a/.chloggen/service-instanc-id-mapping.yaml b/.chloggen/service-instanc-id-mapping.yaml
@@ -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.
+
diff --git a/README.md b/README.md
@@ -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"
@@ -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
@@ -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
@@ -794,6 +794,38 @@ 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: 
+
+- `pod.annotation[resource.opentelemetry.io/service.name]`
+- `if (config[useLabelsForResourceAttributes]) pod.label[app.kubernetes.io/name]`
+- `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
+
+- `pod.annotation[resource.opentelemetry.io/service.version]`
+- `if (cfg[useLabelsForResourceAttributes]) pod.label[app.kubernetes.io/version]`
+- `if (contains(container docker image tag, '/') == false) container docker image tag`
+
+#### How `service.instance.id` is calculated
+
+
+- `pod.annotation[resource.opentelemetry.io/service.instance.id]`
+- `if (config[useLabelsForResourceAttributes]) pod.label[app.kubernetes.io/instance]`
+- `concat([k8s.namespace.name, k8s.pod.name, k8s.container.name], '.')`
+
 ## Contributing and Developing
 
 Please see [CONTRIBUTING.md](CONTRIBUTING.md).

diff --git a/apis/v1alpha1/instrumentation_types.go b/apis/v1alpha1/instrumentation_types.go
@@ -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"`
 }
 

diff --git a/docs/api.md b/docs/api.md
@@ -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>

diff --git a/pkg/constants/env.go b/pkg/constants/env.go
@@ -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/"

diff --git a/pkg/instrumentation/sdk.go b/pkg/instrumentation/sdk.go
@@ -474,11 +474,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
 	}
@@ -527,7 +532,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 {

diff --git a/pkg/instrumentation/sdk_test.go b/pkg/instrumentation/sdk_test.go
@@ -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",
@@ -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",
@@ -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",
@@ -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",
@@ -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",
 								},
 							},
 						},
@@ -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{
@@ -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{