-
Notifications
You must be signed in to change notification settings - Fork 505
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 stdout exporter example for Logs. #1072
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #1072 +/- ##
=====================================
Coverage 50.9% 50.9%
=====================================
Files 165 165
Lines 19689 19689
=====================================
Hits 10025 10025
Misses 9664 9664 ☔ View full report in Codecov by Sentry. |
let mut span = tracer.start("test_span"); | ||
span.set_attribute(KeyValue::new("test_key", "test_value")); | ||
let span_active = mark_span_as_active(span); | ||
let log_record = LogRecordBuilder::new() |
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.
I think we should not be showing this, as this is never intended for end-users to use like this. This could be part of a tutorial for "how to write own appenders", or shown with existing log api like in this PR: #1071
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.
Yes agree, but I think it is still helpful to have a vendor neutral example (with no external dependencies) using only core API and SDK - which would be useful to both - external developers who want to (somehow) use only the core api and sdk in their application, and the developers creating their own appenders/subscriber. We can add a disclaimer as comment for LogRecordBuilder
that it is not recommended for front facing API if that helps.
Said that, I don't have strong opinion, and can close this PR if we all agree this not to be good showcase example :)
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.
Options:
- If the intended audience is authors of new appenders, cover it in the docs, no need of example.
- Keep this, but explicitly warn at the top saying this is not for end users to call directly. They should use the existing Log APIs. (eg: slog,log,tracing...)
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.
I'd go for option 1 as that seems least confusing for lower context people.
Closing this PR- as discussed, it is better to add documentation for Appender devs, instead of having a example. |
Design discussion issue (if applicable) #
Changes
Add the stdout exporter example for Logs. The example will
This will export the Span, along with the Log. The Log will have the trace_id and span_id of currently active span.
Output example:
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes