Skip to content
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

Add more annotations to remove cross-cutting concerns #2632

Open
dbnex14 opened this issue Jun 2, 2021 · 12 comments
Open

Add more annotations to remove cross-cutting concerns #2632

dbnex14 opened this issue Jun 2, 2021 · 12 comments
Labels
enhancement A general enhancement

Comments

@dbnex14
Copy link

dbnex14 commented Jun 2, 2021

Jackarta Microprofile has a number of metrics annotations, not only @timed and @counted.
It would be nice to have more annotations like in Microprofile to make code cleaner and get more metrics.

And it would be nice to have better documentation of same. I have read lots of articles but all of theme were lacking information on how to remove cross-cutting concerns using annotations. I believe, I managed to only find information on this for @timed but from what I read, there are also counters, gauges and metered annotations?

It seem to be great functionality to have, no doubt in that and it would help to have good examples to help everyone move into the direction of using micrometer for their projects and embrace the technology.

@checketts
Copy link
Contributor

Thanks for opening an issue! Could you please list the specific annotations and use cases you have in mind?

@dbnex14
Copy link
Author

dbnex14 commented Jun 3, 2021

This is from their github. The nice thing is that all of these are annotations so you dont have to polute the code with various Metrics.counted, Metrics.timed calls plust there is many more annotations with their descriptions

https://github.com/eclipse/microprofile-metrics/blob/master/spec/src/main/asciidoc/app-programming-model.adoc

Annotation | Applies to | Description | Default Unit
-- | -- | -- | --
@Counted | M, C, T | Denotes a counter, which counts the invocations of the annotated object. | MetricUnits.NONE
@ConcurrentGauge | M, C, T | Denotes a gauge which counts the parallel invocations of the annotated object. | MetricUnits.NONE
@Gauge | M | Denotes a gauge, which samples the value of the annotated object. | no default, must be supplied by the user
@Metered | M, C, T | Denotes a meter, which tracks the frequency of invocations of the annotated object. | MetricUnits.PER_SECOND
@Metric | F, P | An annotation that contains the metadata information when requesting a metric to be injected. | MetricUnits.NONE
@Timed | M, C, T | Denotes a timer, which tracks duration of the annotated object. | MetricUnits.NANOSECONDS
@SimplyTimed | M, C, T | Denotes a simple timer, which tracks duration and invocations of the annotated object. | MetricUnits.NANOSECONDS


Annotation | Description | Default
-- | -- | --
@RegistryType | Qualifies the scope of Metric Registry to inject when injecting a MetricRegistry. | application (scope)


@ebullient
Copy link
Contributor

ebullient commented Jun 10, 2021

Note that MP Metrics is based on Dropwizard, where Micrometer is not, so the nuance surrounding @Metered, for example, is lost.

Based on adaptation of MP Metrics to Micrometer for Quarkus:

  • @SimplyTimed and @Timed both map to the @Timed annotation.

  • For @ConcurrentGauge, you should look at long task timers (which use the @Timed annotation)

  • @Metered is covered by @Counted (where the registry handles calculating the rate of change or not)

  • While @Metric effectively constructs and injects some metric types, I would never personally recommend its use. Same with @Gauge.

@dbnex14
Copy link
Author

dbnex14 commented Jun 14, 2021

Note that MP Metrics is based on Dropwizard, where Micrometer is not, so the nuance surrounding @metered, for example, is lost.

Based on adaptation of MP Metrics to Micrometer for Quarkus:

  • @SimplyTimed and @timed both map to the @timed annotation.
  • For @ConcurrentGauge, you should look at long task timers (which use the @timed annotation)
  • @metered is covered by @counted (where the registry handles calculating the rate of change or not)
  • While @Metric effectively constructs and injects some metric types, I would never personally recommend its use. Same with @Gauge.

... none of which is well and clearly documented and supported by working examples demonstrating how to annotate methods to get above metrics without introducing crosscutting concerns all over the code. Even in case of the only one, which is @timed, which has some partial but pure documentation/samples. I hope #4623 will at least solve that.

Here is an example how to address all these in Microprofile https://www.tomitribe.com/blog/getting-started-with-microprofile-metrics/
I never needed to go beyond that single one post to figure out how these are done in Microprofile. We finished the change for that project in a day. Our other project, using Micrometer is going on for a month now as we still have no solution.

@ebullient
Copy link
Contributor

ebullient commented Jun 14, 2021

It's because Micrometer does not make the annotations work, at all. It's up to the framework that uses micrometer. Spring glues the annotations into it's dependency injection framework in one way (using aspects). Quarkus does it using CDI. Both are outside the scope of the Micrometer docs.

@ebullient
Copy link
Contributor

ebullient commented Jun 14, 2021

There is no difference between specifying an annotation and calling regsitry.whatever. There are plenty of other patterns that can be used if you want to consolidate/constrain how and where the registry is referenced, or how or when tags are defined.. but there is nothing "better" about using annotations for metrics.

</rant> . Sorry. ;) I see this stated often. I think you miss functionality when you force everything through the narrow aperture of annotations for no real difference in terms of code. Happy to show some other examples of how to reduce "contaimination" while collecting robust, meaningful metrics.

@checketts
Copy link
Contributor

Our other project, using Micrometer is going on for a month now as we still have no solution.

@db I'm sorry you are struggling with Micrometer, unfortunately I haven't see a specific request aside from frustration with the documentation.

Are you perhaps struggling with trying to make Micrometer 'behave like my old thing' and the trouble comes from not having a 1 to 1 migration approach?

For example there is no concept of '@metered' in Micrometer since many backends calculate the rate via a query in the metrics storage (ie Prometheus: rate(my_counter[5m])). So it doesn't make sense to just add annotations that apply in the case of one backend.

By the way, feel free to ask usage questions on Stackoverflow and link them here. There are lots of folks (including myself) that try to answer those questions.

@dbnex14
Copy link
Author

dbnex14 commented Jun 17, 2021

Our other project, using Micrometer is going on for a month now as we still have no solution.

@db I'm sorry you are struggling with Micrometer, unfortunately I haven't see a specific request aside from frustration with the documentation.

Are you perhaps struggling with trying to make Micrometer 'behave like my old thing' and the trouble comes from not having a 1 to 1 migration approach?

For example there is no concept of '@metered' in Micrometer since many backends calculate the rate via a query in the metrics storage (ie Prometheus: rate(my_counter[5m])). So it doesn't make sense to just add annotations that apply in the case of one backend.

By the way, feel free to ask usage questions on Stackoverflow and link them here. There are lots of folks (including myself) that try to answer those questions.

Hi and thanks for getting back to me.
No, I would not say it has to do anything with "behave like my old thing" but rather with the documentation.
For example, the documentation says "Micrometer packs with a supported set of Meter primitives including: Timer, Counter, Gauge, DistributionSummary, LongTaskTimer, FunctionCounter, FunctionTimer, and TimeGauge."
Yet further down, there is only annotation for @timed annotation and no annotations for any of the others. I did find online a poor explanation how to use @counted annotation which I could not make to work either.
So, I assume that @timed is related to Timer meter primitive
Similarly that @counted is related to Counter meter primitive

The outcome is that is is not clear how to use:

  • @timed - this one is explained only but it is still not clear to me how to use various settings for it. I would says it is OK, I can deal with that but than, and if I understand correctly Erin is saying that Timer is used for most of the ones I listed above for Microprofile. But I dont see example how to do that anywhere. So, it would help to make better examples showing various settings you can use with it (e.g, somewhere I saw an example where you can tab it by region name, say "east coast") but I understand other options are available as well. Anyways, I am happy with what I can find online for this annotations but it clearly could be better documented
  • @counted - this one is not explained at all in the documentation. I did find some post online about it, but it was not working so I gave up
  • Other annotations - they seem to not exist in Micrometer. But if I understand correctly Erin's response, they do but through use of @timed and @counted (I guess with some specific settings which again are nowhere to find)

So, what I am asking is simply to provide better examples and document for what you support, not necessarily to match what Microprofile is doing. But based on Erin's very 1st reply above, you do support all of these through @timed and @counted, it is just that it is not clear from the documentation how to do that.

But again, this is not request to match what they support at Microprofile but rather to have better examples how to use what Micrometer already supports. Because right now, it feels like smell to see that "Micrometer packs with a supported set of Meter primitives including: Timer, Counter, Gauge, DistributionSummary, LongTaskTimer, FunctionCounter, FunctionTimer, and TimeGauge.", yet to see only one single @timed annotation documented.

And why I talk about annotations? To avoid having crosscutting concerns through out the code. This is why I used Microprofile example above because they really clearly show that and several blogs explain it very well. I am not finding similar for MIcrometer.

@reinhapa
Copy link

And why I talk about annotations? To avoid having crosscutting concerns through out the code. This is why I used Microprofile example above because they really clearly show that and several blogs explain it very well. I am not finding similar for MIcrometer.

@dbnex14 would it not be an option to for Micrometer to implement the MicroProfile in a sense as a logging implementer implements SLF4J as example or am I totally wrong?

@ebullient
Copy link
Contributor

I will repeat: you can do all of these with annotations, but it depends on the framework (Quarkus, Spring, CDI-provider x) to make the annotations do something.

As someone that learned micrometer on their own, and wrote the Quarkus extension supporting it, I can say that all of the documentation is there. If you aren't using Quarkus or Spring, you may need to either implement the CDI binding yourself, or help the application framework get there (PR).

There is no magic for all frameworks.. something has to provide the binding. Micrometer doesn't do that automatically. That's the correct choice, as two frameworks that use Micrometer (Spring and Quarkus) use two different injection patterns for annotations (AspectJ and CDI).

I have a few presentations that may help contextualize things, but all of this information is in the micrometer docs:

https://www.ebullient.dev/2021/06/10/devnation-know-your-app.html

It is mostly possible to adapt MP Metrics onto Micrometer, but not completely. Quarkus does build-time BCI, which lets us cover some use cases that others might not be able to. I am pretty sure the most problematic ones have been deprecated/removed.

@reinhapa
Copy link

I will repeat: you can do all of these with annotations, but it depends on the framework (Quarkus, Spring, CDI-provider x) to make the annotations do something.

I see it the same way. Just before I jump into implementing the according CDI extention in my case, I wanted to be checking if there is any of this available somewhere already...

@ebullient
Copy link
Contributor

ebullient commented Sep 12, 2023

I see it the same way. Just before I jump into implementing the according CDI extention in my case, I wanted to be checking if there is any of this available somewhere already...

Will be framework dependent. But for inspiration (and cribbing):

The really important bit about micrometer is the notion that the framework (and libraries) do most of the instrumentation for you (and there is much less in your application generally).

Quarkus does its own instrumentation for a lot of things, Vert.x has a separate layer for some things (not used by Quarkus), all of the various binders instrument things, which generally reduces the need for application-level timers and counters.

Maybe this helps with the separation of concepts (where the framework does a lot for you, but there is some context for when you might want to add your own):

From an "application metrics that aren't littered everyplace" perspective, I built this:

HTH

@marcingrzejszczak marcingrzejszczak added the enhancement A general enhancement label Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants