Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/datadog]: update resource naming and span naming #1861

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions exporter/datadogexporter/translate_traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/hex"
"fmt"
"strconv"
"strings"

"github.com/DataDog/datadog-agent/pkg/trace/exportable/pb"
"go.opentelemetry.io/collector/consumer/pdata"
Expand All @@ -41,6 +42,7 @@ const (
httpKind string = "http"
webKind string = "web"
customKind string = "custom"
grpcPath string = "grpc.path"
)

// converts Traces into an array of datadog trace payloads grouped by env
Expand Down Expand Up @@ -302,8 +304,8 @@ func aggregateSpanTags(span pdata.Span, datadogTags map[string]string) map[strin
return spanTags
}

// 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:
Expand Down Expand Up @@ -380,33 +382,47 @@ 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, strings.TrimPrefix(s.Kind().String(), "SPAN_KIND_")))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: strings.TrimPrefix(<a string>, "SPAN_KIND_") is being used in a lot of places, maybe it would make sense to move this to a utility function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense will do

}

if ilnOtelCur, okOtelCur := datadogTags[currentILNameTag]; okOtelCur {
return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtelCur, s.Kind()))
return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", ilnOtelCur, strings.TrimPrefix(s.Kind().String(), "SPAN_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, strings.TrimPrefix(s.Kind().String(), "SPAN_KIND_")))
}

return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", "opentelemetry", s.Kind()))
return utils.NormalizeSpanName(fmt.Sprintf("%s.%s", "opentelemetry", strings.TrimPrefix(s.Kind().String(), "SPAN_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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a test case where messaging.operation is set but not messaging.destination to test this code path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 yup will add test

}

return s.Name()
}

Expand Down
58 changes: 52 additions & 6 deletions exporter/datadogexporter/translate_traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,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)
Expand Down Expand Up @@ -445,6 +445,52 @@ func TestSpanResourceTranslation(t *testing.T) {
assert.Equal(t, "Default Name", resourceNameDefault)
}

// 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 the datadog span name uses IL name +kind whenn available and falls back to opetelemetry + kind
func TestSpanNameTranslation(t *testing.T) {
span := pdata.NewSpan()
Expand Down Expand Up @@ -477,11 +523,11 @@ func TestSpanNameTranslation(t *testing.T) {
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)
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)
}

// ensure that the datadog span type gets mapped from span kind
Expand Down
3 changes: 2 additions & 1 deletion exporter/datadogexporter/utils/trace_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,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 == '.':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably add a tag with a digit or . in a test case to test this branch, now that we don't handle - here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 yup will add test

buf.WriteRune(c)
lastWasUnderscore = false
// convert anything else to underscores (including underscores), but only allow one in a row.
Expand Down