chore: Added otel compliant spans to external http spans #2169
Conversation
Signed-off-by: mrickard <maurice@mauricerickard.com>
Signed-off-by: mrickard <maurice@mauricerickard.com>
mrickard
force-pushed
the
NR-260118/add-http-otel-props
branch
from
ca38dc4
to
297267d
Compare
lib/spans/streaming-span-event.js
Outdated
| @@ -128,6 +128,12 @@ class StreamingSpanEvent { | |||
|
|
|||
| let span = null | |||
| if (StreamingHttpSpanEvent.isHttpSegment(segment)) { | |||
| // Get external host from the segment name | |||
| const nameParams = segment.name.split('/') | |||
There was a problem hiding this comment.
this seems heavy handed. what i would do is abstract away the assigning of the attributes for http externals, and just add another one there for host. the reason i suggest abstracting is because we have multiple libraries that instrument libraries as http externals. in fact looking into this i think we were doing it wrong in grpc. I can show you a diff of what I'm suggesting
…an, updated span event object to take attributes and make server.address and server.port
| @@ -227,6 +235,8 @@ function applySegment(opts, makeRequest, hostname, segment) { | |||
| segment.addSpanAttribute(`request.parameters.${key}`, parsed.parameters[key]) | |||
| } | |||
| } | |||
| segment.attributes.addAttribute(DESTINATIONS.SPAN_EVENT, 'host', host) | |||
There was a problem hiding this comment.
adding destination of span event as we don't want these attrs on the segment. the lib/spans/span-event and streaming-span-event will take those and make server.address and server.port. I plan on filling a follow up to encapsulate all the external segment/span attributes into a function on the segment so we can call captureExternalAttributes
| @@ -197,8 +197,19 @@ class StreamingHttpSpanEvent extends StreamingSpanEvent { | |||
| agentAttributes.url = null | |||
| } | |||
|
|
|||
| if (agentAttributes.host) { | |||
| this.addAgentAttribute('server.address', agentAttributes.host) | |||
| agentAttributes.host = null | |||
There was a problem hiding this comment.
I noticed the nullifying of these properties isn't doing what we expect. I will file another ticket to deal with that
…p.request.method` to external http spans (#2169)
Description
Added OTEL-compliant
http.request.methodandserver.addressproperties to external HTTP spans.How to Test
This PR includes additions to
test/unit/spans/span-event.test.jsandtest/unit/spans/streaming-span-event.test.js, so running unit tests will include these.Related Issues
Closes #2150
Closes NR-260118