opentelemetry-java
opentelemetry-java copied to clipboard
Add Metrics Annotations
Add annotations @Timed
and @Counted
for automatically instrumenting method invocations.
The annotations are similar to the annotations offered by Micrometer, MicroProfile metrics, and Dropwizard metrics.
The default attributes class, method, and exception are similar to the default attributes provided by Micrometer's annotations.
Here's an example of what a @Counted
metric would look like in Prometheus format:
method_invocations_total{class="com.example.demo.Example", method="example", exception="None"} 1.0
This PR is a follow-up to the discussion in #4251.
Codecov Report
Merging #4260 (eaa90a3) into main (5f32636) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## main open-telemetry/opentelemetry-java#4260 +/- ##
=========================================
Coverage 90.04% 90.04%
Complexity 5057 5057
=========================================
Files 580 580
Lines 15573 15573
Branches 1497 1497
=========================================
Hits 14022 14022
Misses 1090 1090
Partials 461 461
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 5f32636...eaa90a3. Read the comment docs.
Do we have a planned version for this to be merge for? This seems like a valuable feature. Hoping it wasn't abandoned.
It feels strange accepting additions to the stable extension annotations module without knowing what an implementation of a corresponding annotation processor looks like.
Can opentelemetry-extension-annotations
move to opentelemetry-java-instrumentation
to live alongside the annotation processor implementation?
If not, can we we put this in opentelemetry-extension-incubator
while we build confidence in the API / implementation?
My initial impulse is that opentelemetry-extension-incubator
is a better fit because users would explicitly use these annotations in their code. Should I move it?
I moved it to extensions/incubator/
. The new annotations are now in package io.opentelemetry.extension.incubator.annotations
.
From the discussion above I think the only unresolved point is how to use return values as attributes.
Current implementation:
@Timed(returnValueAttribute = "process.result")
public int process() {
return 3;
}
Alternative proposal:
@Timed
@ReturnAttribute("process.result")
public int process() {
return 3;
}
I think the idea behind the alternative proposal is that @ReturnAttribute
could work for both, spans and metrics. However, there is a fundamental difference between spans and metrics: For metrics, each attribute value creates a new time series. If you use a high cardinality return value as a metric attribute, you might overload the monitoring backend. For spans, high cardinality attributes are not an issue.
For that reason I think we should provide a way to say that a return value will be a span attribute, but not a metric attribute. With returnValueAttribute
being a property of the @Timed
and @Counted
annotations, it is clear that this is only for the metric but does not affect any spans.
Anyway, I'm happy to change this if consensus is that we should go with a dedicated @ReturnAttribute
annotation.
@fstab sorry about the delay on this. We're going to discuss at today's OTel java office hours and will respond with a recommendation on next steps ASAP.
Hi @fstab just following up on @jkwatson's comment. Here's where we're at:
- We don't think opentelemetry-java is the right long term home for these annotations, given that the primary implementation lives in opentelemetry-java-instrumentation. We'd like to make opentelemetry-java-instrumentation the long term home of annotations.
- Modules in opentelemetry-java-instrumentation generally live in
io.opentelemetry.instrumentation.*
package, versusio.opentelemetry.*
for opentelemetry-java projects. In order to continue following this pattern while not breaking compatibility for the annotations module, we're going to start a new annotations module in opentelemetry-java-instrumentation, deprecating the opentelemetry-javaopentelemetry-extension-annotations
module until we ultimately stop publishing it starting with the next major version. - We'd like to not get further away from this target, and merging this until the span annotations have been moved to opentelemetry-java-instrumentation. At that point, we can open a PR adding these annotations to that new module, and simultaneously include an implementation for the annotations.
Again, sorry for the delay.
I've kicked off the discussion in the instrumentation repo https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/6245, and added the topic of naming to the SIG agenda for tomorrow. Once we close on naming, I'll do the initial copy of existing annotations over to the instrumentation repo.
This PR was marked stale due to lack of activity. It will be closed in 14 days.
hey @fstab! we've finished moving the @WithSpan
instrumentation annotations to the instrumentation repo (https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation-annotations). we're keeping the existing annotations around in this repository (and still supporting them in auto-instrumentation for backwards compatibility), but would like to add new features going forwards in the new module now
I'm going to close this because now that we've moved forward with the decision to move annotations to opentelemetry-java-instrumentation
.
@fstab Could you summarize what the current status of your (great) idea with annotations for metrics is?