-
Notifications
You must be signed in to change notification settings - Fork 246
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
feat: add spans to Trace::ExportError #1582
Conversation
attr_reader :spans | ||
|
||
def initialize(spans) |
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'd be useful to provide documentation for the spans
arg to initialize
and the type of the spans
attribute.
Proving to myself that you can do instance vars + attr_reader on an exception
|
@@ -183,35 +183,32 @@ def reset_on_fork(restart_thread: true) | |||
OpenTelemetry.handle_error(exception: e, message: 'unexpected error in BatchSpanProcessor#reset_on_fork') | |||
end | |||
|
|||
def export_batch(batch, timeout: @exporter_timeout_seconds) | |||
def export_batch(span_array, timeout: @exporter_timeout_seconds) | |||
batch = span_array.map(&:to_span_data) |
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.
✅ calling to_span_data here instead of elsewhere in bsp
✅ no longer mutating span_array
so that we can pass an array of Span
s to report_result
This adds a
spans
attribute toSDK::Trace::ExportError
, containing the array of spans that could not be exported. It refactors the BSP a little to ensure an array ofSpan
rather than (sometimes) an array ofSpanData
is wrapped by the exception.The intent is to allow a pluggable error handler to recognize the error type, retrieve the array of spans, and (for example) report more detailed information (such as the trace IDs of the spans that are dropped).
Example: