propagate trace processor errors to users instead of internal logging#3211
Open
svencowart wants to merge 5 commits intoopen-telemetry:mainfrom
Open
propagate trace processor errors to users instead of internal logging#3211svencowart wants to merge 5 commits intoopen-telemetry:mainfrom
svencowart wants to merge 5 commits intoopen-telemetry:mainfrom
Conversation
|
|
13 tasks
svencowart
force-pushed
the
propogate-trace-errors
branch
from
ffdb86d to
620043d
Compare
Add optional error handler callbacks to BatchSpanProcessor to provide programmatic access to background errors while maintaining internal SDK logging for observability. Changes: - Add error_handler callback to BatchSpanProcessor for background errors (span drops, export failures during timer-based flushes) - Return errors from shutdown_with_timeout when spans were dropped - Maintain all internal otel_debug/otel_warn/otel_error logging - Update BatchSpanProcessor builder with with_error_handler() method - Change dropped_spans_count to Arc<AtomicUsize> for shared ownership - Both log AND invoke error handler when errors occur This gives users explicit control over error handling via opt-in callbacks while preserving default observability through internal SDK logging. The error handler is a supplement to logging, not a replacement. Addresses error handling requirements from the OpenTelemetry specification which states that SDKs MAY expose callbacks for self-diagnostics AND that errors should be logged when they cannot be returned to the caller.
svencowart
force-pushed
the
propogate-trace-errors
branch
from
620043d to
83c695a
Compare
lalitb
reviewed
Oct 29, 2025
| // This should be changed to return Result so users can be notified of export failures. | ||
| // The OpenTelemetry spec requires on_end to be fast/non-blocking, but doesn't mandate | ||
| // a void return - returning an error doesn't violate that requirement. | ||
| // Until the trait is changed, we cannot return this error to the caller. |
Member
There was a problem hiding this comment.
Most spans end via Drop, which can't propagate errors. We should use the similar pattern as for batch processor here too.
lalitb
reviewed
Oct 29, 2025
| current_batch_size: Arc<AtomicUsize>, | ||
| max_export_batch_size: usize, | ||
| dropped_spans_count: AtomicUsize, | ||
| dropped_spans_count: Arc<AtomicUsize>, |
cijothomas
reviewed
Oct 31, 2025
cijothomas
reviewed
Oct 31, 2025
| dropped_span_count = dropped_spans, | ||
| max_queue_size = max_queue_size, | ||
| message = "Spans were dropped due to a queue being full. The count represents the total count of spans dropped in the lifetime of this BatchSpanProcessor. Consider increasing the queue size and/or decrease delay between intervals." | ||
| name: "BatchSpanProcessor.Shutdown", |
Member
There was a problem hiding this comment.
why change event name? anything wrong with the existing name? Happy to improve it.
cijothomas
reviewed
Oct 31, 2025
| message = "Spans were dropped due to a full queue. The count represents the total count of span records dropped in the lifetime of the BatchSpanProcessor. Consider increasing the queue size and/or decrease delay between intervals." | ||
| ); | ||
|
|
||
| // Also return error so user code can handle it programmatically |
Member
There was a problem hiding this comment.
this feels incorrect. This is the shutdown() call, and that method is not failing here to warrant returning an Err - having items dropped due to buffer full is not a failure of shutdown itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove internal logging of errors in span processors and tracer provider in favor of returning errors to callers and providing an error handler callback mechanism for background operations.
Changes
This gives users explicit control over error handling rather than relying on internal SDK logging that may be missed or not actionable.
Fixes #3210
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes