Ignore class level AOP annotation if method level present

[quaff]

Do not advise twice

[quaff]

It seems failed test is not related to my changes.

> Task :micrometer-core:dockerTest

MongoMetricsCommandListenerTest > shouldCreateSuccessCommandMetricWithCustomSettings() FAILED
    io.micrometer.core.instrument.search.MeterNotFoundException: Unable to find a meter that matches all the requirements at once. Here's what was found:
       OK: A meter with name 'mongodb.driver.commands' was found.
       OK: A meter with type timer was found.
       FAIL: No meters have a tag 'cluster.id' with value '6454b516334ad05469170d28'. The only value found was '6454b516334ad05469170d29'.
       OK: A meter with name 'mongodb.driver.commands' has a tag 'collection' with value 'testCol'.
       OK: A meter with name 'mongodb.driver.commands' has a tag 'command' with value 'insert'.
       OK: A meter with name 'mongodb.driver.commands' has a tag 'mongoz' with value '5150'.
       OK: A meter with name 'mongodb.driver.commands' has a tag 'server.address' with value 'localhost:32782'.
       OK: A meter with name 'mongodb.driver.commands' has a tag 'status' with value 'SUCCESS'.
        at app//io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.build(MeterNotFoundException.java:245)
        at app//io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.access$100(MeterNotFoundException.java:49)
        at app//io.micrometer.core.instrument.search.MeterNotFoundException.forSearch(MeterNotFoundException.java:46)
        at app//io.micrometer.core.instrument.search.RequiredSearch.lambda$getOne$1(RequiredSearch.java:199)
        at java.base@17.0.5/java.util.Optional.orElseThrow(Optional.java:403)
        at app//io.micrometer.core.instrument.search.RequiredSearch.getOne(RequiredSearch.java:199)
        at app//io.micrometer.core.instrument.search.RequiredSearch.timer(RequiredSearch.java:120)
        at app//io.micrometer.core.instrument.binder.mongodb.MongoMetricsCommandListenerTest.shouldCreateSuccessCommandMetricWithCustomSettings(MongoMetricsCommandListenerTest.java:114)

[jonatan-ivanov]

Thank you for the PR!
Could you please tell us more about the use-case? In what situation would you have both?
(Also, there is CountedAspect)

[quaff]

Thank you for the PR! Could you please tell us more about the use-case? In what situation would you have both? (Also, there is CountedAspect)

Just like behavior of spring @transactional, class level annotation as default, method level annotation to override default, that's idiomatic.

[jonatan-ivanov]

Could you please do this for CountedAspect too?

[quaff]

Could you please do this for CountedAspect too?

CountedAspect only accept method level annotation currently, If team decide to support class level annotation, I would like to open another PR.

[github-actions]

If you would like us to look at this PR, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[quaff]

If you would like us to look at this PR, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

What?

[shakuzen]

@quaff Sorry about that. We didn't clear the waiting for feedback label after you provided feedback last and we recently enabled a bot which automatically picked up this PR because it had the waiting for feedback label. I've cleared the labels now.

[shakuzen]

CountedAspect only accept method level annotation currently, If team decide to support class level annotation, I would like to open another PR.

Support has been added there with #4758 now. Would you be willing to update this PR to include CountedAspect too? I think we're good to merge this after that. Thanks for the work on this and sorry for it taking so long to get to it.

[quaff]

CountedAspect only accept method level annotation currently, If team decide to support class level annotation, I would like to open another PR.

Support has been added there with #4758 now. Would you be willing to update this PR to include CountedAspect too? I think we're good to merge this after that. Thanks for the work on this and sorry for it taking so long to get to it.

@shakuzen done.

[shakuzen]

Thanks for the pull request and quick feedback. This will be included in the upcoming 1.13.0-M2 pre-release.

[mihalyr]

The and not @annotation(io.micrometer.core.annotation.Counted) is not a valid AspectJ pointcut syntax, the and and not are only allowed in load-time weaving in aop.xml usually and this breaks compile-time weaving as we found out in #1149 (comment)

The pointcut syntax uses && and ! symbols instead that cannot be used in aop.xml. This #5058 should fix it.

[jonatan-ivanov]

fyi: I'm backporting that change to 1.13.x: #5285

EDIT: I reverted the change, sorry for jumping too quickly: #5285 (comment)