-
Notifications
You must be signed in to change notification settings - Fork 437
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
Rewrite readme page for opentelemetry crate #1586
Rewrite readme page for opentelemetry crate #1586
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1586 +/- ##
=====================================
Coverage 67.3% 67.3%
=====================================
Files 138 138
Lines 19355 19355
=====================================
Hits 13029 13029
Misses 6326 6326 ☔ View full report in Codecov by Sentry. |
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.
Looks good overall! Small request.
also aligned with the design philosophy of existing telemetry solutions in the | ||
Rust ecosystem, like `tracing` or `log`, where these crates only offer a facade | ||
and the actual functionality is enabled through additional crates. | ||
|
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 would be good to add a reference to the registry https://opentelemetry.io/ecosystem/registry/?language=rust for Exporters, Propagator, Instrumentation, and more.
I took the liberty to add the missing elements this morning open-telemetry/opentelemetry.io#4062
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!
I am going to skip that suggestion in this PR, and add it next. I am still thinking what is the best way to ensure success - simply linking to registry won't necessarily help much unless the user is already familiar with OTel space.
Will come back to this.
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.
LGTM. nicely done.
https://crates.io/crates/opentelemetry The description shown in the crates page is very generic, so this PR is trying to make it crate specific. Not adding examples/getting-started, as it is probably better to stay in docs.rs.
If the direction is good, will fix every other crate (over time...)