Add doc-comment for SimpleLogProcessor and BatchLogProcessor #2468
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2468 +/- ##
=====================================
Coverage 77.1% 77.1%
=====================================
Files 124 124
Lines 23021 23021
=====================================
Hits 17771 17771
Misses 5250 5250 ☔ View full report in Codecov by Sentry. |
| //! ## Types of Log Processors | ||
| //! | ||
| //! - **SimpleLogProcessor**: Forwards log records to the exporter immediately | ||
| //! after they are emitted. This processor is **synchronous** and is designed |
There was a problem hiding this comment.
not sure of the wording "synchronous" as it may not be understood.
There was a problem hiding this comment.
we'll want to mention something closely related to this -
Exporter's Export() method is invoked in the same thread as the log is emitted. If exporter is OTLP/tonic, then this can work only if the provider was constructed in a tokio context?
| //! - **SimpleLogProcessor**: Forwards log records to the exporter immediately | ||
| //! after they are emitted. This processor is **synchronous** and is designed | ||
| //! for debugging or testing purposes. It is **not suitable for production** | ||
| //! environments due to its lack of batching, performance optimizations, or support |
There was a problem hiding this comment.
the last part(support for high throughput) is redundant and maybe omitted.
| /// | ||
| /// ```rust | ||
| /// use opentelemetry_sdk::logs::{SimpleLogProcessor, LoggerProvider}; | ||
| /// use opentelemetry::global; |
| /// let exporter = InMemoryLogExporter::default(); // Replace with an actual exporter | ||
| /// let processor = BatchLogProcessor::builder(exporter) | ||
| /// .with_batch_config( | ||
| /// BatchConfigBuilder::default() |
There was a problem hiding this comment.
wondering if we really need BatchConfigBuilder ? Can we make them part of the BatchLogProcessorBuilder itself?
Not for this PR!
Changes
Inspired from SpanProcessor docs.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes