Uh oh!
There was an error while loading. Please reload this page.
feat(core): Deprecate span tags, data, context & setters - #10053
Conversation
| () => { | ||
| // TODO: Can we rewrite this to use attributes? | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp }); |
There was a problem hiding this comment.
cc @JonasBa we either need to move this to be an attribute (~~data) on the transaction, or find a way to set these on the surrounding scope of the transaction instead 🤔 nothing to do in this PR but when we actually remove this in v8 we need to handle this somehow.
There was a problem hiding this comment.
Ok sounds good. Thanks for the ping!
| if (this.op === 'ui.action.click') { | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| this.setTag(FINISH_REASON_TAG, this._finishReason); |
There was a problem hiding this comment.
Do we need this tag on the idle transaction in v8? @AbhiPrasad
There was a problem hiding this comment.
OK, I'll just change it to an attribute then!
| */ | ||
| export function spanToTraceContext(span: Span): TraceContext { | ||
| const { data, description, op, parent_span_id, span_id, status, tags, trace_id, origin } = span.toJSON(); | ||
| const { |
There was a problem hiding this comment.
This conflicted with some tests, but I figured it is overall safer as we assumed that this always is a Span class instance with toJSON() which may be a bit unsafe. Now we just treat this as a POJO which should be safer (and .toJSON() will actually go away for v8 as well, so we may as well already do that now)
| // Since formDataObject is not a primitive, we cannot store it on the span as attributes | ||
| // so instead we put it as extra on the scope | ||
| const scope = getCurrentScope(); | ||
| scope.setExtra('server_action_form_data', formDataObject); |
There was a problem hiding this comment.
@lforst do we need this as a nested object here? attributes cannot be nested, so going forward we need to either flatten this, if the shape is known (which I guess it is not?), or store it as context on the scope (?), or as JSON string...
| // TODO: Can we rewrite this to an attribute? Or set this on the scope? | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| sentryTransaction.setContext('trpc', trpcContext); |
There was a problem hiding this comment.
Any thoughts on this? Not sure how important that is, but I guess we need some way to access the scope of the sentryTransaction then we could just put it on there...?
| const { op, description, source, data } = parseOtelSpanDescription(otelSpan); | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| transaction.setContext('otel', { |
There was a problem hiding this comment.
I guess this will generally go away in v8, so no need to worry about this.
Which reminds me, we should also deprecate these I guess...
size-limit report 📦
|
Uh oh!
There was an error while loading. Please reload this page.
5ebab8f to
6cfe78bCompare| } else { | ||
| formDataObject[key] = '[non-string value]'; | ||
| } | ||
| span?.setAttribute( |
There was a problem hiding this comment.
After talking with @lforst, we decided to flatten this here, as the exact shape of this is not important, and thus we can more easily rewrite it to attributes!
30dc0ac to
4cb5aaaComparemydea
commented
Jan 9, 2024
I updated this to use |
Also deprecate direct access to `span.attributes` (instead use `spanGetAttributes(span)`). There are a few usages that we still need to figure out how we will replace them.
Also deprecate direct access to
span.attributes(instead usespanToJSON(span)).There are a few usages that we still need to figure out how we will replace them...!