Skip to content

Commit

Permalink
feat!(datadogexporter): added peer.service priority instead service.n…
Browse files Browse the repository at this point in the history
…ame (#2817)

peer.service is a user-defined name for a service, which is conceptually the same as a service name.
We should use it when defined and not service.name

That behavior is because datadog need to "separate" the service:
Before:
![Screenshot from 2021-03-22 12-40-09](https://user-images.githubusercontent.com/1574240/112017047-03875000-8b0c-11eb-9265-c8f3763f48f0.png)
After:
![Screenshot from 2021-03-22 12-41-18](https://user-images.githubusercontent.com/1574240/112017065-06824080-8b0c-11eb-8dce-6e8b048688bf.png)

You can see, when we don't prioritize the `peer.service`, the whole operation is showed in the **main** service (GET request and SETEX operation). After we change, the exporter could create the separation between operations and give the correct attribution 

The [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/a44d863edcdef63b0adce7b47df001933b7a158a/specification/trace/semantic_conventions/span-general.md#general-remote-service-attributes) says:

>  peer.service - The service.name of the remote service. SHOULD be equal to the actual service.name resource attribute of the remote service if any.


So I think makes sense to change that behavior.

I created a really simple implementation, so any feedback would be useful
  • Loading branch information
angeliski authored and pmatyjasek-sumo committed Apr 28, 2021
1 parent 626e70f commit e52ac29
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 0 deletions.
5 changes: 5 additions & 0 deletions exporter/datadogexporter/translate_traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ func spanToDatadogSpan(s pdata.Span,
}
}

// peer.service should always be prioritized for service names when set because it is what the user decided.
if peerService, ok := tags[conventions.AttributePeerService]; ok {
serviceName = peerService
}

normalizedServiceName := utils.NormalizeServiceName(serviceName)

// canonical resource attribute version should override others if it exists
Expand Down
74 changes: 74 additions & 0 deletions exporter/datadogexporter/translate_traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,80 @@ func TestTracesTranslationInvalidService(t *testing.T) {
assert.Equal(t, "alt-service", datadogPayloadStartWithInvalidService.Traces[0].Spans[0].Service)
}

// ensure that the datadog span uses the peer.name instead service.name when provided
func TestTracesTranslationServicePeerName(t *testing.T) {
hostname := "testhostname"
calculator := newSublayerCalculator()

// generate mock trace, span and parent span ids
mockTraceID := [16]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}
mockSpanID := [8]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8}
mockParentSpanID := [8]byte{0xEF, 0xEE, 0xED, 0xEC, 0xEB, 0xEA, 0xE9, 0xE8}
mockEndTime := time.Now().Round(time.Second)

// create mock resource span data
// set shouldError and resourceServiceandEnv to false to test defaut behavior
rs := NewResourceSpansData(mockTraceID, mockSpanID, mockParentSpanID, pdata.StatusCodeUnset, false, mockEndTime)

span := rs.InstrumentationLibrarySpans().At(0).Spans().At(0)
span.Attributes().InsertString(conventions.AttributePeerService, "my_peer_service_name")

// translate mocks to datadog traces
datadogPayload := resourceSpansToDatadogSpans(rs, calculator, hostname, &config.Config{})
// ensure we return the correct type
assert.IsType(t, pb.TracePayload{}, datadogPayload)

// ensure hostname arg is respected
assert.Equal(t, hostname, datadogPayload.HostName)
assert.Equal(t, 1, len(datadogPayload.Traces))

// ensure trace id gets translated to uint64 correctly
assert.Equal(t, decodeAPMTraceID(mockTraceID), datadogPayload.Traces[0].TraceID)

// ensure the correct number of spans are expected
assert.Equal(t, 1, len(datadogPayload.Traces[0].Spans))

// ensure span's trace id matches payload trace id
assert.Equal(t, datadogPayload.Traces[0].TraceID, datadogPayload.Traces[0].Spans[0].TraceID)

// ensure span's spanId and parentSpanId are set correctly
assert.Equal(t, decodeAPMSpanID(mockSpanID), datadogPayload.Traces[0].Spans[0].SpanID)
assert.Equal(t, decodeAPMSpanID(mockParentSpanID), datadogPayload.Traces[0].Spans[0].ParentID)

// ensure that span.resource defaults to otlp span.name
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[conventions.InstrumentationLibraryName], 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)

// ensure that span.meta and span.metrics pick up attibutes, instrumentation ibrary and resource attribs
assert.Equal(t, 11, len(datadogPayload.Traces[0].Spans[0].Meta))
assert.Equal(t, 1, len(datadogPayload.Traces[0].Spans[0].Metrics))

// ensure that span error is based on otlp span status
assert.Equal(t, int32(0), datadogPayload.Traces[0].Spans[0].Error)

// ensure that span meta also inccludes correctly sets resource attributes
assert.Equal(t, "kube-system", datadogPayload.Traces[0].Spans[0].Meta["namespace"])

// ensure that span service name gives resource service.name priority
assert.Equal(t, "my_peer_service_name", datadogPayload.Traces[0].Spans[0].Service)

// ensure a duration and start time are calculated
assert.NotNil(t, datadogPayload.Traces[0].Spans[0].Start)
assert.NotNil(t, datadogPayload.Traces[0].Spans[0].Duration)

pdataMockEndTime := pdata.TimestampFromTime(mockEndTime)
pdataMockStartTime := pdata.TimestampFromTime(mockEndTime.Add(-90 * time.Second))
mockEventsString := fmt.Sprintf("[{\"attributes\":{},\"name\":\"start\",\"time\":%d},{\"attributes\":{\"flag\":false},\"name\":\"end\",\"time\":%d}]", pdataMockStartTime, pdataMockEndTime)

// ensure that events tag is set if span events exist and contains structured json fields
assert.Equal(t, mockEventsString, datadogPayload.Traces[0].Spans[0].Meta["events"])
}

// ensure that datadog span resource naming uses http method+route when available
func TestSpanResourceTranslation(t *testing.T) {
span := pdata.NewSpan()
Expand Down

0 comments on commit e52ac29

Please sign in to comment.