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

Infer destination service resource adapt public api #1003

Merged
Show file tree
Hide file tree
Changes from 19 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
6 changes: 3 additions & 3 deletions model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,11 @@ type DestinationSpanContext struct {
// DestinationServiceSpanContext holds contextual information about a
// destination service.
type DestinationServiceSpanContext struct {
// Type holds the destination service type.
// Type holds the destination service type. Deprecated.
Type string `json:"type,omitempty"`

// Name holds the destination service name.
Name string `json:"name,omitempty"`
// Name holds the destination service name. Deprecated.
Name string `json:"name"`

// Resource identifies the destination service
// resource, e.g. a URI or message queue name.
Expand Down
41 changes: 39 additions & 2 deletions span.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (tx *Transaction) StartSpan(name, spanType string, parent *Span) *Span {
// span type, subtype, and action; a single dot separates span type and
// subtype, and the action will not be set.
func (tx *Transaction) StartSpanOptions(name, spanType string, opts SpanOptions) *Span {
if tx == nil {
if tx == nil || opts.parent.IsExitSpan() {
return newDroppedSpan()
}

Expand Down Expand Up @@ -101,6 +101,9 @@ func (tx *Transaction) StartSpanOptions(name, spanType string, opts SpanOptions)
span := tx.tracer.startSpan(name, spanType, transactionID, opts)
span.tx = tx
span.parent = opts.parent
if opts.ExitSpan {
span.exit = true
}

// Guard access to spansCreated, spansDropped, rand, and childrenTimer.
tx.TransactionData.mu.Lock()
Expand Down Expand Up @@ -143,7 +146,10 @@ func (tx *Transaction) StartSpanOptions(name, spanType string, opts SpanOptions)
// way will not have the "max spans" configuration applied, nor will they be
// considered in any transaction's span count.
func (t *Tracer) StartSpan(name, spanType string, transactionID SpanID, opts SpanOptions) *Span {
if opts.Parent.Trace.Validate() != nil || opts.Parent.Span.Validate() != nil || transactionID.Validate() != nil {
if opts.Parent.Trace.Validate() != nil ||
opts.Parent.Span.Validate() != nil ||
transactionID.Validate() != nil ||
opts.parent.IsExitSpan() {
return newDroppedSpan()
}
if !opts.Parent.Options.Recorded() {
Expand All @@ -166,6 +172,9 @@ func (t *Tracer) StartSpan(name, spanType string, transactionID SpanID, opts Spa
instrumentationConfig := t.instrumentationConfig()
span.stackFramesMinDuration = instrumentationConfig.spanFramesMinDuration
span.stackTraceLimit = instrumentationConfig.stackTraceLimit
if opts.ExitSpan {
span.exit = true
}

return span
}
Expand All @@ -179,6 +188,10 @@ type SpanOptions struct {
// will be generated and used instead.
SpanID SpanID

// Indicates whether a span is an exit span or not. All child spans
// will be noop spans.
ExitSpan bool

// parent, if non-nil, holds the parent span.
//
// This is only used if Parent is zero, and is only available to internal
Expand Down Expand Up @@ -239,6 +252,7 @@ type Span struct {
traceContext TraceContext
transactionID SpanID
parentID SpanID
exit bool

mu sync.RWMutex

Expand Down Expand Up @@ -295,6 +309,11 @@ func (s *Span) End() {
if s.ended() {
return
}
if s.exit && !s.Context.setDestinationServiceCalled {
// The span was created as an exit span, but the user did not
// manually set the destination.service.resource
s.setExitSpan()
}
if s.Duration < 0 {
s.Duration = time.Since(s.timestamp)
}
Expand Down Expand Up @@ -384,6 +403,24 @@ func (s *Span) ended() bool {
return s.SpanData == nil
}

func (s *Span) setExitSpan() {
Copy link
Member

Choose a reason for hiding this comment

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

setExitSpanDestinationService might be a bit clearer on the intent?

resource := s.Subtype
if resource == "" {
resource = s.Type
}
s.Context.SetDestinationService(DestinationServiceSpanContext{
Resource: resource,
})
}

// IsExitSpan returns true if the span is an exit span.
func (s *Span) IsExitSpan() bool {
if s == nil {
return false
}
return s.exit
}

// SpanData holds the details for a span, and is embedded inside Span.
// When a span is ended or discarded, its SpanData field will be set
// to nil.
Expand Down
40 changes: 40 additions & 0 deletions span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,46 @@ func TestSpanType(t *testing.T) {
check(spans[3], "type", "subtype", "action.figure")
}

func TestStartExitSpan(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.End()
})
require.Len(t, spans, 1)
// When the context's DestinationService is not explicitly set, ending
// the exit span will assign the value.
assert.Equal(t, spans[0].Context.Destination.Service.Resource, "type")

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

tx := tracer.StartTransaction("name", "type")
span := tx.StartSpanOptions("name", "type", apm.SpanOptions{ExitSpan: true})
assert.True(t, span.IsExitSpan())
// when the parent span is an exit span, any children should be noops.
span2 := tx.StartSpan("name", "type", span)
assert.True(t, span2.Dropped())
span.End()
span2.End()
// Spans should still be marked as an exit span after they've been
// ended.
assert.True(t, span.IsExitSpan())
}

func TestExitSpanDoesNotOverwriteDestinationServiceResource(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.SetDestinationService(apm.DestinationServiceSpanContext{
Resource: "my-custom-resource",
})
span.End()
})
require.Len(t, spans, 1)
assert.Equal(t, spans[0].Context.Destination.Service.Resource, "my-custom-resource")
}

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
9 changes: 7 additions & 2 deletions spancontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ type SpanContext struct {
databaseRowsAffected int64
database model.DatabaseSpanContext
http model.HTTPSpanContext

// If SetDestinationService has been called, we do not auto-set its
// resource value on span end.
setDestinationServiceCalled bool
}

// DatabaseSpanContext holds database span context.
Expand All @@ -58,7 +62,7 @@ type DatabaseSpanContext struct {
// DestinationServiceSpanContext holds destination service span span.
type DestinationServiceSpanContext struct {
// Name holds a name for the destination service, which may be used
// for grouping and labeling in service maps.
// for grouping and labeling in service maps. Deprecated.
Name string

// Resource holds an identifier for a destination service resource,
Expand Down Expand Up @@ -222,7 +226,8 @@ func (c *SpanContext) SetMessage(message MessageSpanContext) {
// Both service.Name and service.Resource are required. If either is empty,
// then SetDestinationService is a no-op.
func (c *SpanContext) SetDestinationService(service DestinationServiceSpanContext) {
if service.Name == "" || service.Resource == "" {
c.setDestinationServiceCalled = true
if service.Resource == "" {
return
}
c.destinationService.Name = truncateString(service.Name)
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
9 changes: 1 addition & 8 deletions spancontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,29 +132,23 @@ func TestSetDestinationService(t *testing.T) {
}

testcases := []testcase{{
name: "",
resource: "",
expectEmpty: true,
}, {
name: "",
resource: "nonempty",
expectEmpty: true,
expectEmpty: false,
}, {
name: "nonempty",
resource: "",
expectEmpty: true,
}, {
name: "nonempty",
resource: "nonempty",
}}

for _, tc := range testcases {
t.Run(fmt.Sprintf("%s_%s", tc.name, tc.resource), func(t *testing.T) {
_, spans, _ := apmtest.WithTransaction(func(ctx context.Context) {
span, _ := apm.StartSpan(ctx, "name", "span_type")
span.Context.SetDestinationAddress("testing.invalid", 123)
span.Context.SetDestinationService(apm.DestinationServiceSpanContext{
Name: tc.name,
Resource: tc.resource,
})
span.End()
Expand All @@ -164,7 +158,6 @@ func TestSetDestinationService(t *testing.T) {
assert.Nil(t, spans[0].Context.Destination.Service)
} else {
assert.Equal(t, &model.DestinationServiceSpanContext{
Name: tc.name,
Resource: tc.resource,
Type: "span_type",
}, spans[0].Context.Destination.Service)
Expand Down