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 4 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
11 changes: 11 additions & 0 deletions gocontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@ func StartSpan(ctx context.Context, name, spanType string) (*Span, context.Conte
return StartSpanOptions(ctx, name, spanType, SpanOptions{})
}

// StartExitSpan starts an exit span. If the provided context already contains
// an exit span, this function returns a dropped span.
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved
func StartExitSpan(ctx context.Context, name, spanType string) (*Span, context.Context) {
span, ctx := StartSpanOptions(ctx, name, spanType, SpanOptions{})
if span.Dropped() {
return span, ctx
}
span.setExitSpan(name, spanType)
return span, ctx
}

// StartSpanOptions starts and returns a new Span within the sampled transaction
// and parent span in the context, if any. If the span isn't dropped, it will be
// stored in the resulting context.
Expand Down
4 changes: 2 additions & 2 deletions model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,10 @@ 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 holds the destination service name. Deprecated.
Name string `json:"name,omitempty"`
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved

// Resource identifies the destination service
Expand Down
1 change: 0 additions & 1 deletion module/apmawssdkgo/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ func send(req *request.Request) {
span.Action = req.Operation.Name

span.Context.SetDestinationService(apm.DestinationServiceSpanContext{
Name: spanSubtype,
Resource: svc.resource(),
})

Expand Down
45 changes: 43 additions & 2 deletions span.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"sync"
"time"

"go.elastic.co/apm/model"
"go.elastic.co/apm/stacktrace"
)

Expand Down Expand Up @@ -54,6 +55,17 @@ func (tx *Transaction) StartSpan(name, spanType string, parent *Span) *Span {
})
}

// StartExitSpan starts an exit span. If the provided parent is already an exit
// span, this function returns a dropped span.
func (tx *Transaction) StartExitSpan(name, spanType string, parent *Span) *Span {
span := tx.StartSpan(name, spanType, parent)
if span.Dropped() {
return span
}
span.setExitSpan(name, spanType)
return span
}

// StartSpanOptions starts and returns a new Span within the transaction,
// with the specified name, type, and options.
//
Expand All @@ -64,7 +76,8 @@ 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() {
// TODO: Is this the equivalent of a noop span?
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved
return newDroppedSpan()
}

Expand Down Expand Up @@ -143,7 +156,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 Down Expand Up @@ -384,6 +400,30 @@ 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.exit = true
s.Context.SetDestinationService(DestinationServiceSpanContext{
Resource: resource,
})
}

// IsExitSpan returns true if the span is an exit span.
func (s *Span) IsExitSpan() bool {
if s == nil || s.SpanData == nil {
return false
}
ctx := s.Context
return s.exit ||
ctx.destination != model.DestinationSpanContext{} ||
ctx.database != model.DatabaseSpanContext{} ||
ctx.message != model.MessageSpanContext{} ||
ctx.http != model.HTTPSpanContext{}
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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 All @@ -392,6 +432,7 @@ type SpanData struct {
stackTraceLimit int
timestamp time.Time
childrenTimer childrenTimer
exit bool
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved

// Name holds the span name, initialized with the value passed to StartSpan.
Name string
Expand Down
66 changes: 66 additions & 0 deletions span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,72 @@ 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.StartExitSpan(ctx, "name", "type")
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.StartExitSpan("name", "type", nil)
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()

// when a span has the correct fields set to qualify as an exit span,
// it should be considered as such.
span = tx.StartSpan("name", "type", nil)
assert.False(t, span.IsExitSpan())
span.Context.SetDestinationService(apm.DestinationServiceSpanContext{
Resource: "my-resource",
})
assert.True(t, span.IsExitSpan())
span2 = tx.StartSpan("name", "type", span)
assert.True(t, span2.Dropped())
span.End()
span2.End()

span = tx.StartSpan("name", "type", nil)
assert.False(t, span.IsExitSpan())
span.Context.SetMessage(apm.MessageSpanContext{
QueueName: "my-queue",
})
assert.True(t, span.IsExitSpan())
span2 = tx.StartSpan("name", "type", span)
assert.True(t, span2.Dropped())
span.End()
span2.End()

span = tx.StartSpan("name", "type", nil)
assert.False(t, span.IsExitSpan())
span.Context.SetDatabase(apm.DatabaseSpanContext{
Instance: "my-instance",
})
assert.True(t, span.IsExitSpan())
span2 = tx.StartSpan("name", "type", span)
assert.True(t, span2.Dropped())
span.End()
span2.End()

span = tx.StartSpan("name", "type", nil)
assert.False(t, span.IsExitSpan())
span.Context.SetHTTPStatusCode(200)
assert.True(t, span.IsExitSpan())
span2 = tx.StartSpan("name", "type", span)
assert.True(t, span2.Dropped())
span.End()
span2.End()
}

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
8 changes: 1 addition & 7 deletions spancontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ 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.
Name string
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved

// Resource holds an identifier for a destination service resource,
// such as a message queue.
Resource string
Expand Down Expand Up @@ -178,7 +174,6 @@ func (c *SpanContext) SetHTTPRequest(req *http.Request) {
}
}
c.SetDestinationService(DestinationServiceSpanContext{
Name: destinationServiceURL.String(),
Resource: destinationServiceResource,
})
}
Expand Down Expand Up @@ -222,10 +217,9 @@ 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
c.destinationService.Resource = truncateString(service.Resource)
c.destination.Service = &c.destinationService
c.model.Destination = &c.destination
Expand Down
16 changes: 1 addition & 15 deletions spancontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,37 +65,31 @@ func TestSpanContextSetHTTPRequest(t *testing.T) {
url: "http://localhost/foo/bar",
addr: "localhost",
port: 80,
name: "http://localhost",
resource: "localhost:80",
}, {
url: "http://localhost:80/foo/bar",
addr: "localhost",
port: 80,
name: "http://localhost",
resource: "localhost:80",
}, {
url: "https://[::1]/foo/bar",
addr: "::1",
port: 443,
name: "https://[::1]",
resource: "[::1]:443",
}, {
url: "https://[::1]:8443/foo/bar",
addr: "::1",
port: 8443,
name: "https://[::1]:8443",
resource: "[::1]:8443",
}, {
url: "gopher://gopher.invalid:70",
addr: "gopher.invalid",
port: 70,
name: "gopher://gopher.invalid:70",
resource: "gopher.invalid:70",
}, {
url: "gopher://gopher.invalid",
addr: "gopher.invalid",
port: 0,
name: "gopher://gopher.invalid",
resource: "gopher.invalid",
}}

Expand All @@ -116,7 +110,6 @@ func TestSpanContextSetHTTPRequest(t *testing.T) {
Port: tc.port,
Service: &model.DestinationServiceSpanContext{
Type: spans[0].Type,
Name: tc.name,
Resource: tc.resource,
},
}, spans[0].Context.Destination)
Expand All @@ -132,29 +125,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 +151,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
3 changes: 1 addition & 2 deletions validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,11 @@ func TestValidateDatabaseSpanContext(t *testing.T) {

func TestValidateDestinationSpanContext(t *testing.T) {
overlong := strings.Repeat("x", 1025)
for _, name := range []string{"", overlong} {
for range []string{"", overlong} {
stuartnelson3 marked this conversation as resolved.
Show resolved Hide resolved
for _, resource := range []string{"", overlong} {
validateSpan(t, func(s *apm.Span) {
s.Context.SetDestinationAddress(overlong, 0)
s.Context.SetDestinationService(apm.DestinationServiceSpanContext{
Name: name,
Resource: resource,
})
})
Expand Down