-
Notifications
You must be signed in to change notification settings - Fork 696
fix: documentation on "Well known exporters" and OTEL_TRACES_EXPORTER variable #2023
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
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.
Good catch, thanks for the fixes ✌️
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 fixing this!
- otlp | ||
- otlp_proto_grpc_span |
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.
We have proto_grpc exporter right?
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.
Good catch, that's correct, otlp_proto_grpc_span
should stay.
- otlp | ||
- otlp_proto_grpc_span |
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.
Good catch, that's correct, otlp_proto_grpc_span
should stay.
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.
Please fix the requested change ✌️
/
Actually it already says just below in the doc that otlp is an alias for otlp_proto_grpc_span but ok i can re-add it as well if you prefer. |
fix: documentation OTEL_TRACE_EXPORTER -> OTEL_TRACES_EXPORTER
b88af7c
to
1b771ee
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!
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.
@ocelotl please take a look and approve if your comments have been addressed.
Will review tonight ✌️ |
Following the auto-instrumentation README and examples, I encountered 2 inconsistencies between doc and code, so I fixed the documentation:
fix: documentation on "Well known exporters" -> zipkin doesn't exist but zipkin_json and zipkin_proto exist, etc. Thanks @ocelotl
fix: documentation env var is not OTEL_TRACE_EXPORTER but OTEL_TRACES_EXPORTER