Add OpenTelemetry support via ActivitySource

[stebet]

Proposed Changes

This is the basic implementation to add OpenTelemetry support to the .NET RabbitMQ Client. This implements an ActivitySource that creates and propagates Activities for BasicPublish (send), BasicDeliver (receive) and when consumers receive messages (process). This enables distributed tracing scenarios such as showing traces. Below is a screenshot from Honeycomb.io to show what happens when tracing is enabled.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)

Checklist

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• All tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in related repositories

Further Comments

This should have minimal overhead when tracing is not enabled since no Activity generation occurs and no tags need to be populated.

Currently, to propagate the trace and span ids to correlate events, I'm taking advantage of the BasicProperties.Headers to send and parse the information needed to correlate individual traces.

Here is a screenshot from Honeycomb of what this can look like:

[stebet]

It would be great to get some feedback from @cijothomas, @MikeGoldsmith and @martinjt if they have time. I think I've populated the most important tags that are easy to get, but in case I'm missing some, it'd be great to get feedback.

[lukebakken]

Hello @stebet - would you have a moment to give an update here?

[stebet]

This should hopefully be ready now. Feel free to give it another pass @cijothomas if you have time :) @bollhals might find this interesting as well.

[bollhals]

This should have minimal overhead when tracing is not enabled since no Activity generation occurs and no tags need to be populated.

Would you mind running some tests on this (with and without tracing enabled)? I suspect some boxing occurring even in the disabled case.
For the enabled case, this will cause significant impact (performance and allocation wise), as it has lots of tostrings and boxing and stuff.

One question I had was do we need a separate option to enable it? E.g. my business has tracing enabled, but we'd might not want to know this level of detail from RabbitMQ.

[stebet]

Would you mind running some tests on this (with and without tracing enabled)? I suspect some boxing occurring even in the disabled case. For the enabled case, this will cause significant impact (performance and allocation wise), as it has lots of tostrings and boxing and stuff.

Yeah, will do. Although even when tracing is enabled, if the activity isn't sampled, it'll still skip A LOT of the processing since it checks first if the Activity is requesting all data.

B.t.w, overhead is expected, but the APIs are very fast and used for both runtime stuff (SQL client, HTTP client) as well as as third party libraries like StackExchange.Redis so i.m.o the overhead when tracing is enabled is acceptable and known. That's why Sampling is a built-in feature for Activities.

One question I had was do we need a separate option to enable it? E.g. my business has tracing enabled, but we'd might not want to know this level of detail from RabbitMQ.

Yes. For example if you are using the OpenTelemetry libraries you'll need to do something like this:

builder.Services.AddOpenTelemetryTracing(builder =>
{
    builder
    .Configure((IServiceProvider serviceProvider, TracerProviderBuilder traceBuilder) =>
    {
        traceBuilder
        .SetResourceBuilder(ResourceBuilder.CreateDefault().AddService("test-app", serviceVersion: "1.0.0"))
        .AddSource("RabbitMQ.Client") // Enable tracing for the RabbitMQ.Client library. Not setting this causes no activities to be created within the library.
        .AddAspNetCoreInstrumentation(options => options.RecordException = true)
        .... // more initialization stuff here
        ;
    });
});

[bollhals]

Would you mind running some tests on this (with and without tracing enabled)? I suspect some boxing occurring even in the disabled case. For the enabled case, this will cause significant impact (performance and allocation wise), as it has lots of tostrings and boxing and stuff.

Yeah, will do. Although even when tracing is enabled, if the activity isn't sampled, it'll still skip A LOT of the processing since it checks first if the Activity is requesting all data.

B.t.w, overhead is expected, but the APIs are very fast and used for both runtime stuff (SQL client, HTTP client) as well as as third party libraries like StackExchange.Redis so i.m.o the overhead when tracing is enabled is acceptable and known. That's why Sampling is a built-in feature for Activities.

Yes, sure it's expected if you enable such a feature, but there are still things we could optimize for to avoid allocations. But this can also wait for later. But we should look at overhead of the two other options of no tracing enabled and tracing enabled but not for RabbitMQ.

One question I had was do we need a separate option to enable it? E.g. my business has tracing enabled, but we'd might not want to know this level of detail from RabbitMQ.

Yes. For example if you are using the OpenTelemetry libraries you'll need to do something like this:

builder.Services.AddOpenTelemetryTracing(builder =>
{
    builder
    .Configure((IServiceProvider serviceProvider, TracerProviderBuilder traceBuilder) =>
    {
        traceBuilder
        .SetResourceBuilder(ResourceBuilder.CreateDefault().AddService("test-app", serviceVersion: "1.0.0"))
        .AddSource("RabbitMQ.Client") // Enable tracing for the RabbitMQ.Client library. Not setting this causes no activities to be created within the library.
        .AddAspNetCoreInstrumentation(options => options.RecordException = true)
        .... // more initialization stuff here
        ;
    });
});

Ah yes, forgot about it, as I only had to do it once for our project :D Thanks for reminding me.

[stebet]

@bollhals
I did some simple benchmarks with the MassTest app modified to do 1000 batches (up from 100) with 500 items each, and seems the overhead is minimal (after I fixed a small bug):

Current master build

ActivitySource build with OpenTelemetry initialized but not listening for RabbitMQ.Client events

[xsoheilalizadeh]

Hi @stebet, thanks for making this PR. Do you think we could have it in the next coming months?

[stebet]

Rebased this on the latest master and removed the need for OpenTelemetry.API reference as suggested by @rwkarg :)

[Kielek]

There were a lot of changes related to tag-naming for messaging. Still, it is not stable version, but it reflects current state.

[stebet]

Updated to be in line with current spec. Thanks @Kielek, would love it if you could verify that I got things correcly :)

[Kielek]

One obvious typo.
I am not an subject expert for messaging system so please double check the second one.

[Kielek]

Traces` attributes LGTM

[stebet]

I would leave out the baggage support completely and document it as such. This PR is big enough that we don't need to block on that one feature. Let's open a new issue and come up with something reasonable outside of this PR.

In that case, this PR is pretty much ready to go @michaelklishin @lukebakken. Documentation/examples is handled in another repo AFAIK. I'm assuming there is still some time before a new stable major version of the NuGet package (which would include this PR) is released?

That should give us tome to iterate a bit on this and get better baggage handling, otel metrics etc. in.

[cijothomas]

nit: Would you point to a specific version, as these conventions are not stable ?

[lukebakken]

cc @stebet ?

[cijothomas]

Thanks.
Would you add examples/docs as a follow up? The name of the ActivitySource used must be documented so OTel consumers can enable this instrumentation.
Also - please call out that this relies on experimental semantic convention and is subject to change, so users are not caught with surprise if the conventions change before they stabilize.

[lukebakken]

If people need this feature soon, a user has found the following workaround that uses AMQP headers:

https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/examples/MicroserviceExample

[lukebakken]

@cijothomas I personally know next to nothing about OpenTelemetry and the changes in this PR, so someone who is more familiar will have to address your concerns:

• Adding examples/docs. The name of the ActivitySource used must be documented so OTel consumers can enable this instrumentation
• Call out that this relies on experimental semantic convention and is subject to change, so users are not caught with surprise if the conventions change before they stabilize.

[lukebakken]

@stebet I'm merging the latest main into this PR at the moment.

[lukebakken]

@stebet I have completed rebasing this PR on main ... please go over the code again to make sure I didn't change something inadertently.

[stebet]

@stebet I have completed rebasing this PR on main ... please go over the code again to make sure I didn't change something inadertently.

I'll take a look in a bit.

[lukebakken]

@stebet everything seems to be fine, except that the Test.Integration.TestActivitySource.TestPublisherAndBasicGetActivityTags is flaky on GitHub actions - https://github.com/rabbitmq/rabbitmq-dotnet-client/actions/runs/7658052226/job/20870377811

Of course it works every time on my machine.

[stebet]

@stebet everything seems to be fine, except that the Test.Integration.TestActivitySource.TestPublisherAndBasicGetActivityTags is flaky on GitHub actions - https://github.com/rabbitmq/rabbitmq-dotnet-client/actions/runs/7658052226/job/20870377811

Of course it works every time on my machine.

I'll take a look now and see if I see anything obvious

[stebet]

@lukebakken Moved the Activity tests over to the sequential tests since they are manipulating static variables.

[davidfowl]

Glad to see this making progress!

[lukebakken]

@stebet if you have time to address these comments that would be great:

• Add OpenTelemetry support via ActivitySource #1261 (review)
• Add OpenTelemetry support via ActivitySource #1261 (comment) @cijothomas - if you can address this comment of yours that would be great.
• Documentation/examples is handled in another repo AFAIK - @stebet are you referring to the repo that is used to create this web site?

[stebet]

Will do.

• Documentation/examples is handled in another repo AFAIK - @stebet are you referring to the repo that is used to create this web site?

Yes.

[lukebakken]

@cijothomas I would appreciate example code and doc text that I can use to update the web site. Thanks.

#1478

[davidfowl]

Excellent! I think we'd like to get this backported to the older version. Before we do that though, is it possible to get a preview build that we can ship with an aspire preview so we can get some mileage on it?