-
Notifications
You must be signed in to change notification settings - Fork 41.1k
Improve documentation of tracing dependencies and configuration properties #34991
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
Document which property should contain the Zipkin URL
@@ -95,6 +95,7 @@ All tracer implementations need the `org.springframework.boot:spring-boot-starte | |||
|
|||
* `io.micrometer:micrometer-tracing-bridge-brave` - which is needed to bridge the Micrometer Observation API to Brave. | |||
* `io.zipkin.reporter2:zipkin-reporter-brave` - which is needed to report traces to Zipkin. | |||
* Point the property `management.zipkin.tracing.endpoint` to the Zipkin server |
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.
Each item in the list is a dependency so I don't think this is quite the right place to draw attention to the management.zipkin.tracing.endpoint
property. If we add something for the Zipkin property, we should also do the same for the management.wavefront.uri
property.
I wonder if we'd be better restructuring Tracer Implementations
to have Zipkin
and Wavefront
subsections. We could then mention the property in their respective sections and list the two dependency combinations that each supports as well.
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.
Hey @wilkinsona
I tried to restructure the subsection the way you described.
Does that match your expectations?
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 changes, @RobertRad. Unfortunately, now that I've seen them, I've realised that what I proposed isn't quite right as Zipkin and Wavefront are not tracer implementations. They're tracing systems to which a tracer exports traces. I'll have give this one some more thought.
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 @wilkinsona
I just wonder, how the management.zipkin.tracing.endpoint
property is now documented?
I had a hard time getting tracing to work with Spring Boot 3 and needed to go through the Spring source code, to find out that property.
That was the reason, why I started this pull request.
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.
It's listed in the appendix. It will also be offered by auto-completion in your IDE if you have Spring Boot tooling enabled/installed.
@RobertRad Thank you very much for making your first contribution to Spring Boot. I didn't want to waste more of your time so I went with an amended version of your first suggestion in the end. I've mentioned the configuration properties for Wavefront and Zipkin in each section where they're relevant. There's a bit of repetition but I think that's unavoidable without a much bigger overhaul of the documentation. |
Document which property should contain the Zipkin URL.