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 all commits
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
37 changes: 30 additions & 7 deletions exporter/datadogexporter/translate_traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
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

}

// 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()
}

Expand Down
135 changes: 127 additions & 8 deletions exporter/datadogexporter/translate_traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down
10 changes: 9 additions & 1 deletion exporter/datadogexporter/utils/trace_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package utils
import (
"strings"
"unicode"

"go.opentelemetry.io/collector/consumer/pdata"
)

// constants for tags
Expand Down Expand Up @@ -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 == '.':
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 All @@ -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_")
}