search

eclipse / microprofile-metrics

microprofile-metrics
Apache License 2.0
100 stars 66 forks source link

@Gauge not valid for fields #591

Closed janmaterne closed 4 years ago

janmaterne commented 4 years ago

The JavaDoc of the @Gauge annotation gives an example for use on a field: "Given a field annotated with {@literal @}Gauge like this...". But the annotation itself is not valid for fields: @Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })

You must add an ElementType.FIELD to that array.

jmartisk commented 4 years ago

We don't support any metric annotations on fields, apart from @Metric when used as a qualifier for @Inject. I think we rather need to remove that paragraph from the javadoc, I don't know how it got there, but nothing of that sort is in the specification or the TCK.

janmaterne commented 4 years ago

Personally I prefer Annotations on the fields and I came to this as I wanted to publish a local attribute as metric. But if neither the spec nor the TCK asks for this, I have to use a getter. It's more important that the implementation is in sync with the docs. As the remove of the javadoc is adressed in #593 we can close this issue.

jmartisk commented 4 years ago

Cool. Let's keep the issue open for now though, we usually close an issue once the PR is merged, which is not the case yet Thanks!

donbourne commented 4 years ago

We used to "support" annotating a field with a Gauge. We realized that was problematic at one point and removed that support in https://github.com/eclipse/microprofile-metrics/commit/f2212252c6c860b34c2d0a9bdd554e1d88dcf0c5#diff-bdcb2bfec2531ede80cfeac426815f4c . See the original PR here https://github.com/eclipse/microprofile-metrics/pull/112 . As mentioned, it is probably possible to re-add that, but I think the right change for right now is to finish the work that was missed in the original removal of field-level annotations for Gauge.