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.

Sign up for GitHub

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

feat: add fallback service target type for exit spans #1294

Merged
merged 5 commits into from
Sep 14, 2022
Merged

feat: add fallback service target type for exit spans #1294

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
40c7c59
feat: add fallback service target type for exit spans
kruskall Aug 11, 2022
2799533
feat: allow users to disable inference of service target fields
kruskall Sep 13, 2022
6aadf35
Merge branch 'main' into fallback/service-target-type
kruskall Sep 14, 2022
5b1b329
docs: update test comment to reference the correct field
kruskall Sep 14, 2022
539c8ea
test: add service target infer testcase with explicit name
kruskall Sep 14, 2022
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
39 changes: 39 additions & 0 deletions span.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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{
kruskall marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
53 changes: 53 additions & 0 deletions span_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 DestinationService is not explicitly set, ending
kruskall marked this conversation as resolved.
Show resolved Hide resolved
// the exit span will assign the value.
assert.Equal(t, spans[0].Context.Service.Target.Type, "type")

tracer := apmtest.NewRecordingTracer()
defer tracer.Close()

Expand Down Expand Up @@ -1122,6 +1126,55 @@ 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 TestTracerStartSpanIDSpecified(t *testing.T) {
spanID := apm.SpanID{0, 1, 2, 3, 4, 5, 6, 7}
_, spans, _ := apmtest.WithTransaction(func(ctx context.Context) {
Expand Down
5 changes: 5 additions & 0 deletions spancontext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

To implement this part of the spec:

Agents SHOULD provide an API entrypoint to set the value of span.context.service.target.type and span.context.service.target.name, setting an empty or null value on both of those fields allows the user to discard the inferred values.

Should we check here if the specified name is empty, and then just set c.setServiceTargetCalled = true, set c.service.Target = nil, and return?

i.e.

// SetServiceTarget sets the service target info in the context.
//
// If service.Name is empty, then any previously set service target values
// will be cleared, and inference of service target values will be disabled.
func (c *SpanContext) SetServiceTarget(service ServiceTargetSpanContext) {
	c.setServiceTargetCalled = true
	if service.Name == "" {
		c.service.Target = nil
	} else {
		c.serviceTarget.Type = truncateString(service.Type)
		c.serviceTarget.Name = truncateString(service.Name)
		c.service.Target = nil
		c.model.Service = &c.service
	}
}

c.serviceTarget.Type = truncateString(service.Type)
c.serviceTarget.Name = truncateString(service.Name)
c.service.Target = &c.serviceTarget
Expand Down