From 1645e823d576f68a02695885c98ee060b5f7cdb4 Mon Sep 17 00:00:00 2001 From: kruskall <99559985+kruskall@users.noreply.github.com> Date: Wed, 14 Sep 2022 05:05:55 +0200 Subject: [PATCH] feat: add fallback service target type for exit spans (#1294) * feat: add fallback service target type for exit spans This is the final step for complete service target fields support. Exit spans will fallback to type and subtype similar to what the deprecated destination resource was doing. Assertions have been added to tests to verify the service target type is not overwritten if previously set and that the fallback mechanism is working properly. * feat: allow users to disable inference of service target fields if the user does not call SetServiceTarget at all, we'll use inferred values for both (where possible) if the user calls SetServiceTarget with an empty name, this will disable inference if the user calls SetServiceTarget with a non-empty name, but empty type, we'll use the specified name and infer the type * docs: update test comment to reference the correct field Co-authored-by: Andrew Wilkins * test: add service target infer testcase with explicit name Co-authored-by: Andrew Wilkins --- span.go | 39 +++++++++++++++++++++++++++ span_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ spancontext.go | 5 ++++ 3 files changed, 116 insertions(+) diff --git a/span.go b/span.go index 481bebefa..47586f97a 100644 --- a/span.go +++ b/span.go @@ -350,6 +350,11 @@ func (s *Span) End() { // manually set the destination.service.resource s.setExitSpanDestinationService() } + if s.exit { + // The span was created as an exit span, but the user did not + // manually set the service.target fields. + s.setExitSpanServiceTarget() + } if s.Duration < 0 { s.Duration = time.Since(s.timestamp) } @@ -490,6 +495,40 @@ func (s *Span) setExitSpanDestinationService() { }) } +func (s *Span) setExitSpanServiceTarget() { + fallbackType := s.Subtype + if fallbackType == "" { + fallbackType = s.Type + } + + // Service target fields explicitly provided. + if s.Context.setServiceTargetCalled { + // if the user calls SetServiceTarget with a non-empty name, but empty type, + // we'll use the specified name and infer the type + if s.Context.serviceTarget.Type == "" && s.Context.serviceTarget.Name != "" { + s.Context.SetServiceTarget(ServiceTargetSpanContext{ + Type: fallbackType, + Name: s.Context.serviceTarget.Name, + }) + } + return + } + + var fallbackName string + if s.Context.database.Type != "" { // database spans + fallbackName = s.Context.database.Instance + } else if s.Context.message.Queue != nil { // messaging spans + fallbackName = s.Context.message.Queue.Name + } else if s.Context.http.URL != nil { // http spans + fallbackName = s.Context.http.URL.Host + } + + s.Context.SetServiceTarget(ServiceTargetSpanContext{ + Type: fallbackType, + Name: fallbackName, + }) +} + // IsExitSpan returns true if the span is an exit span. func (s *Span) IsExitSpan() bool { if s == nil { diff --git a/span_test.go b/span_test.go index 5dcdcf855..a08ba13da 100644 --- a/span_test.go +++ b/span_test.go @@ -227,6 +227,10 @@ func TestStartExitSpan(t *testing.T) { // the exit span will assign the value. assert.Equal(t, spans[0].Context.Destination.Service.Resource, "type") + // When the context's ServiceTarget is not explicitly set, ending + // the exit span will assign the value. + assert.Equal(t, spans[0].Context.Service.Target.Type, "type") + tracer := apmtest.NewRecordingTracer() defer tracer.Close() @@ -1122,6 +1126,74 @@ func TestExitSpanDoesNotOverwriteDestinationServiceResource(t *testing.T) { assert.Equal(t, spans[0].Context.Destination.Service.Resource, "my-custom-resource") } +func TestExitSpanDoesNotOverwriteServiceTarget(t *testing.T) { + _, spans, _ := apmtest.WithTransaction(func(ctx context.Context) { + span, _ := apm.StartSpanOptions(ctx, "name", "type", apm.SpanOptions{ExitSpan: true}) + assert.True(t, span.IsExitSpan()) + span.Context.SetServiceTarget(apm.ServiceTargetSpanContext{ + Type: "my-custom-resource", + Name: "foo", + }) + span.Duration = 2 * time.Millisecond + span.End() + }) + require.Len(t, spans, 1) + assert.Equal(t, spans[0].Context.Service.Target.Type, "my-custom-resource") + assert.Equal(t, spans[0].Context.Service.Target.Name, "foo") +} + +func TestExitSpanDisableInferTarget(t *testing.T) { + _, spans, _ := apmtest.WithTransaction(func(ctx context.Context) { + span, _ := apm.StartSpanOptions(ctx, "name", "type", apm.SpanOptions{ExitSpan: true}) + assert.True(t, span.IsExitSpan()) + span.Context.SetDatabase(apm.DatabaseSpanContext{ + Type: "mysql", + Instance: "foo", + }) + span.Context.SetServiceTarget(apm.ServiceTargetSpanContext{}) + span.Duration = 2 * time.Millisecond + span.End() + }) + require.Len(t, spans, 1) + assert.Empty(t, spans[0].Context.Service.Target.Type) + assert.Empty(t, spans[0].Context.Service.Target.Name) +} + +func TestExitSpanInferTarget(t *testing.T) { + _, spans, _ := apmtest.WithTransaction(func(ctx context.Context) { + span, _ := apm.StartSpanOptions(ctx, "name", "type", apm.SpanOptions{ExitSpan: true}) + assert.True(t, span.IsExitSpan()) + span.Context.SetDatabase(apm.DatabaseSpanContext{ + Type: "mysql", + Instance: "foo", + }) + span.Duration = 2 * time.Millisecond + span.End() + }) + require.Len(t, spans, 1) + assert.Equal(t, spans[0].Context.Service.Target.Type, "type") + assert.Equal(t, spans[0].Context.Service.Target.Name, "foo") +} + +func TestExitSpanInferTargetWithName(t *testing.T) { + _, spans, _ := apmtest.WithTransaction(func(ctx context.Context) { + span, _ := apm.StartSpanOptions(ctx, "name", "type", apm.SpanOptions{ExitSpan: true}) + assert.True(t, span.IsExitSpan()) + span.Context.SetDatabase(apm.DatabaseSpanContext{ + Type: "mysql", + Instance: "foo", + }) + span.Context.SetServiceTarget(apm.ServiceTargetSpanContext{ + Name: "bar", + }) + span.Duration = 2 * time.Millisecond + span.End() + }) + require.Len(t, spans, 1) + assert.Equal(t, spans[0].Context.Service.Target.Type, "type") + assert.Equal(t, spans[0].Context.Service.Target.Name, "bar") +} + func TestTracerStartSpanIDSpecified(t *testing.T) { spanID := apm.SpanID{0, 1, 2, 3, 4, 5, 6, 7} _, spans, _ := apmtest.WithTransaction(func(ctx context.Context) { diff --git a/spancontext.go b/spancontext.go index f162ee040..61d6ad36b 100644 --- a/spancontext.go +++ b/spancontext.go @@ -44,6 +44,10 @@ type SpanContext struct { // If SetDestinationService has been called, we do not auto-set its // resource value on span end. setDestinationServiceCalled bool + + // If SetServiceTarget has been called, we do not auto-set its + // values on span end. + setServiceTargetCalled bool } // DatabaseSpanContext holds database span context. @@ -273,6 +277,7 @@ func (c *SpanContext) SetDestinationService(service DestinationServiceSpanContex // SetServiceTarget sets the service target info in the context. func (c *SpanContext) SetServiceTarget(service ServiceTargetSpanContext) { + c.setServiceTargetCalled = true c.serviceTarget.Type = truncateString(service.Type) c.serviceTarget.Name = truncateString(service.Name) c.service.Target = &c.serviceTarget