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.

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

Enable Metrics for RabbitMQ Component #583

Open
eerhardt opened this issue Oct 30, 2023 · 8 comments
Open

Enable Metrics for RabbitMQ Component #583

eerhardt opened this issue Oct 30, 2023 · 8 comments
Labels
area-integrations Issues pertaining to Aspire Integrations packages rabbitmq Issues related to rabbitmq integrations
Milestone

Comments

@eerhardt
Copy link
Member

RabbitMQ's current version is 6.6.0, which doesn't have any meters/event counters.

However, they have an alpha 7.0 version out, which does have event counters - see https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/a0321c6c2203cedeacdd5c1ec5bcdc924e85a842/projects/RabbitMQ.Client/client/logging/RabbitMqClientEventSource.Counters.cs#L51-L58.

When we have a RabbitMQ component that depends on v7, we should enable Metrics via these EventCounters.

@eerhardt eerhardt added the area-integrations Issues pertaining to Aspire Integrations packages label Oct 30, 2023
@eerhardt eerhardt added this to the some time after preview milestone Oct 30, 2023
@danmoseley
Copy link
Member

For future ref their commit was rabbitmq/rabbitmq-dotnet-client@6e6ab3e and indeed it's only in v7+.

Looks like they're pretty early in their v7 so it will be a question of whether it's stable in time for us to ship with it.

@danmoseley danmoseley removed this from the needs milestone (for GA) milestone Nov 13, 2023
@davidfowl
Copy link
Member

@eerhardt backlog or will we get this in preview?

@davidfowl davidfowl added this to the preview TBD (but in 8.0) milestone Jan 27, 2024
@eerhardt
Copy link
Member Author

@eerhardt backlog or will we get this in preview?

I don't expect RabbitMQ v7 to be released before Aspire's 8.0 release. (But would be pleasantly surprised.)

@davidfowl davidfowl added enhancement rabbitmq Issues related to rabbitmq integrations labels Sep 8, 2024
@eerhardt eerhardt added help wanted Issue that is a good candidate for community contribution. and removed help wanted Issue that is a good candidate for community contribution. labels Jan 22, 2025
@eerhardt
Copy link
Member Author

We have support for RabbitMQ v7 now with #6770.

However, an issue is that we don't/can't use the OpenTelemetry.Instrumentation.EventCounters nuget package to pull EventCounters into OTel Metrics. See #2819 - OpenTelemetry.Instrumentation.EventCounters does not ship a "stable" version, which means our client integration library can't depend on the package (or else it can't be "stable").

Options we have to move forward here:

  1. Push for RabbitMQ.Client to use System.Diagnostics.Metrics.Meter instead of (or in addition to) EventCounters.
  2. "Vendor"/copy in the source code for OpenTelemetry.Instrumentation.EventCounters like we do for other opentelemetry-dotnet-contrib libraries.
  3. Don't add the metrics for now and wait for OpenTelemetry.Instrumentation.EventCounters to go stable.

cc @samsp-msft @noahfalk

@maddymontaquila
Copy link
Member

Sounds good. Any of those are fine with me so I'll leave it up to you / @samsp-msft / @noahfalk to make a call on this one

@noahfalk
Copy link
Member

I think the ideal option is moving to Meters (option 1). The Meter API is our default recommendation for anything new as it is more flexible, more powerful, and aligned with OpenTelemetry, a growing industry standard. (https://learn.microsoft.com/en-us/dotnet/core/diagnostics/compare-metric-apis)

If for whatever reason Meters is off the table then we could weigh the pros and cons of (2), (3), or add option (4) do nothing. I don't know if OTel is seeing enough demand that their EventCounters instrumentation will ever get promoted to stable. EventCounters are more like a back-compat bridge and demand is probably going down over time rather than up at this point.

@eerhardt
Copy link
Member Author

It looks like there are a few discussions in the RabbitMQ repo regarding using Meters.

@stebet - I see you've been heavily involved in adding OTel support for the RabbitMQ.Client library. Any thoughts on what should be done here? Do you think it is feasible to get RabbitMQ.Client to start using Meters (either instead of or in addition to) the existing EventCounters?

@stebet
Copy link

stebet commented Jan 30, 2025

@stebet - I see you've been heavily involved in adding OTel support for the RabbitMQ.Client library. Any thoughts on what should be done here? Do you think it is feasible to get RabbitMQ.Client to start using Meters (either instead of or in addition to) the existing EventCounters?

Certainly :) I'll take a look at it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-integrations Issues pertaining to Aspire Integrations packages rabbitmq Issues related to rabbitmq integrations
Projects
None yet
Development

No branches or pull requests

6 participants