read resource attributes from annotations (#3204)

* read resource attributes from annotations * use labels * changelog entry * fix rebase * change prefix to "resource.opentelemetry.io/" * change prefix to "resource.opentelemetry.io/" * fix rebase * update changelog and docs * changes from feedback * changes from feedback * generate * Update pkg/instrumentation/apachehttpd.go Co-authored-by: Jacob Aronoff <[email protected]> * pr review * comments * lint * lint * Update README.md Co-authored-by: Jacob Aronoff <[email protected]> * change priority to avoid breaking change * add e2e test * add e2e test * add e2e test * pr review * use pod methods * don't modify the existing chainsaw test * don't modify the existing chainsaw test --------- Co-authored-by: Jacob Aronoff <[email protected]>
open-telemetry · Sep 25, 2024 · e82f595 · e82f595
1 parent e40e3eb
commit e82f595
Show file tree

Hide file tree

Showing 20 changed files with 720 additions and 46 deletions.
diff --git a/.chloggen/resource-attribute-from-annotations.yaml b/.chloggen/resource-attribute-from-annotations.yaml
@@ -0,0 +1,24 @@
+change_type: enhancement
+
+component: auto-instrumentation
+
+note: Add support for k8s labels such as app.kubernetes.io/name for resource attributes
+
+issues: [3112]
+
+subtext: |
+  You can opt-in as follows:
+  ```yaml
+  apiVersion: opentelemetry.io/v1alpha1
+  kind: Instrumentation
+  metadata:
+    name: my-instrumentation
+  spec:
+    defaults:
+      useLabelsForResourceAttributes: true
+  ```
+  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`
diff --git a/README.md b/README.md
@@ -717,7 +717,9 @@ spec:
 EOF
 ```
 
-### Setting instrumentation resource attributes via namespace annotations
+## Configure resource attributes
+
+### 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.
 
@@ -734,7 +736,59 @@ spec:
   containers:
   - name: main-container
     image: your-image:tag
- ```
+```
+
+### Configure resource attributes with labels
+
+You can also use common labels to set resource attributes. 
+
+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
+kind: Pod
+metadata:
+  name: example-pod
+  labels:
+    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
+    image: your-image:tag
+```
+
+This requires an explicit opt-in as follows:
+
+```yaml
+apiVersion: opentelemetry.io/v1alpha1
+kind: Instrumentation
+metadata:
+  name: my-instrumentation
+spec:
+  defaults:
+    useLabelsForResourceAttributes: true
+```
+
+### Priority for setting resource attributes
+
+The priority for setting resource attributes is as follows (first found wins):
+
+1. Resource attributes set via `OTEL_RESOURCE_ATTRIBUTES` and `OTEL_SERVICE_NAME` environment variables
+2. Resource attributes set via annotations (with the `resource.opentelemetry.io/` prefix)
+3. Resource attributes set via labels (e.g. `app.kubernetes.io/name`) 
+   if the `Instrumentation` CR has defaults.useLabelsForResourceAttributes=true (see above)
+4. Resource attributes calculated from the pod's metadata (e.g. `k8s.pod.name`)
+5. Resource attributes set via the `Instrumentation` CR (in the `spec.resource.resourceAttributes` section)
+
+This priority is applied for each resource attribute separately, so it is possible to set some attributes via 
+annotations and others via labels.
 
 ## Compatibility matrix
 

diff --git a/apis/v1alpha1/instrumentation_types.go b/apis/v1alpha1/instrumentation_types.go
@@ -40,6 +40,9 @@ type InstrumentationSpec struct {
 	// +optional
 	Sampler `json:"sampler,omitempty"`
 
+	// Defaults defines default values for the instrumentation.
+	Defaults Defaults `json:"defaults,omitempty"`
+
 	// Env defines common env vars. There are four layers for env vars' definitions and
 	// the precedence order is: `original container env vars` > `language specific env vars` > `common env vars` > `instrument spec configs' vars`.
 	// If the former var had been defined, then the other vars would be ignored.
@@ -114,6 +117,16 @@ type Sampler struct {
 	Argument string `json:"argument,omitempty"`
 }
 
+// Defaults defines default values for the instrumentation.
+type Defaults struct {
+	// 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`
+	UseLabelsForResourceAttributes bool `json:"useLabelsForResourceAttributes,omitempty"`
+}
+
 // Java defines Java SDK and instrumentation configuration.
 type Java struct {
 	// Image is a container image with javaagent auto-instrumentation JAR.

diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go
diff --git a/bundle/community/manifests/opentelemetry.io_instrumentations.yaml b/bundle/community/manifests/opentelemetry.io_instrumentations.yaml
@@ -224,6 +224,11 @@ spec:
                     pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                     x-kubernetes-int-or-string: true
                 type: object
+              defaults:
+                properties:
+                  useLabelsForResourceAttributes:
+                    type: boolean
+                type: object
               dotnet:
                 properties:
                   env:

diff --git a/bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml b/bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml
@@ -224,6 +224,11 @@ spec:
                     pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                     x-kubernetes-int-or-string: true
                 type: object
+              defaults:
+                properties:
+                  useLabelsForResourceAttributes:
+                    type: boolean
+                type: object
               dotnet:
                 properties:
                   env:

diff --git a/config/crd/bases/opentelemetry.io_instrumentations.yaml b/config/crd/bases/opentelemetry.io_instrumentations.yaml
@@ -222,6 +222,11 @@ spec:
                     pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                     x-kubernetes-int-or-string: true
                 type: object
+              defaults:
+                properties:
+                  useLabelsForResourceAttributes:
+                    type: boolean
+                type: object
               dotnet:
                 properties:
                   env:

diff --git a/docs/api.md b/docs/api.md
@@ -95,6 +95,13 @@ InstrumentationSpec defines the desired state of OpenTelemetry SDK and instrumen
           ApacheHttpd defines configuration for Apache HTTPD auto-instrumentation.<br/>
         </td>
         <td>false</td>
+      </tr><tr>
+        <td><b><a href="#instrumentationspecdefaults">defaults</a></b></td>
+        <td>object</td>
+        <td>
+          Defaults defines default values for the instrumentation.<br/>
+        </td>
+        <td>false</td>
       </tr><tr>
         <td><b><a href="#instrumentationspecdotnet">dotnet</a></b></td>
         <td>object</td>
@@ -887,6 +894,37 @@ only the result of this request.<br/>
 </table>
 
 
+### Instrumentation.spec.defaults
+<sup><sup>[↩ Parent](#instrumentationspec)</sup></sup>
+
+
+
+Defaults defines default values for the instrumentation.
+
+<table>
+    <thead>
+        <tr>
+            <th>Name</th>
+            <th>Type</th>
+            <th>Description</th>
+            <th>Required</th>
+        </tr>
+    </thead>
+    <tbody><tr>
+        <td><b>useLabelsForResourceAttributes</b></td>
+        <td>boolean</td>
+        <td>
+          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/>
+        </td>
+        <td>false</td>
+      </tr></tbody>
+</table>
+
+
 ### Instrumentation.spec.dotnet
 <sup><sup>[↩ Parent](#instrumentationspec)</sup></sup>
 

diff --git a/pkg/constants/env.go b/pkg/constants/env.go
@@ -31,12 +31,18 @@ const (
 	AnnotationDefaultAutoInstrumentationApacheHttpd = InstrumentationPrefix + "default-auto-instrumentation-apache-httpd-image"
 	AnnotationDefaultAutoInstrumentationNginx       = InstrumentationPrefix + "default-auto-instrumentation-nginx-image"
 
-	EnvPodName              = "OTEL_RESOURCE_ATTRIBUTES_POD_NAME"
-	EnvPodUID               = "OTEL_RESOURCE_ATTRIBUTES_POD_UID"
-	EnvPodIP                = "OTEL_POD_IP"
-	EnvNodeName             = "OTEL_RESOURCE_ATTRIBUTES_NODE_NAME"
-	EnvNodeIP               = "OTEL_NODE_IP"
-	OtelAnnotationNamespace = "resource.opentelemetry.io/"
+	LabelAppName     = "app.kubernetes.io/name"
+	LabelAppInstance = "app.kubernetes.io/instance"
+	LabelAppVersion  = "app.kubernetes.io/version"
+	LabelAppPartOf   = "app.kubernetes.io/part-of"
+
+	ResourceAttributeAnnotationPrefix = "resource.opentelemetry.io/"
+
+	EnvPodName  = "OTEL_RESOURCE_ATTRIBUTES_POD_NAME"
+	EnvPodUID   = "OTEL_RESOURCE_ATTRIBUTES_POD_UID"
+	EnvPodIP    = "OTEL_POD_IP"
+	EnvNodeName = "OTEL_RESOURCE_ATTRIBUTES_NODE_NAME"
+	EnvNodeIP   = "OTEL_NODE_IP"
 
 	FlagCRMetrics   = "enable-cr-metrics"
 	FlagApacheHttpd = "enable-apache-httpd-instrumentation"

diff --git a/pkg/instrumentation/apachehttpd.go b/pkg/instrumentation/apachehttpd.go
@@ -59,7 +59,7 @@ const (
 	6) Inject mounting of volumes / files into appropriate directories in application container
 */
 
-func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod corev1.Pod, index int, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
+func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod corev1.Pod, useLabelsForResourceAttributes bool, index int, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
 
 	// caller checks if there is at least one container
 	container := &pod.Spec.Containers[index]
@@ -162,7 +162,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod
 			Env: []corev1.EnvVar{
 				{
 					Name:  apacheAttributesEnvVar,
-					Value: getApacheOtelConfig(pod, apacheSpec, index, otlpEndpoint, resourceMap),
+					Value: getApacheOtelConfig(pod, useLabelsForResourceAttributes, apacheSpec, index, otlpEndpoint, resourceMap),
 				},
 				{Name: apacheServiceInstanceIdEnvVar,
 					ValueFrom: &corev1.EnvVarSource{
@@ -201,7 +201,7 @@ func isApacheInitContainerMissing(pod corev1.Pod, containerName string) bool {
 
 // Calculate Apache HTTPD agent configuration file based on attributes provided by the injection rules
 // and by the pod values.
-func getApacheOtelConfig(pod corev1.Pod, apacheSpec v1alpha1.ApacheHttpd, index int, otelEndpoint string, resourceMap map[string]string) string {
+func getApacheOtelConfig(pod corev1.Pod, useLabelsForResourceAttributes bool, apacheSpec v1alpha1.ApacheHttpd, index int, otelEndpoint string, resourceMap map[string]string) string {
 	template := `
 #Load the Otel Webserver SDK
 LoadFile %[1]s/sdk_lib/lib/libopentelemetry_common.so
@@ -222,7 +222,7 @@ LoadModule otel_apache_module %[1]s/WebServerModule/Apache/libmod_apache_otel%[2
 	if otelEndpoint == "" {
 		otelEndpoint = "http://localhost:4317/"
 	}
-	serviceName := chooseServiceName(pod, resourceMap, index)
+	serviceName := chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, index)
 	serviceNamespace := pod.GetNamespace()
 	if len(serviceNamespace) == 0 {
 		serviceNamespace = resourceMap[string(semconv.K8SNamespaceNameKey)]

diff --git a/pkg/instrumentation/apachehttpd_test.go b/pkg/instrumentation/apachehttpd_test.go
@@ -417,7 +417,7 @@ func TestInjectApacheHttpdagent(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod := injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, test.pod, 0, "http://otlp-endpoint:4317", resourceMap)
+			pod := injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, test.pod, false, 0, "http://otlp-endpoint:4317", resourceMap)
 			assert.Equal(t, test.expected, pod)
 		})
 	}
@@ -527,7 +527,7 @@ func TestInjectApacheHttpdagentUnknownNamespace(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod := injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, test.pod, 0, "http://otlp-endpoint:4317", resourceMap)
+			pod := injectApacheHttpdagent(logr.Discard(), test.ApacheHttpd, test.pod, false, 0, "http://otlp-endpoint:4317", resourceMap)
 			assert.Equal(t, test.expected, pod)
 		})
 	}

diff --git a/pkg/instrumentation/nginx.go b/pkg/instrumentation/nginx.go
@@ -62,7 +62,7 @@ const (
 	6) Inject mounting of volumes / files into appropriate directories in the application container
 */
 
-func injectNginxSDK(_ logr.Logger, nginxSpec v1alpha1.Nginx, pod corev1.Pod, index int, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
+func injectNginxSDK(_ logr.Logger, nginxSpec v1alpha1.Nginx, pod corev1.Pod, useLabelsForResourceAttributes bool, index int, otlpEndpoint string, resourceMap map[string]string) corev1.Pod {
 
 	// caller checks if there is at least one container
 	container := &pod.Spec.Containers[index]
@@ -217,7 +217,7 @@ mv ${NGINX_AGENT_CONF_DIR_FULL}/opentelemetry_agent.conf  ${NGINX_AGENT_CONF_DIR
 			Env: []corev1.EnvVar{
 				{
 					Name:  nginxAttributesEnvVar,
-					Value: getNginxOtelConfig(pod, nginxSpec, index, otlpEndpoint, resourceMap),
+					Value: getNginxOtelConfig(pod, useLabelsForResourceAttributes, nginxSpec, index, otlpEndpoint, resourceMap),
 				},
 				{
 					Name:  "OTEL_NGINX_I13N_SCRIPT",
@@ -277,12 +277,12 @@ func isNginxInitContainerMissing(pod corev1.Pod, containerName string) bool {
 
 // Calculate Nginx agent configuration file based on attributes provided by the injection rules
 // and by the pod values.
-func getNginxOtelConfig(pod corev1.Pod, nginxSpec v1alpha1.Nginx, index int, otelEndpoint string, resourceMap map[string]string) string {
+func getNginxOtelConfig(pod corev1.Pod, useLabelsForResourceAttributes bool, nginxSpec v1alpha1.Nginx, index int, otelEndpoint string, resourceMap map[string]string) string {
 
 	if otelEndpoint == "" {
 		otelEndpoint = "http://localhost:4317/"
 	}
-	serviceName := chooseServiceName(pod, resourceMap, index)
+	serviceName := chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, index)
 	serviceNamespace := pod.GetNamespace()
 	if len(serviceNamespace) == 0 {
 		serviceNamespace = resourceMap[string(semconv.K8SNamespaceNameKey)]

diff --git a/pkg/instrumentation/nginx_test.go b/pkg/instrumentation/nginx_test.go
@@ -477,7 +477,7 @@ func TestInjectNginxSDK(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod := injectNginxSDK(logr.Discard(), test.Nginx, test.pod, 0, "http://otlp-endpoint:4317", resourceMap)
+			pod := injectNginxSDK(logr.Discard(), test.Nginx, test.pod, false, 0, "http://otlp-endpoint:4317", resourceMap)
 			assert.Equal(t, test.expected, pod)
 		})
 	}
@@ -600,7 +600,7 @@ func TestInjectNginxUnknownNamespace(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			pod := injectNginxSDK(logr.Discard(), test.Nginx, test.pod, 0, "http://otlp-endpoint:4317", resourceMap)
+			pod := injectNginxSDK(logr.Discard(), test.Nginx, test.pod, false, 0, "http://otlp-endpoint:4317", resourceMap)
 			assert.Equal(t, test.expected, pod)
 		})
 	}