tracing: set datadog ServiceName (istio#41212)

* tracing: set datadog ServiceName * add UT * add release-notes * update release-notes
WillardHu · Oct 11, 2022 · 20e135f · 20e135f
1 parent 81b063a
commit 20e135f
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 19 deletions.
diff --git a/pilot/pkg/networking/core/v1alpha3/tracing.go b/pilot/pkg/networking/core/v1alpha3/tracing.go
@@ -105,7 +105,7 @@ func configureTracingFromSpec(
 
 	var routerFilterCtx *xdsfilters.RouterFilterContext
 	if spec.Provider != nil {
-		tcfg, rfCtx, err := configureFromProviderConfig(push, proxy.Metadata, spec.Provider)
+		tcfg, rfCtx, err := configureFromProviderConfig(push, proxy, spec.Provider)
 		if err != nil {
 			log.Warnf("Not able to configure requested tracing provider %q: %v", spec.Provider.Name, err)
 			return nil, nil
@@ -137,30 +137,38 @@ func configureTracingFromSpec(
 
 // TODO: follow-on work to enable bootstrapping of clusters for $(HOST_IP):PORT addresses.
 
-func configureFromProviderConfig(pushCtx *model.PushContext, meta *model.NodeMetadata,
+func configureFromProviderConfig(pushCtx *model.PushContext, proxy *model.Proxy,
 	providerCfg *meshconfig.MeshConfig_ExtensionProvider,
 ) (*hpb.HttpConnectionManager_Tracing, *xdsfilters.RouterFilterContext, error) {
 	tracing := &hpb.HttpConnectionManager_Tracing{}
-	var rfCtx *xdsfilters.RouterFilterContext
-	var err error
+	var (
+		rfCtx          *xdsfilters.RouterFilterContext
+		err            error
+		serviceCluster string
+		meta           = proxy.Metadata
+	)
+
+	if proxy.XdsNode != nil {
+		serviceCluster = proxy.XdsNode.Cluster
+	}
 
 	switch provider := providerCfg.Provider.(type) {
 	case *meshconfig.MeshConfig_ExtensionProvider_Zipkin:
 		tracing, err = buildHCMTracing(pushCtx, envoyZipkin, provider.Zipkin.GetService(),
-			provider.Zipkin.GetPort(), provider.Zipkin.GetMaxTagLength(), zipkinConfigGen)
+			provider.Zipkin.GetPort(), provider.Zipkin.GetMaxTagLength(), zipkinConfigGen, serviceCluster)
 	case *meshconfig.MeshConfig_ExtensionProvider_Datadog:
 		tracing, err = buildHCMTracing(pushCtx, envoyDatadog, provider.Datadog.GetService(),
-			provider.Datadog.GetPort(), provider.Datadog.GetMaxTagLength(), datadogConfigGen)
+			provider.Datadog.GetPort(), provider.Datadog.GetMaxTagLength(), datadogConfigGen, serviceCluster)
 	case *meshconfig.MeshConfig_ExtensionProvider_Lightstep:
 		// todo: read raw metadata and retrieve lightstep extensions (instead of relying on version)
 
 		// Envoy dropped support for Lightstep in v1.24+ (~istio 1.16+). So, we generate old-style configuration
 		// for lightstep for everything before 1.16, but OTel-based configuration for all other cases
-		useOTel := util.IsIstioVersionGE116(model.ParseIstioVersion(meta.IstioVersion))
+		useOTel := util.IsIstioVersionGE116(model.ParseIstioVersion(proxy.Metadata.IstioVersion))
 		if useOTel {
 			tracing, err = buildHCMTracing(pushCtx, envoyOpenTelemetry, provider.Lightstep.GetService(),
 				provider.Lightstep.GetPort(), provider.Lightstep.GetMaxTagLength(),
-				func(hostname, clusterName string) (*anypb.Any, error) {
+				func(_, hostname, clusterName string) (*anypb.Any, error) {
 					dc := &tracingcfg.OpenTelemetryConfig{
 						GrpcService: &envoy_config_core_v3.GrpcService{
 							TargetSpecifier: &envoy_config_core_v3.GrpcService_EnvoyGrpc_{
@@ -178,16 +186,16 @@ func configureFromProviderConfig(pushCtx *model.PushContext, meta *model.NodeMet
 						},
 					}
 					return anypb.New(dc)
-				})
+				}, serviceCluster)
 		} else {
 			tracing, err = buildHCMTracing(pushCtx, envoyLightstep, provider.Lightstep.GetService(), provider.Lightstep.GetPort(), provider.Lightstep.GetMaxTagLength(),
-				func(hostname, clusterName string) (*anypb.Any, error) {
+				func(_, hostname, clusterName string) (*anypb.Any, error) {
 					lc := &tracingcfg.LightstepConfig{
 						CollectorCluster: clusterName,
 						AccessTokenFile:  provider.Lightstep.GetAccessToken(),
 					}
 					return protoconv.MessageToAnyWithError(lc)
-				})
+				}, serviceCluster)
 		}
 
 	case *meshconfig.MeshConfig_ExtensionProvider_Opencensus:
@@ -206,7 +214,7 @@ func configureFromProviderConfig(pushCtx *model.PushContext, meta *model.NodeMet
 
 	case *meshconfig.MeshConfig_ExtensionProvider_Skywalking:
 		tracing, err = buildHCMTracing(pushCtx, envoySkywalking, provider.Skywalking.GetService(),
-			provider.Skywalking.GetPort(), 0, func(hostname, clusterName string) (*anypb.Any, error) {
+			provider.Skywalking.GetPort(), 0, func(_, hostname, clusterName string) (*anypb.Any, error) {
 				s := &tracingcfg.SkyWalkingConfig{
 					GrpcService: &envoy_config_core_v3.GrpcService{
 						TargetSpecifier: &envoy_config_core_v3.GrpcService_EnvoyGrpc_{
@@ -219,7 +227,7 @@ func configureFromProviderConfig(pushCtx *model.PushContext, meta *model.NodeMet
 				}
 
 				return protoconv.MessageToAnyWithError(s)
-			})
+			}, serviceCluster)
 
 		rfCtx = &xdsfilters.RouterFilterContext{
 			StartChildSpan: true,
@@ -322,15 +330,15 @@ func configureFromProviderConfig(pushCtx *model.PushContext, meta *model.NodeMet
 
 	case *meshconfig.MeshConfig_ExtensionProvider_Opentelemetry:
 		tracing, err = buildHCMTracing(pushCtx, envoyOpenTelemetry, provider.Opentelemetry.GetService(),
-			provider.Opentelemetry.GetPort(), provider.Opentelemetry.GetMaxTagLength(), otelConfigGen)
+			provider.Opentelemetry.GetPort(), provider.Opentelemetry.GetMaxTagLength(), otelConfigGen, serviceCluster)
 	}
 
 	return tracing, rfCtx, err
 }
 
-type typedConfigGenFromClusterFn func(hostname, clusterName string) (*anypb.Any, error)
+type typedConfigGenFromClusterFn func(serviceName, hostname, clusterName string) (*anypb.Any, error)
 
-func zipkinConfigGen(hostname, cluster string) (*anypb.Any, error) {
+func zipkinConfigGen(_, hostname, cluster string) (*anypb.Any, error) {
 	zc := &tracingcfg.ZipkinConfig{
 		CollectorCluster:         cluster,
 		CollectorEndpoint:        "/api/v2/spans",                   // envoy deprecated v1 support
@@ -342,14 +350,15 @@ func zipkinConfigGen(hostname, cluster string) (*anypb.Any, error) {
 	return protoconv.MessageToAnyWithError(zc)
 }
 
-func datadogConfigGen(hostname, cluster string) (*anypb.Any, error) {
+func datadogConfigGen(serviceName, _, cluster string) (*anypb.Any, error) {
 	dc := &tracingcfg.DatadogConfig{
 		CollectorCluster: cluster,
+		ServiceName:      serviceName,
 	}
 	return protoconv.MessageToAnyWithError(dc)
 }
 
-func otelConfigGen(hostname, cluster string) (*anypb.Any, error) {
+func otelConfigGen(_, hostname, cluster string) (*anypb.Any, error) {
 	dc := &tracingcfg.OpenTelemetryConfig{
 		GrpcService: &envoy_config_core_v3.GrpcService{
 			TargetSpecifier: &envoy_config_core_v3.GrpcService_EnvoyGrpc_{
@@ -367,6 +376,7 @@ type typedConfigGenFn func() (*anypb.Any, error)
 
 func buildHCMTracing(pushCtx *model.PushContext, provider, svc string, port, maxTagLen uint32,
 	anyFn typedConfigGenFromClusterFn,
+	serviceCluster string,
 ) (*hpb.HttpConnectionManager_Tracing, error) {
 	config := &hpb.HttpConnectionManager_Tracing{}
 
@@ -375,7 +385,7 @@ func buildHCMTracing(pushCtx *model.PushContext, provider, svc string, port, max
 		return config, fmt.Errorf("could not find cluster for tracing provider %q: %v", provider, err)
 	}
 
-	cfg, err := anyFn(hostname, cluster)
+	cfg, err := anyFn(serviceCluster, hostname, cluster)
 	if err != nil {
 		return config, fmt.Errorf("could not configure tracing provider %q: %v", provider, err)
 	}

diff --git a/pilot/pkg/networking/core/v1alpha3/tracing_test.go b/pilot/pkg/networking/core/v1alpha3/tracing_test.go
@@ -129,6 +129,14 @@ func TestConfigureTracing(t *testing.T) {
 			wantRfCtx:       nil,
 			wantReqIDExtCtx: &defaultUUIDExtensionCtx,
 		},
+		{
+			name:            "basic config (with datadog provider)",
+			inSpec:          fakeTracingSpec(fakeDatadog(), 99.999, false, true),
+			opts:            fakeOptsOnlyDatadogTelemetryAPI(),
+			want:            fakeTracingConfig(fakeDatadogProvider("fake-cluster", clusterName), 99.999, 256, append(defaultTracingTags(), fakeEnvTag)),
+			wantRfCtx:       nil,
+			wantReqIDExtCtx: &defaultUUIDExtensionCtx,
+		},
 		{
 			name:            "basic config (with skywalking provider)",
 			inSpec:          fakeTracingSpec(fakeSkywalking(), 99.999, false, false),
@@ -302,6 +310,49 @@ func fakeZipkin() *meshconfig.MeshConfig_ExtensionProvider {
 	}
 }
 
+func fakeDatadog() *meshconfig.MeshConfig_ExtensionProvider {
+	return &meshconfig.MeshConfig_ExtensionProvider{
+		Name: "datadog",
+		Provider: &meshconfig.MeshConfig_ExtensionProvider_Datadog{
+			Datadog: &meshconfig.MeshConfig_ExtensionProvider_DatadogTracingProvider{
+				Service:      "datadog",
+				Port:         8126,
+				MaxTagLength: 256,
+			},
+		},
+	}
+}
+
+func fakeOptsOnlyDatadogTelemetryAPI() buildListenerOpts {
+	var opts buildListenerOpts
+	opts.push = &model.PushContext{
+		Mesh: &meshconfig.MeshConfig{
+			ExtensionProviders: []*meshconfig.MeshConfig_ExtensionProvider{
+				{
+					Name: "datadog",
+					Provider: &meshconfig.MeshConfig_ExtensionProvider_Datadog{
+						Datadog: &meshconfig.MeshConfig_ExtensionProvider_DatadogTracingProvider{
+							Service:      "datadog",
+							Port:         8126,
+							MaxTagLength: 256,
+						},
+					},
+				},
+			},
+		},
+	}
+	opts.proxy = &model.Proxy{
+		Metadata: &model.NodeMetadata{
+			ProxyConfig: &model.NodeMetaProxyConfig{},
+		},
+		XdsNode: &envoy_config_core_v3.Node{
+			Cluster: "fake-cluster",
+		},
+	}
+
+	return opts
+}
+
 func fakeOptsMeshAndTelemetryAPI(enableTracing bool) buildListenerOpts {
 	var opts buildListenerOpts
 	opts.push = &model.PushContext{
@@ -544,3 +595,15 @@ func fakeSkywalkingProvider(expectClusterName, expectAuthority string) *tracingc
 		ConfigType: &tracingcfg.Tracing_Http_TypedConfig{TypedConfig: fakeSkywalkingAny},
 	}
 }
+
+func fakeDatadogProvider(expectServcieName, expectClusterName string) *tracingcfg.Tracing_Http {
+	fakeDatadogProviderConfig := &tracingcfg.DatadogConfig{
+		CollectorCluster: expectClusterName,
+		ServiceName:      expectServcieName,
+	}
+	fakeAny := protoconv.MessageToAny(fakeDatadogProviderConfig)
+	return &tracingcfg.Tracing_Http{
+		Name:       envoyDatadog,
+		ConfigType: &tracingcfg.Tracing_Http_TypedConfig{TypedConfig: fakeAny},
+	}
+}
diff --git a/releasenotes/notes/41212.yaml b/releasenotes/notes/41212.yaml
@@ -0,0 +1,9 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: telemetry
+
+issue:
+- 38573
+releaseNotes:
+- |
+  **Fixed** an issue with missing `service_name` in Telemetry API when configuring datadog tracing provider.