You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Differentiate between internal Event and envelope Event types #10222
With our Span/Transcation API changes, the Event types are diverging between what properties of an event we can use and access while the event is processed in the SDK vs. what data is sent to Sentry as an EventItem in an envelope.
This was always "problematic" in the sense that e.g. serialized spans would never have methods although the EventItem type permits that. However, it has only become noticeable with us deprecating (and in v8 removing) properties from event payloads like for example a Span.
A concrete place where this is now causing difficulties for us are the Playwright browser integration tests. Here we assume that when we intercept an event envelope, the payload will be of type Event. However, in reality, it's not and functions like spanToJSON can't work correctly because they require methods on the span and not just a serialized and re-parsed JSON object. This leads to the weird situation that:
Accessing e.g. span.op makes our linter (and VSCode) complain because according to Event['spans'], op is deprecated
In reality, span.op will continue to be sent as long as we don't fully drop it in favor of the span.attributes['sentry.op'] property.
Using spanToJSON(span) will return an empty object instead of the JSON object because span has no getSpanJSON method.
In v8, span.op will be removed according to the type, but in fact still be present in the envelope.
Note: span.op serves as a good example but is not the only affected field.
More evidence that we need the SerializedEvent type:
We have integration tests wrongfully using the Event type. Whenever we check for event.spans' properties, TS will suggest and check with the wrong types. So for instance, we had a test that tried to check for a spanId, parentSpanId match and both times we used the camelCase syntax vs. the snake_case one. Ironically, the tests passed because undefined === undefined 😭
// wrong but passesconstrootSpanId=transaction?.contexts?.trace?.spanId;expect(transaction.spans?.[0]).toEqual(rootSpanId);// correct but TS complainsconstrootSpanId=transaction?.contexts?.trace?.span_id;expect(transaction.spans?.[0]).toEqual(root_span_id);
Activity
Lms24 commentedon Jan 18, 2024
#10240 adds the type, switching to it from
Eventcan only happen in v8Lms24 commentedon Jan 18, 2024
More evidence that we need the
SerializedEventtype:We have integration tests wrongfully using the
Eventtype. Whenever we check forevent.spans' properties, TS will suggest and check with the wrong types. So for instance, we had a test that tried to check for a spanId, parentSpanId match and both times we used the camelCase syntax vs. the snake_case one. Ironically, the tests passed becauseundefined === undefined😭lforst commentedon Nov 11, 2024
@Lms24 I would err on closing this because the spans type is already
SpanJSON. Do we still have a concrete need for this?Lms24 commentedon Nov 11, 2024
yup, I think we fixed this in v8 implicitly but forgot to close this. Closing now!