feat: Added segment and transaction synthesis for http server spans #2833
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2833 +/- ##
========================================
Coverage 97.30% 97.30%
========================================
Files 299 303 +4
Lines 47019 47146 +127
========================================
+ Hits 45751 45877 +126
- Misses 1268 1269 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| this.segment = this.genericHttpSegment() | ||
| } | ||
| this.transaction.baseSegment = this.segment | ||
| return { segment: this.segment, transaction: this.transaction } |
There was a problem hiding this comment.
I started writing class ConsumerSegment this morning and when I got to this equivalent line, I realized that we are returning something other than the object being created with new Whatever. Clearly that doesn't matter since return overriding is a thing, but do you think we should do so?
I ended up switching to writing a module segments/consumer.js that would just contain all of the code specific to that segment type and exporting a createConsumerSegment function.
I'm not opposed to either method. It just feels a little off to me to return something different from a class syntax constructor.
There was a problem hiding this comment.
agreed. let me think on it more. We can chat tomorrow as well
There was a problem hiding this comment.
we agreed to address this in a separate PR
Description
This creates both the transaction and segment when the otel span is a server match. I expect more work to be done once we wire up the context manager. We're forcing the transaction name whereas our instrumentation decorates the namestate and finalizes the same on transaction end. I've found that most otel instrumentation sets the http.route attribute in the web frameworks which is like us and I think that's being used to finalize the http span name on the end of the request.
I also changed what the synthesizer returns. In the case of servers and will be for #2651 we need to create the transaction. So when the instrumentation for otel is wired up we'll need to bind both the transaction and segment to the context manager and not just the segment.
Related Issues
Closes #2649