From 8745d545a017b8917e23b9247b9c268a2d14349f Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 30 Aug 2024 14:28:02 +0200 Subject: [PATCH] changes from feedback --- .../resource-attribute-from-annotations.yaml | 16 ++- README.md | 33 ++++- apis/v1alpha1/instrumentation_types.go | 9 ++ pkg/constants/env.go | 8 +- pkg/instrumentation/apachehttpd.go | 10 +- pkg/instrumentation/apachehttpd_test.go | 4 +- pkg/instrumentation/nginx.go | 8 +- pkg/instrumentation/nginx_test.go | 4 +- pkg/instrumentation/sdk.go | 68 ++++++---- pkg/instrumentation/sdk_test.go | 125 +++++++++++++----- 10 files changed, 201 insertions(+), 84 deletions(-) diff --git a/.chloggen/resource-attribute-from-annotations.yaml b/.chloggen/resource-attribute-from-annotations.yaml index 90b006e03c..1ddf782c5d 100755 --- a/.chloggen/resource-attribute-from-annotations.yaml +++ b/.chloggen/resource-attribute-from-annotations.yaml @@ -1,13 +1,23 @@ -change_type: breaking +change_type: enhancement component: auto-instrumentation -note: Add support for k8s labels such as app.kubernetes.io/name and annotations for resource attributes +note: Add support for k8s labels such as app.kubernetes.io/name for resource attributes issues: [3112] subtext: | - It's a breaking change because these annotations are well-known and widely used in the Kubernetes ecosystem. The following labels are supported: + 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` diff --git a/README.md b/README.md index 32ac3d865d..1f093e4b24 100644 --- a/README.md +++ b/README.md @@ -735,7 +735,7 @@ spec: image: your-image:tag ``` -You can also use labels to set resource attributes. +You can also use common labels to set resource attributes. The following labels are supported: - `app.kubernetes.io/name` becomes `service.name` @@ -754,11 +754,36 @@ metadata: app.kubernetes.io/part-of: "shop" app.kubernetes.io/instance: "my-service-123" spec: - containers: - - name: main-container - image: your-image:tag + 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): + +- Resource attributes set via `OTEL_RESOURCE_ATTRIBUTES` and `OTEL_SERVICE_NAME` environment variables +- Resource attributes calculated from the pod's metadata (e.g. `k8s.pod.name`) +- Resource attributes set via the `Instrumentation` CR (in the `spec.resource.resourceAttributes` section) +- Resource attributes set via annotations (with the `resource.opentelemetry.io/` prefix) +- Resource attributes set via labels (e.g. `app.kubernetes.io/name`) + +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 ### OpenTelemetry Operator vs. OpenTelemetry Collector diff --git a/apis/v1alpha1/instrumentation_types.go b/apis/v1alpha1/instrumentation_types.go index 8345d3c38a..4bba2bab70 100644 --- 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,12 @@ 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. + 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/pkg/constants/env.go b/pkg/constants/env.go index 0935468dbd..ac89f13e6d 100644 --- a/pkg/constants/env.go +++ b/pkg/constants/env.go @@ -31,10 +31,10 @@ const ( AnnotationDefaultAutoInstrumentationApacheHttpd = InstrumentationPrefix + "default-auto-instrumentation-apache-httpd-image" AnnotationDefaultAutoInstrumentationNginx = InstrumentationPrefix + "default-auto-instrumentation-nginx-image" - AnnotationAppName = "app.kubernetes.io/name" - AnnotationAppInstance = "app.kubernetes.io/instance" - AnnotationAppVersion = "app.kubernetes.io/version" - AnnotationAppPartOf = "app.kubernetes.io/part-of" + 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/" diff --git a/pkg/instrumentation/apachehttpd.go b/pkg/instrumentation/apachehttpd.go index 34925473bb..d5ef7615e8 100644 --- 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] @@ -95,7 +95,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod MountPath: apacheAgentConfDirFull, }) // remove resource requirements since those are then reserved for the lifetime of a pod - // and we definitely do not need them for the init container for cp command + // ]and we definitely do not need them for the init container for cp command cloneContainer.Resources = apacheSpec.Resources // remove livenessProbe, readinessProbe, and startupProbe, since not supported on init containers cloneContainer.LivenessProbe = nil @@ -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 index e938f40c70..3a93d7418d 100644 --- 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 index 350a3533b9..7bcc39d611 100644 --- 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 index 65a9d5b8a0..b483c38cf4 100644 --- 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) }) } diff --git a/pkg/instrumentation/sdk.go b/pkg/instrumentation/sdk.go index c7a1073de3..7c36d7a763 100644 --- a/pkg/instrumentation/sdk.go +++ b/pkg/instrumentation/sdk.go @@ -64,6 +64,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations javaContainers := insts.Java.Containers + useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes for _, container := range strings.Split(javaContainers, ",") { index := getContainerIndex(container, pod) pod, err = injectJavaagent(otelinst.Spec.Java, pod, index) @@ -71,7 +72,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations i.logger.Info("Skipping javaagent injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) } else { pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, useLabelsForResourceAttributes, index, index) pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, javaInitContainerName) } } @@ -90,7 +91,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations i.logger.Info("Skipping NodeJS SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) } else { pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, otelinst.Spec.Defaults.UseLabelsForResourceAttributes, index, index) pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, nodejsInitContainerName) } } @@ -109,7 +110,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations i.logger.Info("Skipping Python SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) } else { pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, otelinst.Spec.Defaults.UseLabelsForResourceAttributes, index, index) pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, pythonInitContainerName) } } @@ -128,7 +129,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations i.logger.Info("Skipping DotNet SDK injection", "reason", err.Error(), "container", pod.Spec.Containers[index].Name) } else { pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, otelinst.Spec.Defaults.UseLabelsForResourceAttributes, index, index) pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, dotnetInitContainerName) } } @@ -149,7 +150,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations } else { // Common env vars and config need to be applied to the agent contain. pod = i.injectCommonEnvVar(otelinst, pod, len(pod.Spec.Containers)-1) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, len(pod.Spec.Containers)-1, 0) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, otelinst.Spec.Defaults.UseLabelsForResourceAttributes, len(pod.Spec.Containers)-1, 0) // Ensure that after all the env var coalescing we have a value for OTEL_GO_AUTO_TARGET_EXE idx := getIndexOfEnv(pod.Spec.Containers[len(pod.Spec.Containers)-1].Env, envOtelTargetExe) @@ -169,9 +170,10 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations index := getContainerIndex(container, pod) // Apache agent is configured via config files rather than env vars. // Therefore, service name, otlp endpoint and other attributes are passed to the agent injection method - pod = injectApacheHttpdagent(i.logger, otelinst.Spec.ApacheHttpd, pod, index, otelinst.Spec.Endpoint, i.createResourceMap(ctx, otelinst, ns, pod, index)) + useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes + pod = injectApacheHttpdagent(i.logger, otelinst.Spec.ApacheHttpd, pod, useLabelsForResourceAttributes, index, otelinst.Spec.Endpoint, i.createResourceMap(ctx, otelinst, ns, pod, useLabelsForResourceAttributes, index)) pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, useLabelsForResourceAttributes, index, index) pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, apacheAgentInitContainerName) pod = i.setInitContainerSecurityContext(pod, pod.Spec.Containers[index].SecurityContext, apacheAgentCloneContainerName) } @@ -187,9 +189,10 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations index := getContainerIndex(container, pod) // Nginx agent is configured via config files rather than env vars. // Therefore, service name, otlp endpoint and other attributes are passed to the agent injection method - pod = injectNginxSDK(i.logger, otelinst.Spec.Nginx, pod, index, otelinst.Spec.Endpoint, i.createResourceMap(ctx, otelinst, ns, pod, index)) + useLabelsForResourceAttributes := otelinst.Spec.Defaults.UseLabelsForResourceAttributes + pod = injectNginxSDK(i.logger, otelinst.Spec.Nginx, pod, useLabelsForResourceAttributes, index, otelinst.Spec.Endpoint, i.createResourceMap(ctx, otelinst, ns, pod, useLabelsForResourceAttributes, index)) pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, useLabelsForResourceAttributes, index, index) } } @@ -202,7 +205,7 @@ func (i *sdkInjector) inject(ctx context.Context, insts languageInstrumentations for _, container := range strings.Split(sdkContainers, ",") { index := getContainerIndex(container, pod) pod = i.injectCommonEnvVar(otelinst, pod, index) - pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, index, index) + pod = i.injectCommonSDKConfig(ctx, otelinst, ns, pod, otelinst.Spec.Defaults.UseLabelsForResourceAttributes, index, index) } } @@ -275,14 +278,14 @@ func (i *sdkInjector) injectCommonEnvVar(otelinst v1alpha1.Instrumentation, pod // and appIndex should be the same value. This is true for dotnet, java, nodejs, and python instrumentations. // Go requires the agent to be a different container in the pod, so the agentIndex should represent this new sidecar // and appIndex should represent the application being instrumented. -func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, agentIndex int, appIndex int) corev1.Pod { +func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, useLabelsForResourceAttributes bool, agentIndex int, appIndex int) corev1.Pod { container := &pod.Spec.Containers[agentIndex] - resourceMap := i.createResourceMap(ctx, otelinst, ns, pod, appIndex) + resourceMap := i.createResourceMap(ctx, otelinst, ns, pod, useLabelsForResourceAttributes, appIndex) idx := getIndexOfEnv(container.Env, constants.EnvOTELServiceName) if idx == -1 { container.Env = append(container.Env, corev1.EnvVar{ Name: constants.EnvOTELServiceName, - Value: chooseServiceName(pod, resourceMap, appIndex), + Value: chooseServiceName(pod, useLabelsForResourceAttributes, resourceMap, appIndex), }) } if otelinst.Spec.Exporter.Endpoint != "" { @@ -323,7 +326,7 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph idx = getIndexOfEnv(container.Env, constants.EnvOTELResourceAttrs) if idx == -1 || !strings.Contains(container.Env[idx].Value, string(semconv.ServiceVersionKey)) { - vsn := chooseServiceVersion(pod, appIndex) + vsn := chooseServiceVersion(pod, useLabelsForResourceAttributes, appIndex) if vsn != "" { resourceMap[string(semconv.ServiceVersionKey)] = vsn } @@ -394,8 +397,8 @@ func (i *sdkInjector) injectCommonSDKConfig(ctx context.Context, otelinst v1alph return pod } -func chooseServiceName(pod corev1.Pod, resources map[string]string, index int) string { - if name := chooseLabelOrAnnotation(pod, semconv.ServiceNameKey, constants.AnnotationAppName); name != "" { +func chooseServiceName(pod corev1.Pod, useLabelsForResourceAttributes bool, resources map[string]string, index int) string { + if name := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceNameKey, constants.LabelAppName); name != "" { return name } if name := resources[string(semconv.K8SDeploymentNameKey)]; name != "" { @@ -422,19 +425,21 @@ func chooseServiceName(pod corev1.Pod, resources map[string]string, index int) s return pod.Spec.Containers[index].Name } -func chooseLabelOrAnnotation(pod corev1.Pod, resource attribute.Key, labelKey string) string { +func chooseLabelOrAnnotation(pod corev1.Pod, useLabelsForResourceAttributes bool, resource attribute.Key, labelKey string) string { if v := pod.Annotations[(constants.ResourceAttributeAnnotationPrefix + string(resource))]; v != "" { return v } - if v := pod.Labels[labelKey]; v != "" { - return v + if useLabelsForResourceAttributes { + if v := pod.Labels[labelKey]; v != "" { + return v + } } return "" } // obtains version by splitting image string on ":" and extracting final element from resulting array. -func chooseServiceVersion(pod corev1.Pod, index int) string { - v := chooseLabelOrAnnotation(pod, semconv.ServiceVersionKey, constants.AnnotationAppVersion) +func chooseServiceVersion(pod corev1.Pod, useLabelsForResourceAttributes bool, index int) string { + v := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceVersionKey, constants.LabelAppVersion) if v != "" { return v } @@ -449,8 +454,8 @@ func chooseServiceVersion(pod corev1.Pod, index int) string { // creates the service.instance.id following the semantic defined by // https://github.com/open-telemetry/semantic-conventions/pull/312. -func createServiceInstanceId(pod corev1.Pod, namespaceName, podName, containerName string) string { - serviceInstanceId := chooseLabelOrAnnotation(pod, semconv.ServiceInstanceIDKey, constants.AnnotationAppInstance) +func createServiceInstanceId(pod corev1.Pod, useLabelsForResourceAttributes bool, namespaceName, podName, containerName string) string { + serviceInstanceId := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceInstanceIDKey, constants.LabelAppInstance) if serviceInstanceId != "" { return serviceInstanceId } @@ -464,7 +469,7 @@ func createServiceInstanceId(pod corev1.Pod, namespaceName, podName, containerNa // createResourceMap creates resource attribute map. // User defined attributes (in explicitly set env var) have higher precedence. -func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, index int) map[string]string { +func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.Instrumentation, ns corev1.Namespace, pod corev1.Pod, useLabelsForResourceAttributes bool, index int) map[string]string { // get existing resources env var and parse it into a map existingRes := map[string]bool{} existingResourceEnvIdx := getIndexOfEnv(pod.Spec.Containers[index].Env, constants.EnvOTELResourceAttrs) @@ -479,6 +484,7 @@ func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.I } } + // attributes and labels from the pod have the lowest precedence - they are overridden by later values res := map[string]string{} for k, v := range pod.Annotations { if strings.HasPrefix(k, constants.ResourceAttributeAnnotationPrefix) { @@ -488,12 +494,19 @@ func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.I } } } + partOf := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceNamespaceKey, constants.LabelAppPartOf) + if partOf != "" && !existingRes[string(semconv.ServiceNamespaceKey)] { + res[string(semconv.ServiceNamespaceKey)] = partOf + } + // entries from the CRD have higher precedence for k, v := range otelinst.Spec.Resource.Attributes { if !existingRes[k] { res[k] = v } } + + // k8s resources have the highest precedence (except for values set in environment variables) k8sResources := map[attribute.Key]string{} k8sResources[semconv.K8SNamespaceNameKey] = ns.Name k8sResources[semconv.K8SContainerNameKey] = pod.Spec.Containers[index].Name @@ -502,12 +515,9 @@ 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, ns.Name, fmt.Sprintf("$(%s)", constants.EnvPodName), pod.Spec.Containers[index].Name) + k8sResources[semconv.ServiceInstanceIDKey] = createServiceInstanceId(pod, useLabelsForResourceAttributes, ns.Name, fmt.Sprintf("$(%s)", constants.EnvPodName), pod.Spec.Containers[index].Name) i.addParentResourceLabels(ctx, otelinst.Spec.Resource.AddK8sUIDAttributes, ns, pod.ObjectMeta, k8sResources) - partOf := chooseLabelOrAnnotation(pod, semconv.ServiceNamespaceKey, constants.AnnotationAppPartOf) - if partOf != "" { - k8sResources[semconv.ServiceNamespaceKey] = partOf - } + for k, v := range k8sResources { if !existingRes[string(k)] && v != "" { res[string(k)] = v diff --git a/pkg/instrumentation/sdk_test.go b/pkg/instrumentation/sdk_test.go index 49ff7b1dad..cb7c761ff7 100644 --- a/pkg/instrumentation/sdk_test.go +++ b/pkg/instrumentation/sdk_test.go @@ -202,7 +202,7 @@ func TestSDKInjection(t *testing.T) { Env: []corev1.EnvVar{ { Name: "OTEL_SERVICE_NAME", - Value: "app-name", + Value: "my-deployment", }, { Name: "OTEL_EXPORTER_OTLP_ENDPOINT", @@ -238,7 +238,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.version=latest", }, }, }, @@ -247,7 +247,7 @@ func TestSDKInjection(t *testing.T) { }, }, { - name: "SDK env vars not defined - with explicit resource attributes annotations", + name: "SDK env vars not defined - use labels for resource attributes", inst: v1alpha1.Instrumentation{ Spec: v1alpha1.InstrumentationSpec{ Exporter: v1alpha1.Exporter{ @@ -261,6 +261,9 @@ func TestSDKInjection(t *testing.T) { Type: "parentbased_traceidratio", Argument: "0.25", }, + Defaults: v1alpha1.Defaults{ + UseLabelsForResourceAttributes: true, + }, }, }, pod: corev1.Pod{ @@ -277,17 +280,13 @@ func TestSDKInjection(t *testing.T) { }, }, Labels: map[string]string{ - "app.kubernetes.io/name": "app-name-hidden", - "app.kubernetes.io/instance": "app-id-hidden", - "app.kubernetes.io/version": "v1-hidden", - "app.kubernetes.io/part-of": "shop-hidden", + "app.kubernetes.io/name": "app-name", + "app.kubernetes.io/instance": "app-id", + "app.kubernetes.io/version": "v1", + "app.kubernetes.io/part-of": "shop", }, Annotations: map[string]string{ - "resource.opentelemetry.io/foo": "bar", - "resource.opentelemetry.io/service.name": "app-name", - "resource.opentelemetry.io/service.instance.id": "app-id", - "resource.opentelemetry.io/service.version": "v1", - "resource.opentelemetry.io/service.namespace": "shop", + "resource.opentelemetry.io/foo": "bar", }, }, Spec: corev1.PodSpec{ @@ -305,17 +304,13 @@ func TestSDKInjection(t *testing.T) { Name: "app", UID: "pod-uid", Labels: map[string]string{ - "app.kubernetes.io/name": "app-name-hidden", - "app.kubernetes.io/instance": "app-id-hidden", - "app.kubernetes.io/version": "v1-hidden", - "app.kubernetes.io/part-of": "shop-hidden", + "app.kubernetes.io/name": "app-name", + "app.kubernetes.io/instance": "app-id", + "app.kubernetes.io/version": "v1", + "app.kubernetes.io/part-of": "shop", }, Annotations: map[string]string{ - "resource.opentelemetry.io/foo": "bar", - "resource.opentelemetry.io/service.name": "app-name", - "resource.opentelemetry.io/service.instance.id": "app-id", - "resource.opentelemetry.io/service.version": "v1", - "resource.opentelemetry.io/service.namespace": "shop", + "resource.opentelemetry.io/foo": "bar", }, OwnerReferences: []metav1.OwnerReference{ { @@ -395,12 +390,21 @@ func TestSDKInjection(t *testing.T) { Type: "parentbased_traceidratio", Argument: "0.25", }, + Defaults: v1alpha1.Defaults{ + UseLabelsForResourceAttributes: true, + }, }, }, pod: corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ 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", + }, }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -409,7 +413,7 @@ func TestSDKInjection(t *testing.T) { Env: []corev1.EnvVar{ { Name: "OTEL_SERVICE_NAME", - Value: "explicitly_set", + Value: "explicit-name", }, { Name: "OTEL_EXPORTER_OTLP_ENDPOINT", @@ -425,7 +429,7 @@ func TestSDKInjection(t *testing.T) { }, { Name: "OTEL_RESOURCE_ATTRIBUTES", - Value: "foo=bar,k8s.container.name=other,service.version=explicitly_set,", + Value: "foo=bar,k8s.container.name=other,service.version=explicit-version,service.namespace=explicit-ns,service.instance.id=explicit-id,", }, }, }, @@ -436,6 +440,12 @@ func TestSDKInjection(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ 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", + }, }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -444,7 +454,7 @@ func TestSDKInjection(t *testing.T) { Env: []corev1.EnvVar{ { Name: "OTEL_SERVICE_NAME", - Value: "explicitly_set", + Value: "explicit-name", }, { Name: "OTEL_EXPORTER_OTLP_ENDPOINT", @@ -476,7 +486,7 @@ func TestSDKInjection(t *testing.T) { }, { Name: "OTEL_RESOURCE_ATTRIBUTES", - Value: "foo=bar,k8s.container.name=other,service.version=explicitly_set,fromcr=val,k8s.namespace.name=project1,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME)", + Value: "foo=bar,k8s.container.name=other,service.version=explicit-version,service.namespace=explicit-ns,service.instance.id=explicit-id,fromcr=val,k8s.namespace.name=project1,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME)", }, }, }, @@ -779,7 +789,7 @@ func TestSDKInjection(t *testing.T) { inj := sdkInjector{ client: k8sClient, } - pod := inj.injectCommonSDKConfig(context.Background(), test.inst, corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: test.pod.Namespace}}, test.pod, 0, 0) + pod := inj.injectCommonSDKConfig(context.Background(), test.inst, corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: test.pod.Namespace}}, test.pod, test.inst.Spec.Defaults.UseLabelsForResourceAttributes, 0, 0) _, err = json.MarshalIndent(pod, "", " ") assert.NoError(t, err) assert.Equal(t, test.expected, pod) @@ -2271,10 +2281,13 @@ func TestParentResourceLabels(t *testing.T) { func TestChooseServiceName(t *testing.T) { tests := []struct { - name string - resources map[string]string - index int - expectedServiceName string + name string + resources map[string]string + index int + expectedServiceName string + useLabelsForResourceAttributes bool + labelValue string + annotationValue string }{ { name: "first container", @@ -2296,6 +2309,48 @@ func TestChooseServiceName(t *testing.T) { index: 0, expectedServiceName: "my-pod", }, + { + name: "from pod label - useLabelsForResourceAttributes=false", + resources: map[string]string{ + string(semconv.K8SPodNameKey): "my-pod", + }, + index: 0, + labelValue: "annotation", + useLabelsForResourceAttributes: false, + expectedServiceName: "my-pod", + }, + { + name: "from pod label - useLabelsForResourceAttributes=true", + resources: map[string]string{ + string(semconv.K8SPodNameKey): "my-pod", + }, + index: 0, + labelValue: "label", + useLabelsForResourceAttributes: true, + expectedServiceName: "label", + }, + { + name: "from pod annotation - useLabelsForResourceAttributes=false", + resources: map[string]string{ + string(semconv.K8SPodNameKey): "my-pod", + }, + index: 0, + annotationValue: "annotation", + labelValue: "label", + useLabelsForResourceAttributes: false, + expectedServiceName: "annotation", + }, + { + name: "from pod annotation - useLabelsForResourceAttributes=true", + resources: map[string]string{ + string(semconv.K8SPodNameKey): "my-pod", + }, + index: 0, + annotationValue: "annotation", + labelValue: "label", + useLabelsForResourceAttributes: true, + expectedServiceName: "annotation", + }, { name: "from replicaset", resources: map[string]string{ @@ -2356,13 +2411,21 @@ func TestChooseServiceName(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { serviceName := chooseServiceName(corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app.kubernetes.io/name": test.labelValue, + }, + Annotations: map[string]string{ + "resource.opentelemetry.io/service.name": test.annotationValue, + }, + }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ {Name: "1st"}, {Name: "2nd"}, }, }, - }, test.resources, test.index) + }, test.useLabelsForResourceAttributes, test.resources, test.index) assert.Equal(t, test.expectedServiceName, serviceName) })