chore: updated mapping#2615
Conversation
abhilash-sivan
force-pushed
the
chore-update-mapping
branch
2 times, most recently
from
9b234ee to
7706438
Compare
abhilash-sivan
changed the title
aryamohanan
force-pushed
the
chore-otel-span-mapper
branch
from
97f77cd to
ce82272
Compare
abhilash-sivan
force-pushed
the
chore-update-mapping
branch
from
e4c0b88 to
468fecb
Compare
abhilash-sivan
changed the title
| [INSTRUMENTATION_TYPES.KAFKA]: { | ||
| spanName: data => `${data.operation} ${data.endpoints}`, | ||
| // In KafkaNode, we collect service directly | ||
| spanName: data => `${data.operation || data.access} ${data.service || data.endpoints}`, |
There was a problem hiding this comment.
I don't think this change is correct. Kafka doesn't have an access field in the internal format, we already normalize it to operation, same for service and endpoint.
| spanName: data => `${data.operation || data.access} ${data.service || data.endpoints}`, | |
| spanName: data => `${data.operation} ${data.endpoints}`, |
There was a problem hiding this comment.
In KafkaNode, we collect access and service directly.
see:
Seems like this is a valid case 🤔
There was a problem hiding this comment.
I think this is something we ignored earlier. I think we need a refactoring PR for kafka node to use internal format.
There was a problem hiding this comment.
its not a good idea to handle internal and be formats
| { otlp: OTLP.messaging.SYSTEM, value: 'kafka' }, | ||
| { otlp: OTLP.messaging.DESTINATION_NAME, instana: 'endpoints' }, | ||
| { otlp: OTLP.messaging.OPERATION_NAME, instana: 'operation' }, | ||
| { otlp: OTLP.messaging.DESTINATION_NAME, instana: ['service', 'endpoints'], transform: firstDefined }, |
| transform: values => { | ||
| const op = firstDefined(values); | ||
| if (op === 'send') return 'send'; | ||
| if (op === 'consume') return 'receive'; |
There was a problem hiding this comment.
Why are we updating the value? IMO, we shouldn't modify it, we should send it as-is. These are package-specific operation names, and they can differ across packages or even between language implementations.
TBD
There was a problem hiding this comment.
Yeah, we already discussed this. I took the mapping from RFD, but as you said, modifying the value doesn't seem correct. I will revert this change
| return op || null; | ||
| } | ||
| }, | ||
| { otlp: OTLP.messaging.BATCH_MESSAGE_COUNT, spanLevel: 'b.s' }, |
There was a problem hiding this comment.
I don't think this logic is correct. b.s represents the span batch size and shouldn't be coupled with the messaging batch size.
| // bull.sort 'exit' → 'send', 'entry' → 'process' | ||
| otlp: OTLP.messaging.OPERATION_NAME, | ||
| instana: 'sort', | ||
| transform: sort => BULL_SORT_OP[String(sort)] || sort |
| { otlp: OTLP.messaging.DESTINATION_NAME, instana: 'queue' }, | ||
| { otlp: OTLP.messaging.MESSAGE_BODY_SIZE, instana: 'size' }, | ||
| // sqs.size = number of messages in a batch send (SendMessageBatch), not body bytes | ||
| { otlp: OTLP.messaging.BATCH_MESSAGE_COUNT, instana: 'size' }, |
aryamohanan
force-pushed
the
chore-otel-span-mapper
branch
from
89a73c7 to
dfaf341
Compare
3 participants
grpc
aws s3
azure storage
aws lambda
aws lambda invoke
revisitted messaging libs for gap filling