Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[OTel] Activity is not produced for BasicConsumeAsync on .NET Framework #1533

Closed
Kielek opened this issue Apr 16, 2024 · 9 comments
Closed

[OTel] Activity is not produced for BasicConsumeAsync on .NET Framework #1533

Kielek opened this issue Apr 16, 2024 · 9 comments
Assignees
@lukebakken @stebet
Labels
bug
Milestone
7.0.0

Comments

@Kielek
Copy link

Kielek commented Apr 16, 2024 •
edited by lukebakken
Loading

Describe the bug

Following code does not generate activity for RabbitMQ.Client.Subscriber activity source.

var consumer = new EventingBasicConsumer(channel);
consumer.Received  = (model, ea) =>
{
    var receivedBody = ea.Body.ToArray();
    var receivedMessage = Encoding.UTF8.GetString(receivedBody);
    Console.WriteLine($"Received: {receivedMessage}");
};
channel.BasicConsumeAsync(queue: "hello", autoAck: true, consumer);

Reproduction steps

  1. Have an application consuming messages from queue: https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/pull/3373/files#diff-b8bf5e4638613373d455983b52f680cbac4c13732b860567bda571a9e0b29638R40-R44
  2. Have properly defined listener for RabbitMQ.Client.Subscriber
  3. If you are using channel.BasicConsumeAsync on .NET Framework 4.6.2, Activity is not generated for channel.BasicConsumeAsync.

It is working fine for .NET6, .NET7 and .NET8. channel.BasicGetAsync generated proper activities.

Expected behavior

Activity RabbitMQ.Client.Subscriber is generated also for channel.BasicConsumeAsync method on .NET Framework.

Additional context

Found while working on introducing RabbitMQ.Client in OpenTelemetry .NET Automatic Instrumentation.

PR: open-telemetry/opentelemetry-dotnet-instrumentation#3373
@Kielek Kielek added the bug label Apr 16, 2024
@Kielek Kielek mentioned this issue Apr 16, 2024
Closed
3 tasks
@michaelklishin
Copy link
Member

@Kielek thank you for investigating. Have you filed an issue here just for visibility or you expect that in addition to open-telemetry/opentelemetry-dotnet-instrumentation#3373, this client's OpenTelemetry integration would also have to change?

If it's the latter, feel free to submit a PR :)

@Kielek
Copy link
Author

Kielek commented Apr 16, 2024

I would consider this as a bug in RabbitMQ.Client.

I cannot promise that I will have time for fixing it here. Linked PR was the easiest way to provide you application/case which is not working.

@lukebakken lukebakken added this to the 7.0.0 milestone Apr 16, 2024
@lukebakken
Copy link
Contributor

@stebet is there a chance this issue will be fixed by #1528 ?

@lukebakken lukebakken changed the title [OTel] Acitity is not produced for BasicConsumeAsync on .NET Framework [OTel] Activity is not produced for BasicConsumeAsync on .NET Framework May 12, 2024
@lukebakken
Copy link
Contributor

@Kielek now that #1528 is merged, I will release 7.0.0-RC1. Could you PLEASE test that version to ensure it meets your requirements?

Please note that there will be a RabbitMQ.Client.OpenTelemetry library published as well.

@lukebakken
Copy link
Contributor

@Kielek - 7.0.0-rc.1 is available for you to test with:

Let me know if you have any issues or questions. Thanks!

@Kielek
Copy link
Author

Kielek commented Jun 6, 2024

@lukebakken, I have checked the behavior once more time.
The problem was race condition between shouting down event and receiving message event.
For some reasons, .NET always handled receiving correctly, .NET Framework proffered shutting down.

I am closing an issues, sorry for bothering you.

@Kielek Kielek closed this as completed Jun 6, 2024
@lukebakken
Copy link
Contributor

Thank you for following up @Kielek!!!

@Kielek
Copy link
Author

Kielek commented Jun 6, 2024

@lukebakken, one more thing from the code analysis.
This line can be removed:

rabbitmq-dotnet-client/projects/RabbitMQ.Client.OpenTelemetry/TraceProviderBuilderExtensions.cs

Line 15 in 3adee90

RabbitMQActivitySource.UseRoutingKeyAsOperationName = true;

The property is set to true on the RabbitMQActivitySource class.

I have had plan to create PR, but it will take ages to have CLA approval, could you please handle it?

lukebakken added a commit that referenced this issue Jun 6, 2024
@lukebakken
Use default value for `RabbitMQActivitySource.UseRoutingKeyAsOperatio…
e54375d 
…nName`

The default is `true`, anyway. No need to set it here.

Pointed out by @kielex in this comment:
#1533 (comment)
@lukebakken lukebakken mentioned this issue Jun 6, 2024
Merged
@lukebakken
Copy link
Contributor

@Kielek sure thing, here it is - #1595

Thanks for noticing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukebakken lukebakken

@stebet stebet

Labels
bug
Projects
None yet
Milestone
7.0.0
Development

No branches or pull requests
4 participants
@michaelklishin @lukebakken @stebet @Kielek