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

@MeterTag does not work on package private method #4506

Conversation

MartinUhlen
Copy link
Contributor

Fixes NoSuchMethodException being thrown when using @MeterTag on package private method.

@shakuzen shakuzen added this to the 1.11.9 milestone Jan 17, 2024
@shakuzen shakuzen force-pushed the metertag-on-package-private-method branch from 7b63e9a to 9a370e5 Compare January 17, 2024 09:10
@shakuzen shakuzen changed the base branch from main to 1.11.x January 17, 2024 09:10
Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR complete with tests.

@shakuzen
Copy link
Member

I've rebased and targeted the pull request at the 1.11.x branch so we can merge forward to include the fix in upcoming maintenance releases for 1.11.x and 1.12.x.

@shakuzen shakuzen changed the title @MeterTag on package private method @MeterTag does not work on package private method Jan 17, 2024
@shakuzen shakuzen merged commit 4b04dea into micrometer-metrics:1.11.x Jan 17, 2024
6 checks passed
@shakuzen shakuzen changed the title @MeterTag does not work on package private method @MeterTag does not work on package private method Jan 17, 2024
@shakuzen shakuzen added module: micrometer-core An issue that is related to our core module module: micrometer-common An issue that is related to our common module labels Jan 17, 2024
@@ -89,7 +89,7 @@ public AnnotationHandler(BiConsumer<KeyValue, T> keyValueConsumer,
public void addAnnotatedParameters(T objectToModify, ProceedingJoinPoint pjp) {
try {
Method method = ((MethodSignature) pjp.getSignature()).getMethod();
method = pjp.getTarget().getClass().getMethod(method.getName(), method.getParameterTypes());
method = pjp.getTarget().getClass().getDeclaredMethod(method.getName(), method.getParameterTypes());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have broken existing support for methods that are only declared in the parent class, was it intended?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think that was intended. Could you open an issue for it? I guess we're missing a test case to cover that scenario since we didn't notice it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done: #4983

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpernetupgrade Ooops, no it wasn't intended and as @shakuzen says the lack of test coverage is at fault here.

Good found though, I'll see if I can provide a fix for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpernetupgrade @shakuzen

Please take a look at #4985

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug module: micrometer-common An issue that is related to our common module module: micrometer-core An issue that is related to our core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants