-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Deprecate Span.status
#10208
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
Conversation
* | ||
* @deprecated Use `.setStatus` to set or update and `spanToJSON()` to read the status. | ||
*/ | ||
status?: string; |
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.
Had to re-declare status on the Span
interface because it's inherited from SpanContext
. I don't think we want to deprecate it there in v7 but tbh not sure.
Side note:
This is similar to #10189 where I realized I also didn't deprecate op
on the Span interface because of this inheritance yet. I'll tackle this in a follow-up PR though.
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.
yes, I think that's fine! I'd deprecate stuff on the Span
interface directly and leave the span context alone, it will go away "automatically" when all methods that use it are gone in v8!
And yeah, if we missed some there let's just add it, may very well be! 😅
size-limit report 📦
|
1dd0b12
to
75f1cd8
Compare
75f1cd8
to
c068724
Compare
As discovered during #10208, the `op` property of the `Span` interface was still missing a deprecation because `Span` inherited it from `SpanContext` which I missed.
This PR deprecates the
status
field on the span interface as well as on the class. The replacements for this field arespan.setStatus
to set or update a span status (this API exists on the Otel Span interface but the types don't align yet)spanToJson
to read the statusref #10184