Skip to content

Conversation

@crossoverJie
Copy link
Member

  • A new monitoring metric has been added for Pulsar 4.0: messaging.client.sent.messages.
  • Refactor common components into pulsar-common module

crossoverJie and others added 23 commits June 14, 2024 15:40
…nstrumentation/api/incubator/semconv/messaging/MessagingMetricsAdvice.java

Co-authored-by: Lauri Tulmin <[email protected]>
…nstrumentation/api/incubator/semconv/messaging/MessagingProducerMetrics.java

Co-authored-by: Lauri Tulmin <[email protected]>
…nstrumentation/api/incubator/semconv/messaging/MessagingProducerMetricsTest.java

Co-authored-by: Steve Rao <[email protected]>
…nstrumentation/api/incubator/semconv/messaging/MessagingProducerMetricsTest.java

Co-authored-by: Steve Rao <[email protected]>
@crossoverJie crossoverJie requested a review from a team as a code owner March 27, 2025 10:18
@laurit
Copy link
Contributor

laurit commented Mar 27, 2025

Have a look at https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/ At the start of the document there is the following warning

Existing messaging instrumentations that are using v1.24.0 of this document (or prior):

SHOULD NOT change the version of the messaging conventions that they emit by default until the messaging semantic conventions are marked stable. Conventions include, but are not limited to, attributes, metric and span names, span kind and unit of measure.
SHOULD introduce an environment variable OTEL_SEMCONV_STABILITY_OPT_IN in the existing major version which is a comma-separated list of values. The list of values includes:
messaging - emit the new, stable messaging conventions, and stop emitting the old experimental messaging conventions that the instrumentation emitted previously.
messaging/dup - emit both the old and the stable messaging conventions, allowing for a seamless transition.
The default behavior (in the absence of one of these values) is to continue emitting whatever version of the old experimental messaging conventions the instrumentation was emitting previously.
Note: messaging/dup has higher precedence than messaging in case both values are present
SHOULD maintain (security patching at a minimum) the existing major version for at least six months after it starts emitting both sets of conventions.
SHOULD drop the environment variable in the next major version.
SHOULD emit the new, stable values for span name, span kind and similar “single” valued concepts when messaging/dup is present in the list.

Secondly the purpose of the refactor is unclear. As there is no latest dep restriction in 2.8 instrumentation it must still apply to 4.0.

@crossoverJie
Copy link
Member Author

Have a look at opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics At the start of the document there is the following warning

This is a brand new metric: messaging.client.sent.messages, so there are no compatibility issues.

Secondly the purpose of the refactor is unclear. As there is no latest dep restriction in 2.8 instrumentation it must still apply to 4.0.

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.

@trask
Copy link
Member

trask commented Mar 27, 2025

there are no compatibility issues

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)

@crossoverJie
Copy link
Member Author

there are no compatibility issues

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(-Dotel.semconv-stability.opt-in=messaging) for messaging semantics?

Or should we wait until the messaging semantics are stable before addressing this?  

@trask
Copy link
Member

trask commented Apr 10, 2025

Is your opinion to handle this PR after enabling compatibility(-Dotel.semconv-stability.opt-in=messaging) for messaging semantics?

yes, you can implement it behind this flag, check out also #13192

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2025

This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 14 days.

@github-actions github-actions bot added the stale label Oct 7, 2025
@github-actions github-actions bot closed this Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants