-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
Description
This issue for now loosely collects some implications we'll sooner or later have to deal with in the SDK when it is configured to send span envelopes instead of transactions. Individual points mentioned here should be extracted and discussed further in concrete issues once we tackle them.
[API breaking] beforeSendTransaction
This optional callback will not be called in span streaming mode. This is logical but means that this hook must be removed in a future SDK version where span streaming becomes the (only) default.
[Behaviour breaking] Event processors
Event processors will not be called for spans. Since transactions were simply a type of event
, they were passed into event processors and users could distinguish them by event.type
. Spans are no longer events (good!), so they also shouldn't go through the same API.
The issue here is that users can register event processors. So anyone opting into span streaming needs to be aware that event processors will no longer apply to (root) spans. Regular users likely don't do this a lot but users writing their own instrumentations most likely do.
Once span streaming becomes the default, this will be a behaviour breaking change.
We have a couple of options for a migration path:
- Expose something like an
addSpanProcessor
API (naming collision with OTel!!) - Avoid introducing a replacement but use a client hook instead. This is good enough for our internal usage (we actually use this quite a lot already) and quite possibly still adequate for 3rd party instrumentation authors
- Redirect users to use
beforeSendSpan
as much as possible (for regular users)- This does not cover the case where anyone would temporaroly add a a scoped event processor. How to handle this case?
[Behaviour breaking] Integration.(pre)processEvent
different API, same problem as event processors
[Potentially API breaking] beforeSendSpan
The beforeSendSpan
callback is currently invoked with a serialized SpanJSON
format (i.e. the format we send spans in transaction envelops or standalone span envelopes). In span streaming mode, spans must still be passed through this hook. So while it is opt in, this means that spans must be converted into the current SpanJSON
format and then (along with any modifications made in the hook) converted into the new SpanV2JSON
(name TBD) format. Performance-wise, this isn't great but fine for the moment.
Once span streaming becomes the default we need to decide if we continue to live with the hassle of double conversion and its performance implications or if we break the beforeSendSpan
callback signature.