Repository: istio/istio (go)

Issue:
Support OpenTelemetry resource attributes in traces
(This is used to request new product features, please visit <https://discuss.istio.io> for questions on using Istio)

**Describe the feature request**

Envoy supports resource attributes through the standard `OTEL_RESOURCE_ATTRIBUTES` environment variable, however it has to be enabled via: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/tracers/opentelemetry/resource_detectors/v3/environment_resource_detector.proto#extension-envoy-tracers-opentelemetry-resource-detectors-environment

Ideally istio would support this in addition to tags. Resource attributes are critical to be able to set the `environment` that a service lives in.

**Describe alternatives you've considered**
There are no alternatives, you cannot change this property on istio-proxies. Perhaps it would be possible with an EnvoyFilter?

**Affected product area (please put an X in all that apply)**

[ ] Ambient
[ ] Docs
[ ] Dual Stack
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[x] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

**Affected features (please put an X in all that apply)**

[x] Multi Cluster
[x] Virtual Machine
[x] Multi Control Plane


Pull request description:
OpenTelemetryTracingProvider: Allow to configure resource detectors
**Please provide a description of this PR:**
This PR uses the new resource detectors API config (https://github.com/istio/api/pull/3002) and allows configuring OpenTelemetry [resource detectors](https://opentelemetry.io/docs/languages/js/resources/#process--environment-resource-detection). 

Resource detectors are an important part of observability, allowing to correlate telemetry with things such as hosts, processes, cloud resources and much more. This PR adds the possibility to configure two resource detectors: 

- [Environment](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/tracers/opentelemetry/resource_detectors/v3/environment_resource_detector.proto#extension-envoy-tracers-opentelemetry-resource-detectors-environment)
- [Dynatrace](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/tracers/opentelemetry/resource_detectors/v3/dynatrace_resource_detector.proto#extension-envoy-tracers-opentelemetry-resource-detectors-dynatrace)


Closes #48885

I tested this locally with both resource detectors enabled and I confirmed the attributes are all added to the exported telemetry. 

Patch:
diff --git a/pilot/pkg/networking/core/v1alpha3/tracing.go b/pilot/pkg/networking/core/v1alpha3/tracing.go
index 7e458bd533..12630f8ed3 100644
--- a/pilot/pkg/networking/core/v1alpha3/tracing.go
+++ b/pilot/pkg/networking/core/v1alpha3/tracing.go
@@ -37,6 +37,7 @@ import (
 	"istio.io/istio/pilot/pkg/networking"
 	authz_model "istio.io/istio/pilot/pkg/security/authz/model"
 	"istio.io/istio/pilot/pkg/util/protoconv"
+	xdsfilters "istio.io/istio/pilot/pkg/xds/filters"
 	"istio.io/istio/pilot/pkg/xds/requestidextension"
 	"istio.io/istio/pkg/bootstrap/platform"
 	"istio.io/istio/pkg/config/constants"
@@ -269,17 +270,17 @@ func datadogConfig(serviceName, hostname, cluster string) (*anypb.Any, error) {
 }
 
 func otelConfig(serviceName, hostname, cluster string, otelProvider *meshconfig.MeshConfig_ExtensionProvider_OpenTelemetryTracingProvider) (*anypb.Any, error) {
-	var oc *tracingcfg.OpenTelemetryConfig
+	oc := &tracingcfg.OpenTelemetryConfig{
+		ServiceName: serviceName,
+	}
 
 	if otelProvider.GetHttp() == nil {
 		// export via gRPC
-		oc = &tracingcfg.OpenTelemetryConfig{
-			GrpcService: &core.GrpcService{
-				TargetSpecifier: &core.GrpcService_EnvoyGrpc_{
-					EnvoyGrpc: &core.GrpcService_EnvoyGrpc{
-						ClusterName: cluster,
-						Authority:   hostname,
-					},
+		oc.GrpcService = &core.GrpcService{
+			TargetSpecifier: &core.GrpcService_EnvoyGrpc_{
+				EnvoyGrpc: &core.GrpcService_EnvoyGrpc{
+					ClusterName: cluster,
+					Authority:   hostname,
 				},
 			},
 		}
@@ -290,18 +291,15 @@ func otelConfig(serviceName, hostname, cluster string, otelProvider *meshconfig.
 		if err != nil {
 			return nil, fmt.Errorf("could not parse otlp/http traces endpoint: %v", err)
 		}
-		oc = &tracingcfg.OpenTelemetryConfig{
-			HttpService: &core.HttpService{
-				HttpUri: &core.HttpUri{
-					Uri: te,
-					HttpUpstreamType: &core.HttpUri_Cluster{
-						Cluster: cluster,
-					},
-					Timeout: httpService.GetTimeout(),
+		oc.HttpService = &core.HttpService{
+			HttpUri: &core.HttpUri{
+				Uri: te,
+				HttpUpstreamType: &core.HttpUri_Cluster{
+					Cluster: cluster,
 				},
+				Timeout: httpService.GetTimeout(),
 			},
 		}
-
 		for _, h := range httpService.GetHeaders() {
 			hvo := &core.HeaderValueOption{
 				AppendAction: core.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD,
@@ -314,7 +312,20 @@ func otelConfig(serviceName, hostname, cluster string, otelProvider *meshconfig.
 		}
 	}
 
-	oc.ServiceName = serviceName
+	// Add configured resource detectors
+	if otelProvider.ResourceDetectors != nil {
+		res := []*core.TypedExtensionConfig{}
+		rd := otelProvider.ResourceDetectors
+
+		if rd.Environment != nil {
+			res = append(res, xdsfilters.EnvironmentResourceDetector)
+		}
+		if rd.Dynatrace != nil {
+			res = append(res, xdsfilters.DynatraceResourceDetector)
+		}
+		oc.ResourceDetectors = res
+	}
+
 	return anypb.New(oc)
 }
 
diff --git a/pilot/pkg/xds/filters/filters.go b/pilot/pkg/xds/filters/filters.go
index f007c4f4fe..d964591529 100644
--- a/pilot/pkg/xds/filters/filters.go
+++ b/pilot/pkg/xds/filters/filters.go
@@ -35,6 +35,7 @@ import (
 	hcm "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
 	sfsnetwork "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/set_filter_state/v3"
 	previoushost "github.com/envoyproxy/go-control-plane/envoy/extensions/retry/host/previous_hosts/v3"
+	resourcedetectors "github.com/envoyproxy/go-control-plane/envoy/extensions/tracers/opentelemetry/resource_detectors/v3"
 	rawbuffer "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/raw_buffer/v3"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 
@@ -426,3 +427,15 @@ var (
 	mtlsHTTP11ALPN = []string{"istio-http/1.1", "istio", "http/1.1"}
 	mtlsHTTP2ALPN  = []string{"istio-h2", "istio", "h2"}
 )
+
+// OpenTelemetry Resource Detectors
+var (
+	EnvironmentResourceDetector = &core.TypedExtensionConfig{
+		Name:        "envoy.tracers.opentelemetry.resource_detectors.environment",
+		TypedConfig: protoconv.MessageToAny(&resourcedetectors.EnvironmentResourceDetectorConfig{}),
+	}
+	DynatraceResourceDetector = &core.TypedExtensionConfig{
+		Name:        "envoy.tracers.opentelemetry.resource_detectors.dynatrace",
+		TypedConfig: protoconv.MessageToAny(&resourcedetectors.DynatraceResourceDetectorConfig{}),
+	}
+)
diff --git a/releasenotes/notes/otel-tracing-provider-resource-detectors.yaml b/releasenotes/notes/otel-tracing-provider-resource-detectors.yaml
new file mode 100644
index 0000000000..3aa77dffe2
--- /dev/null
+++ b/releasenotes/notes/otel-tracing-provider-resource-detectors.yaml
@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: feature
+area: telemetry
+issue:
+  - 48885
+releaseNotes:
+  - |
+    **Enabled** configuring Resource Detectors for the `OpenTelemetryTracingProvider` in `MeshConfig`.


Test patch:
diff --git a/pilot/pkg/networking/core/v1alpha3/tracing_test.go b/pilot/pkg/networking/core/v1alpha3/tracing_test.go
index 090df367ec..7940dadaa4 100644
--- a/pilot/pkg/networking/core/v1alpha3/tracing_test.go
+++ b/pilot/pkg/networking/core/v1alpha3/tracing_test.go
@@ -20,6 +20,7 @@ import (
 	core "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	tracingcfg "github.com/envoyproxy/go-control-plane/envoy/config/trace/v3"
 	hcm "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
+	resourcedetectors "github.com/envoyproxy/go-control-plane/envoy/extensions/tracers/opentelemetry/resource_detectors/v3"
 	tracing "github.com/envoyproxy/go-control-plane/envoy/type/tracing/v3"
 	xdstype "github.com/envoyproxy/go-control-plane/envoy/type/v3"
 	"github.com/google/go-cmp/cmp"
@@ -189,6 +190,15 @@ func TestConfigureTracing(t *testing.T) {
 			wantStartChildSpan: false,
 			wantReqIDExtCtx:    &defaultUUIDExtensionCtx,
 		},
+		{
+			name:   "basic config (with opentelemetry provider with resource detectors)",
+			inSpec: fakeTracingSpec(fakeOpenTelemetryResourceDetectors(), 99.999, false, true),
+			opts:   fakeOptsOnlyOpenTelemetryResourceDetectorsTelemetryAPI(),
+			want: fakeTracingConfig(
+				fakeOpenTelemetryResourceDetectorsProvider(clusterName, authority), 99.999, 256, append(defaultTracingTags(), fakeEnvTag)),
+			wantStartChildSpan: false,
+			wantReqIDExtCtx:    &defaultUUIDExtensionCtx,
+		},
 		{
 			name:               "client-only config for server",
 			inSpec:             fakeClientOnlyTracingSpec(fakeSkywalking(), 99.999, false, false),
@@ -583,6 +593,17 @@ func fakeOpenTelemetryHTTP() *meshconfig.MeshConfig_ExtensionProvider {
 	}
 }
 
+func fakeOpenTelemetryResourceDetectors() *meshconfig.MeshConfig_ExtensionProvider {
+	ep := fakeOpenTelemetryHTTP()
+
+	ep.GetOpentelemetry().ResourceDetectors = &meshconfig.MeshConfig_ExtensionProvider_ResourceDetectors{
+		Environment: &meshconfig.MeshConfig_ExtensionProvider_ResourceDetectors_EnvironmentResourceDetector{},
+		Dynatrace:   &meshconfig.MeshConfig_ExtensionProvider_ResourceDetectors_DynatraceResourceDetector{},
+	}
+
+	return ep
+}
+
 func fakeOptsOnlyOpenTelemetryGrpcTelemetryAPI() gatewayListenerOpts {
 	var opts gatewayListenerOpts
 	opts.push = &model.PushContext{
@@ -647,6 +668,18 @@ func fakeOptsOnlyOpenTelemetryHTTPTelemetryAPI() gatewayListenerOpts {
 	return opts
 }
 
+func fakeOptsOnlyOpenTelemetryResourceDetectorsTelemetryAPI() gatewayListenerOpts {
+	opts := fakeOptsOnlyOpenTelemetryHTTPTelemetryAPI()
+
+	ep := opts.push.Mesh.ExtensionProviders[0]
+	ep.GetOpentelemetry().ResourceDetectors = &meshconfig.MeshConfig_ExtensionProvider_ResourceDetectors{
+		Environment: &meshconfig.MeshConfig_ExtensionProvider_ResourceDetectors_EnvironmentResourceDetector{},
+		Dynatrace:   &meshconfig.MeshConfig_ExtensionProvider_ResourceDetectors_DynatraceResourceDetector{},
+	}
+
+	return opts
+}
+
 func fakeInboundOptsOnlySkywalkingTelemetryAPI() gatewayListenerOpts {
 	opts := fakeOptsOnlySkywalkingTelemetryAPI()
 	return opts
@@ -867,3 +900,41 @@ func fakeOpenTelemetryHTTPProvider(expectClusterName, expectAuthority string) *t
 		ConfigType: &tracingcfg.Tracing_Http_TypedConfig{TypedConfig: fakeOtelHTTPAny},
 	}
 }
+
+func fakeOpenTelemetryResourceDetectorsProvider(expectClusterName, expectAuthority string) *tracingcfg.Tracing_Http {
+	fakeOTelHTTPProviderConfig := &tracingcfg.OpenTelemetryConfig{
+		HttpService: &core.HttpService{
+			HttpUri: &core.HttpUri{
+				Uri: expectAuthority + "/v1/traces",
+				HttpUpstreamType: &core.HttpUri_Cluster{
+					Cluster: expectClusterName,
+				},
+				Timeout: &durationpb.Duration{Seconds: 3},
+			},
+			RequestHeadersToAdd: []*core.HeaderValueOption{
+				{
+					Header: &core.HeaderValue{
+						Key:   "custom-header",
+						Value: "custom-value",
+					},
+					AppendAction: core.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD,
+				},
+			},
+		},
+		ResourceDetectors: []*core.TypedExtensionConfig{
+			{
+				Name:        "envoy.tracers.opentelemetry.resource_detectors.environment",
+				TypedConfig: protoconv.MessageToAny(&resourcedetectors.EnvironmentResourceDetectorConfig{}),
+			},
+			{
+				Name:        "envoy.tracers.opentelemetry.resource_detectors.dynatrace",
+				TypedConfig: protoconv.MessageToAny(&resourcedetectors.DynatraceResourceDetectorConfig{}),
+			},
+		},
+	}
+	fakeOtelHTTPAny := protoconv.MessageToAny(fakeOTelHTTPProviderConfig)
+	return &tracingcfg.Tracing_Http{
+		Name:       envoyOpenTelemetry,
+		ConfigType: &tracingcfg.Tracing_Http_TypedConfig{TypedConfig: fakeOtelHTTPAny},
+	}
+}
diff --git a/tests/integration/telemetry/tracing/otelcollector/testdata/otel-tracing-res-detectors.yaml b/tests/integration/telemetry/tracing/otelcollector/testdata/otel-tracing-res-detectors.yaml
new file mode 100644
index 0000000000..050f979bbf
--- /dev/null
+++ b/tests/integration/telemetry/tracing/otelcollector/testdata/otel-tracing-res-detectors.yaml
@@ -0,0 +1,13 @@
+apiVersion: telemetry.istio.io/v1alpha1
+kind: Telemetry
+metadata:
+  name: otel-tracing
+spec:
+  tracing:
+  - providers:
+    - name: test-otel-res-detectors
+    randomSamplingPercentage: 100.0
+    customTags:
+      "provider":
+        literal:
+          value: "otel-grpc-with-res-detectors"
\ No newline at end of file
diff --git a/tests/integration/telemetry/tracing/otelcollector/tracing_test.go b/tests/integration/telemetry/tracing/otelcollector/tracing_test.go
index 34807e5747..5fc17210db 100644
--- a/tests/integration/telemetry/tracing/otelcollector/tracing_test.go
+++ b/tests/integration/telemetry/tracing/otelcollector/tracing_test.go
@@ -75,77 +75,72 @@ var otelTracingCfg string
 //go:embed testdata/otel-tracing-http.yaml
 var otelTracingHTTPCfg string
 
+//go:embed testdata/otel-tracing-res-detectors.yaml
+var otelTracingResDetectorsCfg string
+
 // TestProxyTracingOpenTelemetryProvider validates that Telemetry API configuration
 // referencing an OpenTelemetry provider will generate traces appropriately.
 // NOTE: This test relies on the priority of Telemetry API over MeshConfig tracing
 // configuration. In the future, these two approaches should likely be separated
 // into two distinct test suites.
 func TestProxyTracingOpenTelemetryProvider(t *testing.T) {
-	framework.NewTest(t).
-		Features("observability.telemetry.tracing.server").
-		Run(func(t framework.TestContext) {
-			appNsInst := tracing.GetAppNamespace()
-
-			// apply Telemetry resource with OTel provider
-			t.ConfigIstio().YAML(appNsInst.Name(), otelTracingCfg).ApplyOrFail(t)
-
-			// TODO fix tracing tests in multi-network https://github.com/istio/istio/issues/28890
-			for _, cluster := range t.Clusters().ByNetwork()[t.Clusters().Default().NetworkName()] {
-				cluster := cluster
-				t.NewSubTest(cluster.StableName()).Run(func(ctx framework.TestContext) {
-					retry.UntilSuccessOrFail(ctx, func() error {
-						err := tracing.SendTraffic(ctx, nil, cluster)
-						if err != nil {
-							return fmt.Errorf("cannot send traffic from cluster %s: %v", cluster.Name(), err)
-						}
-
-						// the OTel collector exports to Zipkin
-						traces, err := tracing.GetZipkinInstance().QueryTraces(300, "", "provider=otel")
-						t.Logf("got traces %v from %s", traces, cluster)
-						if err != nil {
-							return fmt.Errorf("cannot get traces from zipkin: %v", err)
-						}
-						if !tracing.VerifyOtelEchoTraces(ctx, appNsInst.Name(), cluster.Name(), traces) {
-							return errors.New("cannot find expected traces")
-						}
-						return nil
-					}, retry.Delay(3*time.Second), retry.Timeout(80*time.Second))
-				})
-			}
-		})
-}
+	testcases := []struct {
+		name            string
+		customAttribute string
+		cfgFile         string
+	}{
+		{
+			name:            "grpc exporter",
+			customAttribute: "provider=otel",
+			cfgFile:         otelTracingCfg,
+		},
+		{
+			name:            "http exporter",
+			customAttribute: "provider=otel-http",
+			cfgFile:         otelTracingHTTPCfg,
+		},
+		{
+			name:            "resource detectors",
+			customAttribute: "provider=otel-grpc-with-res-detectors",
+			cfgFile:         otelTracingResDetectorsCfg,
+		},
+	}
 
-func TestProxyTracingOpenTelemetryProviderHTTPExporter(t *testing.T) {
 	framework.NewTest(t).
 		Features("observability.telemetry.tracing.server").
-		Run(func(t framework.TestContext) {
+		Run(func(ctx framework.TestContext) {
 			appNsInst := tracing.GetAppNamespace()
 
-			// apply Telemetry resource with OTel provider, exporting via HTTP
-			t.ConfigIstio().YAML(appNsInst.Name(), otelTracingHTTPCfg).ApplyOrFail(t)
-
-			// TODO fix tracing tests in multi-network https://github.com/istio/istio/issues/28890
-			for _, cluster := range t.Clusters().ByNetwork()[t.Clusters().Default().NetworkName()] {
-				cluster := cluster
-				t.NewSubTest(cluster.StableName()).Run(func(ctx framework.TestContext) {
-					retry.UntilSuccessOrFail(ctx, func() error {
-						err := tracing.SendTraffic(ctx, nil, cluster)
-						if err != nil {
-							return fmt.Errorf("cannot send traffic from cluster %s: %v", cluster.Name(), err)
+			for _, tc := range testcases {
+				ctx.NewSubTest(tc.name).
+					Run(func(ctx framework.TestContext) {
+						// apply Telemetry resource with OTel provider
+						ctx.ConfigIstio().YAML(appNsInst.Name(), tc.cfgFile).ApplyOrFail(ctx)
+
+						// TODO fix tracing tests in multi-network https://github.com/istio/istio/issues/28890
+						for _, cluster := range ctx.Clusters().ByNetwork()[ctx.Clusters().Default().NetworkName()] {
+							cluster := cluster
+							ctx.NewSubTest(cluster.StableName()).Run(func(ctx framework.TestContext) {
+								retry.UntilSuccessOrFail(ctx, func() error {
+									err := tracing.SendTraffic(ctx, nil, cluster)
+									if err != nil {
+										return fmt.Errorf("cannot send traffic from cluster %s: %v", cluster.Name(), err)
+									}
+
+									// the OTel collector exports to Zipkin
+									traces, err := tracing.GetZipkinInstance().QueryTraces(300, "", tc.customAttribute)
+									t.Logf("got traces %v from %s", traces, cluster)
+									if err != nil {
+										return fmt.Errorf("cannot get traces from zipkin: %v", err)
+									}
+									if !tracing.VerifyOtelEchoTraces(ctx, appNsInst.Name(), cluster.Name(), traces) {
+										return errors.New("cannot find expected traces")
+									}
+									return nil
+								}, retry.Delay(3*time.Second), retry.Timeout(80*time.Second))
+							})
 						}
-
-						// the OTel collector exports to Zipkin
-						traces, err := tracing.GetZipkinInstance().QueryTraces(300, "", "provider=otel-http")
-						t.Logf("got traces %v from %s", traces, cluster)
-						if err != nil {
-							return fmt.Errorf("cannot get traces from zipkin: %v", err)
-						}
-						if !tracing.VerifyOtelEchoTraces(ctx, appNsInst.Name(), cluster.Name(), traces) {
-							return errors.New("cannot find expected traces")
-						}
-						return nil
-					}, retry.Delay(3*time.Second), retry.Timeout(80*time.Second))
-				})
+					})
 			}
 		})
 }
@@ -187,6 +182,13 @@ meshConfig:
         headers:
           - name: "some-header"
             value: "some-value"
+  - name: test-otel-res-detectors
+    opentelemetry:
+      service: opentelemetry-collector.istio-system.svc.cluster.local
+      port: 4317
+      resource_detectors:
+        environment: {}
+        dynatrace: {}
 `
 	cfg.Values["pilot.traceSampling"] = "100.0"
 	cfg.Values["global.proxy.tracer"] = "openCensusAgent"