From 0e17f0e5ce5d932d47556c63db8550d679351792 Mon Sep 17 00:00:00 2001 From: Eric Mustin Date: Mon, 4 Jan 2021 16:06:45 -0500 Subject: [PATCH] [exporter/datadog]: update resource naming and span naming (#1861) This PR improves datadog trace translation in a few areas 1. Datadog Resource naming has improved to add more rich names for messaging spans (such a google pub sub) and grpc spans. GRPC spans will have resources sorted by `grpc.path` if possible, or ` `, and messaging spans will have resources sorted by ` `. 2. Datadog span operation names will be shortened to only display `.`. previously they were `.span_kind_`, which was not helpful extra info and was not displayed well in UI. 3. Datadog span operation names will coerce `-` to `_`. Previously the `-` was being coerced to `_` in some places in UI but not others, leading to an unusual experience, so just forcing it to `_` regardless will resolve this issue. **Testing:** Updated Unit Tests --- exporter/datadogexporter/translate_traces.go | 37 ++++- .../datadogexporter/translate_traces_test.go | 135 ++++++++++++++++-- .../datadogexporter/utils/trace_helpers.go | 10 +- 3 files changed, 166 insertions(+), 16 deletions(-) diff --git a/exporter/datadogexporter/translate_traces.go b/exporter/datadogexporter/translate_traces.go index 253f9e1d1b4f..6382bea7952f 100644 --- a/exporter/datadogexporter/translate_traces.go +++ b/exporter/datadogexporter/translate_traces.go @@ -42,7 +42,7 @@ const ( httpKind string = "http" webKind string = "web" customKind string = "custom" - + grpcPath string = "grpc.path" // tagContainersTags specifies the name of the tag which holds key/value // pairs representing information about the container (Docker, EC2, etc). tagContainersTags = "_dd.tags.container" @@ -325,8 +325,8 @@ func buildDatadogContainerTags(spanTags map[string]string) string { return strings.TrimSuffix(b.String(), ",") } -// TODO: this seems to resolve to SPAN_KIND_UNSPECIFIED in e2e using jaeger receiver -// even though span.kind is getting set at the app tracer level. Need to file bug ticket +// TODO: some clients send SPAN_KIND_UNSPECIFIED for valid kinds +// we also need a more formal mapping for cache and db types func spanKindToDatadogType(kind pdata.SpanKind) string { switch kind { case pdata.SpanKindCLIENT: @@ -403,33 +403,56 @@ func getDatadogSpanName(s pdata.Span, datadogTags map[string]string) string { // The spec has changed over time and, depending on the original exporter, IL Name could represented a few different ways // so we try to account for all permutations if ilnOtlp, okOtlp := datadogTags[tracetranslator.TagInstrumentationName]; okOtlp { - return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtlp, s.Kind())) + return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtlp, utils.NormalizeSpanKind(s.Kind()))) } if ilnOtelCur, okOtelCur := datadogTags[currentILNameTag]; okOtelCur { - return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtelCur, s.Kind())) + return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtelCur, utils.NormalizeSpanKind(s.Kind()))) } if ilnOtelOld, okOtelOld := datadogTags[oldILNameTag]; okOtelOld { - return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtelOld, s.Kind())) + return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtelOld, utils.NormalizeSpanKind(s.Kind()))) } - return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", "opentelemetry", s.Kind())) + return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", "opentelemetry", utils.NormalizeSpanKind(s.Kind()))) } func getDatadogResourceName(s pdata.Span, datadogTags map[string]string) string { // largely a port of logic here // https://github.com/open-telemetry/opentelemetry-python/blob/b2559409b2bf82e693f3e68ed890dd7fd1fa8eae/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/exporter.py#L229 // Get span resource name by checking for existence http.method + http.route 'GET /api' + // Also check grpc path as fallback for http requests // backing off to just http.method, and then span.name if unrelated to http if method, methodOk := datadogTags[conventions.AttributeHTTPMethod]; methodOk { if route, routeOk := datadogTags[conventions.AttributeHTTPRoute]; routeOk { return fmt.Sprintf("%s %s", method, route) } + if grpcRoute, grpcRouteOk := datadogTags[grpcPath]; grpcRouteOk { + return fmt.Sprintf("%s %s", method, grpcRoute) + } + return method } + //add resource conventions for messaging queues, operaton + destination + if msgOperation, msgOperationOk := datadogTags[conventions.AttributeMessagingOperation]; msgOperationOk { + if destination, destinationOk := datadogTags[conventions.AttributeMessagingDestination]; destinationOk { + return fmt.Sprintf("%s %s", msgOperation, destination) + } + + return msgOperation + } + + // add resource convention for rpc services , method+service, fallback to just method if no service attribute + if rpcMethod, rpcMethodOk := datadogTags[conventions.AttributeRPCMethod]; rpcMethodOk { + if rpcService, rpcServiceOk := datadogTags[conventions.AttributeRPCService]; rpcServiceOk { + return fmt.Sprintf("%s %s", rpcMethod, rpcService) + } + + return rpcMethod + } + return s.Name() } diff --git a/exporter/datadogexporter/translate_traces_test.go b/exporter/datadogexporter/translate_traces_test.go index d4471e47194a..aef496828d35 100644 --- a/exporter/datadogexporter/translate_traces_test.go +++ b/exporter/datadogexporter/translate_traces_test.go @@ -221,7 +221,7 @@ func TestBasicTracesTranslation(t *testing.T) { assert.Equal(t, "End-To-End Here", datadogPayload.Traces[0].Spans[0].Resource) // ensure that span.name defaults to string representing instrumentation library if present - assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", datadogPayload.Traces[0].Spans[0].Meta[tracetranslator.TagInstrumentationName], pdata.SpanKindSERVER)), datadogPayload.Traces[0].Spans[0].Name) + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", datadogPayload.Traces[0].Spans[0].Meta[tracetranslator.TagInstrumentationName], strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), datadogPayload.Traces[0].Spans[0].Name) // ensure that span.type is based on otlp span.kind assert.Equal(t, "web", datadogPayload.Traces[0].Spans[0].Type) @@ -451,7 +451,120 @@ func TestSpanResourceTranslation(t *testing.T) { assert.Equal(t, "Default Name", resourceNameDefault) } -// ensure that the datadog span name uses IL name +kind whenn available and falls back to opetelemetry + kind +// ensure that datadog span resource naming uses http method+ grpc path when available +func TestSpanResourceTranslationGRPC(t *testing.T) { + span := pdata.NewSpan() + span.SetKind(pdata.SpanKindSERVER) + span.SetName("Default Name") + + ddHTTPTags := map[string]string{ + "http.method": "POST", + "grpc.path": "/api", + } + + ddNotHTTPTags := map[string]string{ + "other": "GET", + } + + resourceNameHTTP := getDatadogResourceName(span, ddHTTPTags) + + resourceNameDefault := getDatadogResourceName(span, ddNotHTTPTags) + + assert.Equal(t, "POST /api", resourceNameHTTP) + assert.Equal(t, "Default Name", resourceNameDefault) +} + +// ensure that datadog span resource naming uses messaging operation+destination when available +func TestSpanResourceTranslationMessaging(t *testing.T) { + span := pdata.NewSpan() + span.SetKind(pdata.SpanKindSERVER) + span.SetName("Default Name") + + ddHTTPTags := map[string]string{ + "messaging.operation": "receive", + "messaging.destination": "example.topic", + } + + ddNotHTTPTags := map[string]string{ + "other": "GET", + } + + resourceNameHTTP := getDatadogResourceName(span, ddHTTPTags) + + resourceNameDefault := getDatadogResourceName(span, ddNotHTTPTags) + + assert.Equal(t, "receive example.topic", resourceNameHTTP) + assert.Equal(t, "Default Name", resourceNameDefault) +} + +// ensure that datadog span resource naming uses messaging operation even when destination is not available +func TestSpanResourceTranslationMessagingFallback(t *testing.T) { + span := pdata.NewSpan() + span.SetKind(pdata.SpanKindSERVER) + span.SetName("Default Name") + + ddHTTPTags := map[string]string{ + "messaging.operation": "receive", + } + + ddNotHTTPTags := map[string]string{ + "other": "GET", + } + + resourceNameHTTP := getDatadogResourceName(span, ddHTTPTags) + + resourceNameDefault := getDatadogResourceName(span, ddNotHTTPTags) + + assert.Equal(t, "receive", resourceNameHTTP) + assert.Equal(t, "Default Name", resourceNameDefault) +} + +// ensure that datadog span resource naming uses rpc method + rpc service when available +func TestSpanResourceTranslationRpc(t *testing.T) { + span := pdata.NewSpan() + span.SetKind(pdata.SpanKindSERVER) + span.SetName("Default Name") + + ddHTTPTags := map[string]string{ + "rpc.method": "example_method", + "rpc.service": "example_service", + } + + ddNotHTTPTags := map[string]string{ + "other": "GET", + } + + resourceNameHTTP := getDatadogResourceName(span, ddHTTPTags) + + resourceNameDefault := getDatadogResourceName(span, ddNotHTTPTags) + + assert.Equal(t, "example_method example_service", resourceNameHTTP) + assert.Equal(t, "Default Name", resourceNameDefault) +} + +// ensure that datadog span resource naming uses rpc method even when rpc service is not available +func TestSpanResourceTranslationRpcFallback(t *testing.T) { + span := pdata.NewSpan() + span.SetKind(pdata.SpanKindSERVER) + span.SetName("Default Name") + + ddHTTPTags := map[string]string{ + "rpc.method": "example_method", + } + + ddNotHTTPTags := map[string]string{ + "other": "GET", + } + + resourceNameHTTP := getDatadogResourceName(span, ddHTTPTags) + + resourceNameDefault := getDatadogResourceName(span, ddNotHTTPTags) + + assert.Equal(t, "example_method", resourceNameHTTP) + assert.Equal(t, "Default Name", resourceNameDefault) +} + +// ensure that the datadog span name uses IL name +kind when available and falls back to opetelemetry + kind func TestSpanNameTranslation(t *testing.T) { span := pdata.NewSpan() span.SetName("Default Name") @@ -477,17 +590,23 @@ func TestSpanNameTranslation(t *testing.T) { "otel.library.name": "@unusual/\\::value", } + ddIlTagsHyphen := map[string]string{ + "otel.library.name": "hyphenated-value", + } + spanNameIl := getDatadogSpanName(span, ddIlTags) spanNameDefault := getDatadogSpanName(span, ddNoIlTags) spanNameOld := getDatadogSpanName(span, ddIlTagsOld) spanNameCur := getDatadogSpanName(span, ddIlTagsCur) spanNameUnusual := getDatadogSpanName(span, ddIlTagsUnusual) - - assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "il_name", pdata.SpanKindSERVER)), spanNameIl) - assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "opentelemetry", pdata.SpanKindSERVER)), spanNameDefault) - assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "old_value", pdata.SpanKindSERVER)), spanNameOld) - assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "current_value", pdata.SpanKindSERVER)), spanNameCur) - assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "unusual_value", pdata.SpanKindSERVER)), spanNameUnusual) + spanNameHyphen := getDatadogSpanName(span, ddIlTagsHyphen) + + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "il_name", strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), spanNameIl) + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "opentelemetry", strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), spanNameDefault) + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "old_value", strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), spanNameOld) + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "current_value", strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), spanNameCur) + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "unusual_value", strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), spanNameUnusual) + assert.Equal(t, strings.ToLower(fmt.Sprintf("%s.%s", "hyphenated_value", strings.TrimPrefix(pdata.SpanKindSERVER.String(), "SPAN_KIND_"))), spanNameHyphen) } // ensure that the datadog span type gets mapped from span kind diff --git a/exporter/datadogexporter/utils/trace_helpers.go b/exporter/datadogexporter/utils/trace_helpers.go index 83c0cd5e7a27..ba1adf848d09 100644 --- a/exporter/datadogexporter/utils/trace_helpers.go +++ b/exporter/datadogexporter/utils/trace_helpers.go @@ -17,6 +17,8 @@ package utils import ( "strings" "unicode" + + "go.opentelemetry.io/collector/consumer/pdata" ) // constants for tags @@ -80,7 +82,8 @@ func NormalizeSpanName(tag string) string { case buf.Len() == 0: continue // handle valid characters that can't start the string. - case unicode.IsDigit(c) || c == '.' || c == '-': + // '-' creates issues in the UI so we skip it + case unicode.IsDigit(c) || c == '.': buf.WriteRune(c) lastWasUnderscore = false // convert anything else to underscores (including underscores), but only allow one in a row. @@ -99,3 +102,8 @@ func NormalizeSpanName(tag string) string { return s } + +// NormalizeSpanKind returns a span kind with the SPAN_KIND prefix trimmed off +func NormalizeSpanKind(kind pdata.SpanKind) string { + return strings.TrimPrefix(kind.String(), "SPAN_KIND_") +}