-
Notifications
You must be signed in to change notification settings - Fork 40.8k
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 auto-configuration for ObservedAspect #35191
Conversation
Thanks, @vpavic. I don't think we should rush into this. There's some similarity with |
Thank you for the PR! I have mixed feelings about this: To Andy's point: |
The motivation behind this PR is that, from my experience, in order to have application-wide observability based on Micrometer the use of I've never used |
I agree on real-world applications will still have some use cases for |
In case we decide to move forward with this, can we also cover |
@jonatan-ivanov It's unclear to me who is your question aimed at - is it (someone from) the Spring Boot team, or are you asking me to update the PR and add registration of beans you mentioned? |
@vpavic The question is aimed at everyone who is involved in this PR (or will be). |
If the team decides so, and that helps move things forward, no worries about updating the PR. |
Hey @vpavic, |
Sure, I'll update over the next few days. |
This adds support for auto-configuring `ObservedAspect` when AspectJ is on the classpath, which enables the usage of `@Observed`.
I looked into updating this PR today, but auto-configuring additional aspects isn't as straightforward as
Therefore they seem to be targeting different auto-configurations:
|
I think Regarding |
As mentioned in #35191 (comment), I don't have experience with Judging by your comment, there seem to be even more question marks around This PR was opened as purely focused on observability, and I thought the team acknowledged that by making it a part of observability topic (#35776), but having looked into configuring those additional aspects it seems to me that the focus is now being shifted to covering all Micrometer-provided aspects (which is a broader concern than just observability). Is there anything wrong with proceeding with this PR as-is, and opening separate issues for |
I think it's a bug in the docs. This isn't an incubating aspect - it's stable.
Sure, why not, makes sense. |
This adds support for auto-configuring `ObservedAspect` when AspectJ is on the classpath, which enables the usage of `@Observed`. See gh-35191
This adds support for auto-configuring
ObservedAspect
when AspectJ is on the classpath, which enables the usage of@Observed
.Is it possible this gets considered for 3.1? I know the RC1 went out last week, but this is a change of small impact that IMO offers nice convenience for the users.
/cc @jonatan-ivanov