-
Notifications
You must be signed in to change notification settings - Fork 66
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
@MetricOptions
to allow filtering AbstractMethodTagger
beans to apply to metrics
#764
@MetricOptions
to allow filtering AbstractMethodTagger
beans to apply to metrics
#764
Conversation
public @interface MetricOptions { | ||
/** | ||
* TODO if empty or null should this do nothing? no filtering happening unless something defined? but then | ||
* what if you really truly don't want anything applied? Null is not an option for default, some secondary annotation value? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can add a containers check for the annotation member and if it is empty then disable all taggers I guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow what you are referring to, but it sounds like disable all taggers if taggers
is empty?
The idea I am going back and forth on is how do we differentiate "use all taggers" without needing to specify all additional taggers? Ideally what I was hoping for was something like:
taggers == null
--> no filtering done, all taggers get appliedf
taggers == empty array
--> filtering is enabled, this filters out all taggers effectively disabling them
taggers == populated array
--> filters just to taggers in populated array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a helper annotation value as a different option also, not sure what is most ideal really
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok looks fine can you update the javadoc and remove the TODO
micrometer-core/src/main/java/io/micronaut/configuration/metrics/annotation/MetricOptions.java
Show resolved
Hide resolved
public @interface MetricOptions { | ||
/** | ||
* TODO if empty or null should this do nothing? no filtering happening unless something defined? but then | ||
* what if you really truly don't want anything applied? Null is not an option for default, some secondary annotation value? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok looks fine can you update the javadoc and remove the TODO
Updated docs + added |
Thanks for the contribution! |
Follow up to #753
MetricOptions
to hold metadata for additional information that might be used in building metricstaggers
for class types to apply to the published metric.TODO