fix: Document span.finish optional endTimestamp arg
#508
Conversation
The `span.finish()` and `transaction.finish()` methods can be given an optional `endTimestamp` value that allows for users to override the endTimestamp of a span/transaction. This is currently implemented in the JavaScript SDK https://github.com/getsentry/sentry-javascript/blob/bb6f86502e1ea551fa503136a954844f77cca322/packages/tracing/src/span.ts#L232-L234 This PR updates the develop documentation to reflect this capability.
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/sentry/develop/Cdj3FjmTNxMx4iseHnkmLvka1guD |
| - `Span.finish()` | ||
|
|
||
| - Just set `endTimestamp` to the current time (in payload `timestamp`) | ||
| - Accepts an optional `endTimestamp` to allow users to set a custom `endTimestamp` on the finished span |
There was a problem hiding this comment.
Is there any sort of validation? e.g. if the given endTimestamp if before the startTimestamp and if so, should it log an error or what?
Asking that because of getsentry/sentry-dart#676
There was a problem hiding this comment.
At the current moment there is no validation in JS or Python. We can think about adding this though? Should we add to the spec?
There was a problem hiding this comment.
Ideally yes, our PR logs that the endTimestamp is before the startTimestamp and sets the current timestamp.
If this sounds like a good approach, we could document to the spec.
There was a problem hiding this comment.
I think we should ask around. Think logging is a good idea, but not sure about setting current timestamp if there is an error.
There was a problem hiding this comment.
This is in TSC already, we can make another PR if that's the case, so we unblock this one.
The
span.finish()andtransaction.finish()methods can be given an optionalendTimestampvalue that allows for users to override the endTimestamp of a span/transaction. This is currently implemented in the JavaScript SDKhttps://github.com/getsentry/sentry-javascript/blob/bb6f86502e1ea551fa503136a954844f77cca322/packages/tracing/src/span.ts#L232-L234
This PR updates the develop documentation to reflect this capability.