8000 Deprecate Remaining `Span` class and interface fields · Issue #10184 · getsentry/sentry-javascript · GitHub
[go: up one dir, main page]
Skip to content

Deprecate Remaining Span class and interface fields #10184

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.

Sign up for GitHub

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

Jump to bottom
Closed
Lms24 opened this issue Jan 15, 2024 · 4 comments · Fixed by #10361
Closed

Deprecate Remaining Span class and interface fields #10184

Lms24 opened this issue Jan 15, 2024 · 4 comments · Fixed by #10361
Assignees
@Lms24
Labels
Package: core Issues related to the Sentry Core SDK
Milestone
8.0.0

Comments

@Lms24
Copy link
Member
Lms24 commented Jan 15, 2024 •
edited
Loading

I realized we still have a lot of fields on the Span class and interface that need to be deprecated.

### `Span` Interface
- [x] https://github.com/getsentry/sentry-javascript/pull/10192
- [ ] https://github.com/getsentry/sentry-javascript/pull/10208
- [ ] https://github.com/getsentry/sentry-javascript/pull/10213
- [ ] https://github.com/getsentry/sentry-javascript/pull/10244
- [ ] https://github.com/getsentry/sentry-javascript/pull/10217
- [ ] https://github.com/getsentry/sentry-javascript/pull/10260
- [ ] https://github.com/getsentry/sentry-javascript/pull/10268

### Additional public `Span` class fields
- [ ] https://github.com/getsentry/sentry-javascript/pull/10187
- [x] `Span.endTimestamp` - same as Span.startTimestamp (via #10192)
- [ ] https://github.com/getsentry/sentry-javascript/pull/10197
- [ ] https://github.com/getsentry/sentry-javascript/pull/10199
- [ ] https://github.com/getsentry/sentry-javascript/pull/10189

ref #10033

### Related fields on other interfaces
- [x] https://github.com/getsentry/sentry-javascript/pull/10274
- [ ] https://github.com/getsentry/sentry-javascript/pull/10361
@Lms24 Lms24 self-assigned this Jan 15, 2024
@lforst
Copy link
Contributor
lforst commented Jan 15, 2024 •
edited
Loading
  • setHttpStatus should probably stay? It's pretty useful and doesn't hurt, unless I am missing something. can probably just go.
  • We have multiple instances where we (and our customers) set start- and endTimestamps. A getter like spanToJson doesn't help here 8000 as a migration path. @mydea Can you set the start and end timestamps on otel spans retroactively?
  • spanRecorder can likely be deprecated and be an implementation detail on the span object/class itself.

The rest looks good to me 👍

@Lms24 Lms24 mentioned this issue Jan 15, 2024
Merged
@Lms24 Lms24 added this to the 8.0.0 milestone Jan 15, 2024
@Lms24 Lms24 added the Package: core Issues related to the Sentry Core SDK label Jan 15, 2024
@Lms24
Copy link
Member Author
Lms24 commented Jan 15, 2024

We have multiple instances where we (and our customers) set start- and endTimestamps. A getter like spanToJson doesn't help here as a migration path. @mydea Can you set the start and end timestamps on otel spans retroactively?

setting them via startSpan et al. should still work I think 🤔 ending would need to happen via span.end(endTimestamp)

IdleTransaction (soon to be IdleSpan) in browser adjusts the endTimestamp. I wonder if for this it's enough to make the timestamps of Span protected (so still remove them publicly).

@mydea
Copy link
Member
mydea commented Jan 15, 2024

yeah, retroactive update of start/end time is not possible. This needs to be rewritten to buffer the endtime somewhere (?) and set it - but I think I did this here already kind of #9972, so should work, I hope..!

@Lms24 Lms24 mentioned this issue Jan 15, 2024
Merged
@mydea
Copy link
Member
mydea commented Jan 16, 2024

I will deprecate the timestamps!

@Lms24 Lms24 mentioned this issue Jan 16, 2024
Merged
Lms24 added a commit that referenced this issue Jan 16, 2024
@Lms24
chore(core): Mark Span.getSpanJSON method as internal (#10197)
6077cb9 
Clarify the API status of the `Span.getSpanJSON`
method. This method is purely purposed for internal usage and users
should not rely on it but instead on `spanToJSON`.

ref #10184
This was referenced Jan 16, 2024
Merged
Merged
Merged
Lms24 added a commit that referenced this issue Jan 17, 2024
@Lms24
feat(core): Deprecate Span.status (#10208)
64ba9ec 
This PR deprecates the `status` field on the span interface as well as
on the class. The replacements for this field are
* `span.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 status

ref #10184
This was referenced Jan 18, 2024
Merged
Merged
Merged
This was referenced Jan 26, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Lms24 Lms24

Labels
Package: core Issues related to the Sentry Core SDK
Projects
None yet
Milestone
8.0.0
3 participants
@mydea @lforst @Lms24
0