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

@MetricOptions to allow filtering AbstractMethodTagger beans to apply to metrics #764

Merged
merged 4 commits into from
May 28, 2024

Conversation

hrothwell
Copy link
Contributor

Follow up to #753

  • New annotation MetricOptions to hold metadata for additional information that might be used in building metrics
    • includes list of taggers for class types to apply to the published metric.
    • Note: I was unsure on how to best implement a "don't filter any beans, apply everything" so I currently opted for treating an empty array as no filtering, but I don't think this should be a final solution. Looking for more input about it.
  • Add new tests showing filtering is applied correctly
  • Was planning on updating documentation after input on how to go forward with the issue listed above and as a TODO

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?
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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

Copy link
Contributor

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

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?
Copy link
Contributor

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

@hrothwell hrothwell requested a review from graemerocher May 28, 2024 14:57
@hrothwell
Copy link
Contributor Author

Updated docs + added @Experimental per request

@graemerocher graemerocher merged commit b8df881 into micronaut-projects:5.6.x May 28, 2024
6 checks passed
@graemerocher
Copy link
Contributor

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants