feat: Added otel consumer span processing #2834
Conversation
jsumners-nr
force-pushed
the
issue-2651
branch
from
6363baa to
f3a0d1f
Compare
… and enterTransaction to make context propagation more clear (newrelic#2646)
…client otel spans to external http trace segments (newrelic#2745)
jsumners-nr
force-pushed
the
issue-2651
branch
from
a201e19 to
7334acb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2834 +/- ##
===========================================
- Coverage 97.26% 79.24% -18.02%
===========================================
Files 294 287 -7
Lines 46405 45542 -863
===========================================
- Hits 45135 36091 -9044
- Misses 1270 9451 +8181
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
lib/otel/segments/consumer.js
Outdated
| const destination = otelSpan.attributes[SEMATTRS_MESSAGING_DESTINATION] ?? 'unknown' | ||
| const segmentName = `OtherTransaction/consumer/${operation}/${destination}` | ||
|
|
||
| transaction.trace.attributes.addAttribute( |
There was a problem hiding this comment.
there's more logic in the subscribe-consume for adding other segment and trace attributes
There was a problem hiding this comment.
A good test, specific to amqplib but should apply to all but obviously the destination kind and destination vary: https://github.com/newrelic/node-newrelic/blob/main/test/versioned/amqplib/amqp-utils.js#L103
There was a problem hiding this comment.
I don't understand this one. Is there a document somewhere that clearly lists out the attribute names and expected values that are required?
There was a problem hiding this comment.
I think you know the answer to that question 😄
| test('should create consumer segment from otel span', (t) => { | ||
| const { synth, tracer } = t.nr | ||
| const span = createSpan({ tracer, kind: SpanKind.CONSUMER }) | ||
| span.setAttribute('messaging.operation', 'receive') |
There was a problem hiding this comment.
this test will have to be updated once the feedback is addressed for the new name and also attributes
jsumners-nr
force-pushed
the
issue-2651
branch
from
424f32b to
b2c4033
Compare
jsumners-nr
force-pushed
the
issue-2651
branch
from
b2c4033 to
39716e2
Compare
|
|
||
| const txAttrs = transaction.trace.attributes | ||
| txAttrs.addAttribute(DESTINATIONS.TRANS_SCOPE, 'message.queueName', destination) | ||
| // txAttrs.addAttribute( |
There was a problem hiding this comment.
you can remove this. All the other attributes seem to be all over the place. I'm not sure if we have access to host/port in the otel spans. Then the message.parameters attributes vary depending on the library. Same with message.routingKey
There was a problem hiding this comment.
Looking at amqplib a lot of the parameters live on these span attributes:
[SemanticAttributes.MESSAGING_DESTINATION]: exchange,
[SemanticAttributes.MESSAGING_DESTINATION_KIND]:
MessagingDestinationKindValues.TOPIC,
[SemanticAttributes.MESSAGING_RABBITMQ_ROUTING_KEY]:
msg.fields?.routingKey,
[SemanticAttributes.MESSAGING_OPERATION]:
MessagingOperationValues.PROCESS,
[SemanticAttributes.MESSAGING_MESSAGE_ID]:
msg?.properties.messageId,
[SemanticAttributes.MESSAGING_CONVERSATION_ID]:
msg?.properties.correlationId,
bizob2828
force-pushed
the
next
branch
2 times, most recently
from
26f5ea2 to
fe95d98
Compare
This PR resolves #2651.