Add service.instance.id with Pod name (open-telemetry#2678)

* Add service.instance.id with Pod name * Add e2e assert Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com> * Fix tests Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com> * Remove unnecessary `service.name` Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com> * Rename var `someNamespace` - > `testNamespace` Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com> * Fix tests Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com> * Add changelog Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com> --------- Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>
yuriolisa · Apr 25, 2024 · 5bafbfd · 5bafbfd
1 parent e05bb5e
commit 5bafbfd
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 55 deletions.
diff --git a/.chloggen/add-service-instance-id.yaml b/.chloggen/add-service-instance-id.yaml
@@ -0,0 +1,26 @@
+# 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: Add attribute `service.instance.id` while pod is mutated.
+
+# One or more tracking issues related to the change
+issues: [2679]
+
+# (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: |
+  `service.instance.id` is expected to be `<namespace>.<podName>.<containerName>`
+  
+  But while pod is created it may not have the `podName` yet at the podMutator webhooks.
+  
+  This changed to use the env var `OTEL_RESOURCE_ATTRIBUTES_POD_NAME` which will be present at runtime.
+  `<namespace>.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).<containerName>`
+  
+  Making a valid and complete value for `service.instance.id` to be added.
+  
+  
diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go
diff --git a/pkg/instrumentation/sdk.go b/pkg/instrumentation/sdk.go
@@ -472,7 +472,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(ns.Name, pod.Name, pod.Spec.Containers[index].Name)
+	k8sResources[semconv.ServiceInstanceIDKey] = createServiceInstanceId(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 {
 		if !existingRes[string(k)] && v != "" {

diff --git a/pkg/instrumentation/sdk_test.go b/pkg/instrumentation/sdk_test.go
@@ -36,6 +36,12 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 )
 
+var testNamespace = corev1.Namespace{
+	ObjectMeta: metav1.ObjectMeta{
+		Name: "ns",
+	},
+}
+
 var defaultVolumeLimitSize = resource.MustParse("200Mi")
 
 var testResourceRequirements = corev1.ResourceRequirements{
@@ -206,7 +212,7 @@ func TestSDKInjection(t *testing.T) {
 								},
 								{
 									Name:  "OTEL_RESOURCE_ATTRIBUTES",
-									Value: "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=app,k8s.pod.uid=pod-uid,k8s.replicaset.name=my-replicaset,k8s.replicaset.uid=rsuid,service.instance.id=project1.app.application-name,service.version=latest",
+									Value: "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=app,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.version=latest",
 								},
 							},
 						},
@@ -374,7 +380,7 @@ func TestSDKInjection(t *testing.T) {
 								},
 								{
 									Name:  "OTEL_RESOURCE_ATTRIBUTES",
-									Value: "k8s.container.name=application-name,k8s.deployment.name=my-deployment,k8s.namespace.name=project1,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=app,k8s.pod.uid=pod-uid,k8s.replicaset.name=my-replicaset,service.instance.id=project1.app.application-name,service.version=latest",
+									Value: "k8s.container.name=application-name,k8s.deployment.name=my-deployment,k8s.namespace.name=project1,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=app,k8s.pod.uid=pod-uid,k8s.replicaset.name=my-replicaset,service.instance.id=project1.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).application-name,service.version=latest",
 								},
 							},
 						},
@@ -513,7 +519,7 @@ func TestInjectJava(t *testing.T) {
 	}
 	config := config.New()
 	pod := inj.inject(context.Background(), insts,
-		corev1.Namespace{},
+		testNamespace,
 		corev1.Pod{
 			Spec: corev1.PodSpec{
 				Containers: []corev1.Container{
@@ -605,7 +611,7 @@ func TestInjectJava(t *testing.T) {
 						},
 						{
 							Name:  "OTEL_RESOURCE_ATTRIBUTES",
-							Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+							Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 						},
 					},
 				},
@@ -634,7 +640,7 @@ func TestInjectNodeJS(t *testing.T) {
 	}
 	config := config.New()
 	pod := inj.inject(context.Background(), insts,
-		corev1.Namespace{},
+		testNamespace,
 		corev1.Pod{
 			Spec: corev1.PodSpec{
 				Containers: []corev1.Container{
@@ -726,7 +732,7 @@ func TestInjectNodeJS(t *testing.T) {
 						},
 						{
 							Name:  "OTEL_RESOURCE_ATTRIBUTES",
-							Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+							Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 						},
 					},
 				},
@@ -755,7 +761,7 @@ func TestInjectPython(t *testing.T) {
 	}
 	config := config.New()
 	pod := inj.inject(context.Background(), insts,
-		corev1.Namespace{},
+		testNamespace,
 		corev1.Pod{
 			Spec: corev1.PodSpec{
 				Containers: []corev1.Container{
@@ -862,7 +868,7 @@ func TestInjectPython(t *testing.T) {
 						},
 						{
 							Name:  "OTEL_RESOURCE_ATTRIBUTES",
-							Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+							Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 						},
 					},
 				},
@@ -890,7 +896,7 @@ func TestInjectDotNet(t *testing.T) {
 	}
 	config := config.New()
 	pod := inj.inject(context.Background(), insts,
-		corev1.Namespace{},
+		testNamespace,
 		corev1.Pod{
 			Spec: corev1.PodSpec{
 				Containers: []corev1.Container{
@@ -1005,7 +1011,7 @@ func TestInjectDotNet(t *testing.T) {
 						},
 						{
 							Name:  "OTEL_RESOURCE_ATTRIBUTES",
-							Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+							Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 						},
 					},
 				},
@@ -1183,7 +1189,7 @@ func TestInjectGo(t *testing.T) {
 								},
 								{
 									Name:  "OTEL_RESOURCE_ATTRIBUTES",
-									Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+									Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 								},
 							},
 						},
@@ -1300,7 +1306,7 @@ func TestInjectGo(t *testing.T) {
 								},
 								{
 									Name:  "OTEL_RESOURCE_ATTRIBUTES",
-									Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+									Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 								},
 							},
 						},
@@ -1325,7 +1331,7 @@ func TestInjectGo(t *testing.T) {
 			inj := sdkInjector{
 				logger: logr.Discard(),
 			}
-			pod := inj.inject(context.Background(), test.insts, corev1.Namespace{}, test.pod, test.config)
+			pod := inj.inject(context.Background(), test.insts, testNamespace, test.pod, test.config)
 			assert.Equal(t, test.expected, pod)
 		})
 	}
@@ -1406,7 +1412,7 @@ func TestInjectApacheHttpd(t *testing.T) {
 							Env: []corev1.EnvVar{
 								{
 									Name:  apacheAttributesEnvVar,
-									Value: "\n#Load the Otel Webserver SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_common.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_resources.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_trace.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_otlp_recordable.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_ostream_span.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_otlp_grpc.so\n#Load the Otel ApacheModule SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_webserver_sdk.so\n#Load the Apache Module. In this example for Apache 2.4\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Load the Apache Module. In this example for Apache 2.2\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel22.so\nLoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Attributes\nApacheModuleEnabled ON\nApacheModuleOtelExporterEndpoint https://collector:4318\nApacheModuleOtelSpanExporter otlp\nApacheModuleResolveBackends  ON\nApacheModuleServiceInstanceId <<SID-PLACEHOLDER>>\nApacheModuleServiceName app\nApacheModuleServiceNamespace apache-httpd\nApacheModuleTraceAsError  ON\n",
+									Value: "\n#Load the Otel Webserver SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_common.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_resources.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_trace.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_otlp_recordable.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_ostream_span.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_otlp_grpc.so\n#Load the Otel ApacheModule SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_webserver_sdk.so\n#Load the Apache Module. In this example for Apache 2.4\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Load the Apache Module. In this example for Apache 2.2\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel22.so\nLoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Attributes\nApacheModuleEnabled ON\nApacheModuleOtelExporterEndpoint https://collector:4318\nApacheModuleOtelSpanExporter otlp\nApacheModuleResolveBackends  ON\nApacheModuleServiceInstanceId <<SID-PLACEHOLDER>>\nApacheModuleServiceName app\nApacheModuleServiceNamespace ns\nApacheModuleTraceAsError  ON\n",
 								},
 								{Name: apacheServiceInstanceIdEnvVar,
 									ValueFrom: &corev1.EnvVarSource{
@@ -1484,7 +1490,7 @@ func TestInjectApacheHttpd(t *testing.T) {
 								},
 								{
 									Name:  "OTEL_RESOURCE_ATTRIBUTES",
-									Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME)",
+									Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app",
 								},
 							},
 						},
@@ -1499,7 +1505,7 @@ func TestInjectApacheHttpd(t *testing.T) {
 				logger: logr.Discard(),
 			}
 
-			pod := inj.inject(context.Background(), test.insts, corev1.Namespace{}, test.pod, test.config)
+			pod := inj.inject(context.Background(), test.insts, testNamespace, test.pod, test.config)
 			assert.Equal(t, test.expected, pod)
 		})
 	}
@@ -1585,7 +1591,7 @@ func TestInjectNginx(t *testing.T) {
 							Env: []corev1.EnvVar{
 								{
 									Name:  nginxAttributesEnvVar,
-									Value: "NginxModuleEnabled ON;\nNginxModuleOtelExporterEndpoint http://otlp-endpoint:4317;\nNginxModuleOtelMaxQueueSize 4096;\nNginxModuleOtelSpanExporter otlp;\nNginxModuleResolveBackends ON;\nNginxModuleServiceInstanceId <<SID-PLACEHOLDER>>;\nNginxModuleServiceName my-nginx-6c44bcbdd;\nNginxModuleServiceNamespace nginx;\nNginxModuleTraceAsError ON;\n",
+									Value: "NginxModuleEnabled ON;\nNginxModuleOtelExporterEndpoint http://otlp-endpoint:4317;\nNginxModuleOtelMaxQueueSize 4096;\nNginxModuleOtelSpanExporter otlp;\nNginxModuleResolveBackends ON;\nNginxModuleServiceInstanceId <<SID-PLACEHOLDER>>;\nNginxModuleServiceName my-nginx-6c44bcbdd;\nNginxModuleServiceNamespace ns;\nNginxModuleTraceAsError ON;\n",
 								},
 								{
 									Name:  "OTEL_NGINX_I13N_SCRIPT",
@@ -1664,7 +1670,7 @@ func TestInjectNginx(t *testing.T) {
 								},
 								{
 									Name:  "OTEL_RESOURCE_ATTRIBUTES",
-									Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=my-nginx-6c44bcbdd",
+									Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=my-nginx-6c44bcbdd,service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app",
 								},
 							},
 						},
@@ -1678,7 +1684,7 @@ func TestInjectNginx(t *testing.T) {
 			inj := sdkInjector{
 				logger: logr.Discard(),
 			}
-			pod := inj.inject(context.Background(), test.insts, corev1.Namespace{}, test.pod, test.config)
+			pod := inj.inject(context.Background(), test.insts, testNamespace, test.pod, test.config)
 			assert.Equal(t, test.expected, pod)
 		})
 	}
@@ -1701,7 +1707,7 @@ func TestInjectSdkOnly(t *testing.T) {
 	}
 	config := config.New()
 	pod := inj.inject(context.Background(), insts,
-		corev1.Namespace{},
+		testNamespace,
 		corev1.Pod{
 			Spec: corev1.PodSpec{
 				Containers: []corev1.Container{
@@ -1761,7 +1767,7 @@ func TestInjectSdkOnly(t *testing.T) {
 						},
 						{
 							Name:  "OTEL_RESOURCE_ATTRIBUTES",
-							Value: "k8s.container.name=app,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.version=latest",
+							Value: "k8s.container.name=app,k8s.namespace.name=ns,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=ns.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app,service.version=latest",
 						},
 					},
 				},

diff --git a/tests/e2e/env-vars/01-assert.yaml b/tests/e2e/env-vars/01-assert.yaml
@@ -42,4 +42,5 @@ spec:
         ( contains(@, 'k8s.deployment.name=my-deploy') ): true
         ( contains(@, concat('k8s.namespace.name=', $namespace)) ): true
         ( contains(@, 'k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME)') ): true
+        ( contains(@, join('', ['service.instance.id=', $namespace, '.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).myapp'])  ) ): true
         ( contains(@, 'service.version=main') ): true
diff --git a/tests/e2e/env-vars/02-assert.yaml b/tests/e2e/env-vars/02-assert.yaml
@@ -45,4 +45,5 @@ spec:
             ( contains(@, 'k8s.cronjob.name=my-cron-job') ): true
             ( contains(@, concat('k8s.namespace.name=', $namespace)) ): true
             ( contains(@, 'k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME)') ): true
+            ( contains(@, join('', ['service.instance.id=', $namespace, '.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).myapp'])  ) ): true
             ( contains(@, 'service.version=main') ): true
diff --git a/tests/e2e/env-vars/03-assert.yaml b/tests/e2e/env-vars/03-assert.yaml
@@ -44,4 +44,5 @@ spec:
             ( contains(@, 'k8s.job.name=my-job') ): true
             ( contains(@, concat('k8s.namespace.name=', $namespace)) ): true
             ( contains(@, 'k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME)') ): true
+            ( contains(@, join('', ['service.instance.id=', $namespace, '.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).myapp'])  ) ): true
             ( contains(@, 'service.version=main') ): true