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
Changes from 16 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
@@ -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.
37 changes: 35 additions & 2 deletions span.go
Original file line number Diff line number Diff line change
@@ -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()
}

@@ -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.setExitSpan(name, spanType)
}

// Guard access to spansCreated, spansDropped, rand, and childrenTimer.
tx.TransactionData.mu.Lock()
@@ -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() {
@@ -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.setExitSpan(name, spanType)
}

return span
}
@@ -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
@@ -239,6 +252,7 @@ type Span struct {
traceContext TraceContext
transactionID SpanID
parentID SpanID
exit bool

mu sync.RWMutex

@@ -384,6 +398,25 @@ func (s *Span) ended() bool {
return s.SpanData == nil
}

func (s *Span) setExitSpan(name, spanType string) {
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the spec, I believe the resource is meant to be set to subtype || type.

Subtype and Type can be set after starting a span, so I think we need to do that when ending the span. What we can do is keep track of whether SpanContext.SetDestinationService has been called (even with empty strings), and if it is never called for an exit span, then do this implicit call when ending the exit span.

resource := spanType
if resource == "" {
resource = name
}
s.Context.SetDestinationService(DestinationServiceSpanContext{
Resource: resource,
})
s.exit = true
}

// 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.
25 changes: 25 additions & 0 deletions span_test.go
Original file line number Diff line number Diff line change
@@ -163,6 +163,31 @@ 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)
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 TestTracerStartSpanIDSpecified(t *testing.T) {
spanID := apm.SpanID{0, 1, 2, 3, 4, 5, 6, 7}
_, spans, _ := apmtest.WithTransaction(func(ctx context.Context) {
4 changes: 2 additions & 2 deletions spancontext.go
Original file line number Diff line number Diff line change
@@ -58,7 +58,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,
@@ -222,7 +222,7 @@ 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 == "" {
if service.Resource == "" {
return
}
c.destinationService.Name = truncateString(service.Name)
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 1 addition & 8 deletions spancontext_test.go
Original file line number Diff line number Diff line change
@@ -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()
@@ -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)