Skip to content
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

Deprecate Remaining Span class and interface fields #10184

Closed
14 tasks done
Tracked by #10033
Lms24 opened this issue Jan 15, 2024 · 4 comments · Fixed by #10361
Closed
14 tasks done
Tracked by #10033

Deprecate Remaining Span class and interface fields #10184

Lms24 opened this issue Jan 15, 2024 · 4 comments · Fixed by #10361
Assignees
Milestone

Comments

@Lms24
Copy link
Member

Lms24 commented Jan 15, 2024

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

Additional public Span class fields

  1. Lms24
  2. Lms24
  3. Lms24
  4. Lms24

ref #10033

Related fields on other interfaces

  1. Lms24
  2. Lms24
@Lms24 Lms24 self-assigned this Jan 15, 2024
@lforst
Copy link
Member

lforst commented Jan 15, 2024

  • 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 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
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..!

@mydea
Copy link
Member

mydea commented Jan 16, 2024

I will deprecate the timestamps!

Lms24 added a commit that referenced this issue Jan 16, 2024
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
Lms24 added a commit that referenced this issue Jan 17, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants