docs: Add docs for new performance APIs #10017
Conversation
| | `parentSpanId` | Same | | ||
| | `status` | TODO: new signature | | ||
| | `sampled` | `spanContext().traceFlags` | | ||
| | `startTimestamp` | `startTime` - note that this has a different format! | |
There was a problem hiding this comment.
This will be sad, because this is not nice in OTEL, but I guess we have to do it 😬 they use a different format for timestamps :(
There was a problem hiding this comment.
l: Can we add either a link explaining the new format or a 1-liner explaining the difference?
| | `end()` | Same | | ||
| | `setTag()` | `setAttribute()` | | ||
| | `setData()` | `setAttribute()` | | ||
| | `setStatus()` | TODO: new signature | |
There was a problem hiding this comment.
we need a long term strategy for status, might be notable to write about in it's own section.
There was a problem hiding this comment.
yeah, I'll go through all the "??" sections here tomorrow a bit 😓
docs/v8-new-performance-apis.md
Outdated
| | `startChild()` | Call `Sentry.startSpan()` independently | | ||
| | `isSuccess()` | ??? | | ||
| | `toTraceparent()` | ??? | | ||
| | `toContext()` | ??? | |
There was a problem hiding this comment.
Looking through the codebase, I can't find a single occurrence of this (toContext()) - do we need this anymore at all?
There was a problem hiding this comment.
Maybe downstream SDKs depend on this but I can't imagine a situation where this would be needed. Let's trash it.
docs/v8-new-performance-apis.md
Outdated
| | `isSuccess()` | ??? | | ||
| | `toTraceparent()` | ??? | | ||
| | `toContext()` | ??? | | ||
| | `updateWithContext()` | ??? | |
There was a problem hiding this comment.
Couldn't find any occurrence of updateWithContex(), so I think we can drop this?
docs/v8-new-performance-apis.md
Outdated
| | `setHttpStatus()` | ??? | | ||
| | `setName()` | TODO: `updateName()` | | ||
| | `startChild()` | Call `Sentry.startSpan()` independently | | ||
| | `isSuccess()` | ??? | |
There was a problem hiding this comment.
isSuccess() is not used by us (and really only checks the status), so I think we can deprecate/remove this.
docs/v8-new-performance-apis.md
Outdated
| | Old name | Replace with | | ||
| | --------------------------- | ----------------------------------------------------- | | ||
| | `name` | Same | | ||
| | `trimEnd` | Removed | |
There was a problem hiding this comment.
What should we do about trimEnd? 🤔 should we keep this somehow, or rework how this API works? I think with only spans this maybe also makes less sense overall, but not sure...?
There was a problem hiding this comment.
trimEnd is just about idleTransactions. @mydea is there a way in the new performance API to inspect the child spans for a particular span and set its endTimestamp to the highest timestamp of the child spans? If so, we can drop this.
There was a problem hiding this comment.
I think as we keep a list of the child spans, we should be able to keep the highest end timestamp and use this for idleSpan.end(highestTimestamp) 🤔 So I'd say this should be possible!
| | `parentSpanId` | Same | | ||
| | `status` | TODO: new signature | | ||
| | `sampled` | `spanContext().traceFlags` | | ||
| | `startTimestamp` | `startTime` - note that this has a different format! | |
There was a problem hiding this comment.
l: Can we add either a link explaining the new format or a 1-liner explaining the difference?
| Currently, `tracesSampler()` can receive an arbitrary `SamplingContext` passed as argument. | ||
| While this is not defined anywhere in detail, the shape of this context will change in v8. Going forward, this will mostly receive the attributes of the span, as well as some other relevant data of the span. Some properties we used to (sometimes) pass there, like `req` for node-based SDKs or `location` for browser tracing, will not be passed anymore. | ||
|
|
||
| ### No more `undefined` spans |
There was a problem hiding this comment.
not new but just wanted to say again: big DX improvement!
|
whoops forgot an overall comment: Take my "l:" suggestions as completely optional, depending on how important you think it is to be explicit here. IMO this mainly comes down to the target audience which iiuc is mainly internal comms. We should be very explicit though when we're using or linking to this doc in the official migration guide. |
Co-authored-by: Lukas Stracke <[email protected]>
mydea
force-pushed
the
fn/docs-new-performance-apis
branch
from
76f66e5 to
ac0fca5
Compare
This is a WIP document about the new performance APIs. I'm sure I forgot a bunch of things, but it's a start at least...!
View rendered