feat(core): Deprecate Span.op in favor of op attribute
#10189
Conversation
| data: { | ||
| 'sentry.op': 'test op', | ||
| }, |
There was a problem hiding this comment.
I noticed that spanToJson sets the op also in data because it's saved as an attribute. Should we change this in the spanToJson implementation? I'd imagine, this would just be duplicated data in the product as we still need the top level event.op field anyway.
thoughts @mydea @AbhiPrasad?
There was a problem hiding this comment.
I would leave it in, I think - rather if we don't want to send this, we can/should remove it wherever we generate the event 🤔 eventually (after v8? let's see..) I think any special handling of op & origin should go away and they should only be kept as attributes everywhere. But for now we can just keep both I believe...?
There was a problem hiding this comment.
eventually (after v8? let's see..) I think any special handling of op & origin should go away and they should only be kept as attributes everywhere
Agreed, that makes sense to me, thanks! Ok, I'll leave it in then and adjust the tests to accomodate the new behaviour. Worth pointing out that this isn't breaking anything, it's just adding a new data item anyhow.
size-limit report 📦
|
Lms24
force-pushed
the
lms/feat-core-deprecate-span-op
branch
from
840c720 to
d65726c
Compare
| @@ -1,5 +1,6 @@ | |||
| /// <reference types="next" /> | |||
| /// <reference types="next/image-types/global" /> | |||
| /// <reference types="next/navigation-types/compat/navigation" /> | |||
There was a problem hiding this comment.
is this on purpose here? Just to clarify!
There was a problem hiding this comment.
huh no, no idea how this happened, thx for catching!
There was a problem hiding this comment.
Ah maybe because I ran the NextJS integration tests locally.
Lms24
force-pushed
the
lms/feat-core-deprecate-span-op
branch
2 times, most recently
from
cd9f4a8 to
29c0ce5
Compare
Lms24
force-pushed
the
lms/feat-core-deprecate-span-op
branch
3 times, most recently
from
9606c1d to
bfdd5a6
Compare
fix missing eslint ignore after rebase :( migration doc revert next-env.d.ts change once again fix things after rebasing :(
Lms24
force-pushed
the
lms/feat-core-deprecate-span-op
branch
from
bfdd5a6 to
7b687f6
Compare
This PR deprecates
Span.opin favour of:spanToJsonto get the span opstartSpanfunctions to set the initial span opsetAttribute('SEMANTIC_ATTRIBUTE_SENTRY_OP', ...)to update the span opGoing forward, the span op will become a semantic, Sentry-specific span attribute. With this PR we already set, get and update the attribute but still provide the deprecated
opgetter and setter for v7 backwards compatibility.Added behaviour:
spanToJsoncurrently writes attributes intospan.data, meaningsentry.opbecomes a new field on thedataobject in addition to being set on the top levelopproperty. After discussing this, it's probably okay to have this duplication at the moment. In an only-spans future, we'll likely remove the top levelopproperty all together so we should be good.The side effect is that I imagine a new
sentry.opfield could be visible in the product UI when looking at span/transaction data.ref #10184