open-telemetry
[Symfony auto-instrumentation] Messenger instrumentation should use SpanKind::KIND_PRODUCER, SpanKind::KIND_CONSUMER
Steps to reproduce
Enable Symfony auto-instrumentation with default setup.
What is the expected behavior?
Spans when creating the message should be marked with SpanKind::KIND_PRODUCER.
Spans when handling the message should be marked with SpanKind::KIND_CONSUMER.
https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/#span-kind
What is the actual behavior?
Everything is marked KIND_INTERNAL and it seems the consumer is currently not instrumented at all (I might be wrong here, please feel free to correct me).
Additionally, it seems to me the names of the spans are also not correct, as shown in the table in the spec linked above?
Hello @AA2512 please I'd like to know if this is still being worked please? @brettmc for possible assigning
Thank you
Hello and welcome, @RichardChukwu Please feel free to submit to work on this and submit a PR.
@chiomalovet it was just assigned to @RichardChukwu, unless he changed his mind, he should get dibs for a while to give it a shot.
On Tue, 8 Oct 2024 at 2:04 PM, Dalibor Karlović @.***> wrote:
@chiomalovet https://github.com/chiomalovet it was just assigned to @RichardChukwu https://github.com/RichardChukwu, unless he changed his mind, he should get dibs for a while to give it a shot.
— Reply to this email directly, view it on GitHub https://github.com/open-telemetry/opentelemetry-php/issues/1314#issuecomment-2399791329, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2JQPTMNSYLGUBWTUON24X3Z2PJ5TAVCNFSM6AAAAABH7T4FAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJZG44TCMZSHE . You are receiving this because you were mentioned.Message ID: @.***>
Alright no problem Let me look for another one to work on
Let me look for another one to work on
Great, there's not a shortage of work on the project(s)! :smile:
Hello @dkarlovi please can you specify the exact folder location for this. I can't seem to figure out here symfony auto-instrumentation is in the repo inorder to apply a fix
@RichardChukwu I know what you mean, I have the same issue navigating these OTEL repos, it's very confusing if you're not here day to day, I was about to suggest the contrib repo gets merged into this repo since the packages are now released from the split repos anyway. WDYT @bobstrecansky @brettmc?
To answer your question, it's here I believe: https://github.com/open-telemetry/opentelemetry-php-contrib/tree/main/src/Instrumentation/Symfony
Hello @brettmc kindly review PR (https://github.com/open-telemetry/opentelemetry-php-contrib/pull/304) as a fix I submitted to this issue. I'll appreciate feedback where necessary too. Thank you
@bobstrecansky I believe this issue shouldn't be closed because
it seems the consumer is currently not instrumented at all it seems to me the names of the spans are also not correct
The linked PR changed spans to KIND_PRODUCER, but KIND_CONSUMER wasn't updated (since it doesn't even exist, as noted) and the names weren't updated to match the spec either.
@RichardChukwu some additional changes noted above, if you're interested in working on them :)
Before working on this, just want to ensure i get you correctly @dkarlovi
Do you mean updating the code to introduce new spans with KIND_CONSUMER for the parts of the code handling message reception or processing as it doesn't exist?
For instance, based on the spec from what I can see, I presume key changes will be:
-
Adding the "consume" operation with SpanKind::KIND_CONSUMER for receiving messages.
-
Updating span names to use publish, send, and consume as per the spec, ensuring naming consistency for operations.
-
Instrumenting the SenderInterface for both sending and receiving spans, with the appropriate SpanKind (producer for sending and consumer for receiving).
Your thoughts please....
Do you mean updating the code to introduce new spans with KIND_CONSUMER for the parts of the code handling message reception or processing as it doesn't exist?
Yes, correct. The messages being created and sent is currently fully instrumented, but them being received and processed is not instrumented at all, from my understanding at least.
Alright, got it. I'll work on that and send a PR for a fix soon @dkarlovi
Kindly check this https://github.com/open-telemetry/opentelemetry-php-contrib/pull/307 @brettmc @dkarlovi
@chiomalovet if you're still up for it, you could take a look at the current state of the PR and continue from there. /cc @brettmc