-
Notifications
You must be signed in to change notification settings - Fork 933
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
feat: Implement Pulsar 4.0 monitoring support with messaging.client.sent.messages #13599
base: main
Are you sure you want to change the base?
feat: Implement Pulsar 4.0 monitoring support with messaging.client.sent.messages #13599
Conversation
…nstrumentation/api/incubator/semconv/messaging/MessagingMetricsAdvice.java Co-authored-by: Lauri Tulmin <tulmin@gmail.com>
…nstrumentation/api/incubator/semconv/messaging/MessagingProducerMetrics.java Co-authored-by: Lauri Tulmin <tulmin@gmail.com>
…nstrumentation/api/incubator/semconv/messaging/MessagingProducerMetricsTest.java Co-authored-by: Steve Rao <raozihao.rzh@alibaba-inc.com>
…nstrumentation/api/incubator/semconv/messaging/MessagingProducerMetricsTest.java Co-authored-by: Steve Rao <raozihao.rzh@alibaba-inc.com>
Have a look at https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/ At the start of the document there is the following warning
Secondly the purpose of the refactor is unclear. As there is no latest dep restriction in |
This is a brand new metric:
When the pulsar-client version in use is greater than or equal to 4.0, the instrumentation for 2.8 will still function normally; in other words, the 4.0 version of the pulsar-client only adds new metrics on top of the 2.8 version. I have tested this locally, and it is working as expected. |
this is true, but the semconv guidance is to hide new metrics and attributes behind the flag, since it represents a newer version of the semconv schema (ideally we should be populating schema url on everything, see #11939) |
Thank you for your additional input. Is your opinion to handle this PR after enabling compatibility( Or should we wait until the messaging semantics are stable before addressing this? |
messaging.client.sent.messages
.