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
feat(core): Update span performance API names #8971
Conversation
size-limit report 📦
|
re: this, FYI, these APIs are exposed from It's easy enough for me to update my changes since I've only just started trying to use this (and also I'm only using |
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.
Changes LGTM. As for merging this:
Let's chat about it in Daily. I think it'd be reasonable to break, especially since the original APIs were out for a little over a week. If we decide to hold off, then let's still do this in v8 because I think ultimately, we want to align with the amended RFC/what we discussed yesterday.
I thought about this more, considering the current names map exactly to OTEL let's just keep them around until we properly break in v8. Better not risk it since it was added to docs. |
After chatting in our daily we decided that we're going to change the behavior for just We'll deprecate the |
As per the new changes in RFC 101 in getsentry/rfcs#113, update the span performance API names.
startActiveSpan
->startSpan
startSpan
->startInactiveSpan
https://github.com/getsentry/rfcs/blob/main/text/0101-revamping-the-sdk-performance-api.md
startActiveSpan
is deprecated, whilestartInactiveSpan
is being introduced. The breaking change is thatstartSpan
is being changed, but considering that basically no-one is using thestartSpan
API, we should be fine to break here for correctness reasons. Better break now than to have everyone refactor their code in v8.This is a breaking change, so maybe we decide against doing the rename in JavaScript - thoughts? #8970 also brought up that I forgot to export the span creator methods in browser SDKs, so perhaps we can get away with the change.