-
Notifications
You must be signed in to change notification settings - Fork 41.5k
Rename ConditionalOnEnabledTracing to ConditionalOnEnabledTracingExport #47029
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
Conversation
bc5bbd3
to
2550efd
Compare
I think, in addition to renaming the annotation, it is also worth renaming the global property from Also, |
2550efd
to
935bde3
Compare
@nosan Thanks for the feedback! I’ll go ahead and make the suggested changes |
935bde3
to
0d1016c
Compare
@nosan, thank you for helping out here. Can you please make it clear to contributors, particularly when you're suggesting changes that go beyond the initial scope of an issue, that you aren't a member of the core team? Hopefully no wasted effort in this case (WDYT, @mhalbritter?), but I'd like to avoid the possibility in the future so that a contributor can choose to wait for a member of the core team to +1 a suggestion before they spend time on it. |
Agreed.
IMO, I don't think they go beyond the initial scope of the issue. This will help maintain consistency between export logging and tracing. In any case, I should have waited for approval from a core team member before suggesting any changes in that direction. |
0d1016c
to
158c395
Compare
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.
Thanks for the PR! I just have one minor comment.
...g/springframework/boot/micrometer/tracing/autoconfigure/OnEnabledTracingExportCondition.java
Outdated
Show resolved
Hide resolved
- Rename @ConditionalOnEnabledTracing to @ConditionalOnEnabledTracingExport - Rename global property from management.tracing.enabled to management.tracing.export.enabled - Update additional-spring-configuration-metadata.json accordingly - Rename OnEnabledTracingCondition to OnEnabledTracingExportCondition Signed-off-by: Maziyar Bahramian <maziyar.bahramian@gmail.com>
158c395
to
22220a8
Compare
See gh-47029 Signed-off-by: Maziyar Bahramian <maziyar.bahramian@gmail.com>
Thanks @maziyarbahramian ! |
And thanks @nosan for review! |
This PR addresses #47025.
As discussed in #46166, the current name @ConditionalOnEnabledTracing is misleading since it only controls tracing export, not all tracing-related beans.
This PR renames it to @ConditionalOnEnabledTracingExport to better reflect its purpose. All related references have been updated accordingly.
Changes