Infer destination service resource adapt public api #1003
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
SpanData is removed when span is ended, but we may want to tell if a span was an exit span after it's been ended.
tests are passing, but it'd be nice to not have this complicated dance.
span.go
Outdated
| if opts.parent != nil { | ||
| opts.parent.mu.Unlock() | ||
| } |
There was a problem hiding this comment.
Moving the unlock away from the lock scares me a bit. TBH I'm really not a fan of this implicit exit span status, so I've raised the question with agent devs whether it is necessary to implement that part of the spec.
EDIT: point being that if we don't need to do the implicit check, and we only have explicitly identified exit spans, then "s.exit" is immutable from span start and requires no locking.
There was a problem hiding this comment.
👍 makes sense. I'm not a fan of this locking either, so let's see what happens.
span.go
Outdated
| @@ -384,6 +398,25 @@ func (s *Span) ended() bool { | |||
| return s.SpanData == nil | |||
| } | |||
|
|
|||
| func (s *Span) setExitSpan(name, spanType string) { | |||
There was a problem hiding this comment.
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.
If the user does not set a value by the time the span is ended, infer the value from the span data
span.go
Outdated
| @@ -384,6 +403,24 @@ func (s *Span) ended() bool { | |||
| return s.SpanData == nil | |||
| } | |||
|
|
|||
| func (s *Span) setExitSpan() { | |||
There was a problem hiding this comment.
setExitSpanDestinationService might be a bit clearer on the intent?
stuartnelson3
deleted the
infer-destination-service-resource-adapt-public-api
branch
destination.service.resourcedestination.service.{name,type}from the public api and mark them as deprecatedcloses #976