Add Pulsar MessagingProducerMetrics #11591
Conversation
| @@ -12,6 +12,7 @@ group = "io.opentelemetry.instrumentation" | |||
|
|
|||
| dependencies { | |||
| api("io.opentelemetry.semconv:opentelemetry-semconv") | |||
| api("io.opentelemetry.semconv:opentelemetry-semconv-incubating") | |||
There was a problem hiding this comment.
We don't add incubating semconv dependency to the libraries we publish, instead we copy the constants from the incubating semconv. The reason we do this is that incubating semconv can change in backwards incompatible way. Imagine if you have 2 libraries depending on different versions of incubating semconv, it is possible that a version of incubating semconv that works for both doesn't exist.
Using incubating semconv in javaagent and test code is fine and encouraged.
There was a problem hiding this comment.
thank you for the clarification.
There was a problem hiding this comment.
Done; please take a look. Thanks.
...io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingMetricsAdvice.java
Outdated
Show resolved
Hide resolved
.../opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingProducerMetrics.java
Outdated
Show resolved
Hide resolved
| import java.util.concurrent.TimeUnit; | ||
| import java.util.logging.Logger; | ||
|
|
||
| public class MessagingProducerMetrics implements OperationListener { |
There was a problem hiding this comment.
Could you add javadoc similar to what the other Metrics classes have
There was a problem hiding this comment.
Done; please take a look. Thanks.
.../opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingProducerMetrics.java
Outdated
Show resolved
Hide resolved
.../java/io/opentelemetry/javaagent/instrumentation/pulsar/v2_8/telemetry/PulsarSingletons.java
Show resolved
Hide resolved
| import java.util.concurrent.TimeUnit; | ||
| import java.util.logging.Logger; | ||
|
|
||
| public class MessagingProducerMetrics implements OperationListener { |
There was a problem hiding this comment.
@trask should we call this MessagingProducerMetrics or MessagingProducerExperimentalMetrics?
There was a problem hiding this comment.
I think MessagingProducerMetrics is good, since all of messaging semconv is experimental still, and the duration metric will probably be included in the initial stability
…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>
...ntelemetry/instrumentation/api/incubator/semconv/messaging/MessagingProducerMetricsTest.java
Outdated
Show resolved
Hide resolved
...ntelemetry/instrumentation/api/incubator/semconv/messaging/MessagingProducerMetricsTest.java
Outdated
Show resolved
Hide resolved
…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>
...st/java/io/opentelemetry/javaagent/instrumentation/pulsar/v2_8/AbstractPulsarClientTest.java
Show resolved
Hide resolved
This reverts commit a46c8a0.
Add Pulsar
MessagingProducerMetrics, reference message semantics.