-
Notifications
You must be signed in to change notification settings - Fork 889
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
Remove Span.Status per OTEP-134 #918
Conversation
1028156
to
b2657f1
Compare
@@ -371,8 +363,8 @@ Links SHOULD preserve the order in which they're set. | |||
|
|||
### Span operations | |||
|
|||
With the exception of the function to retrieve the `Span`'s `SpanContext` and | |||
recording status, none of the below may be called after the `Span` is finished. |
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.
Note: This is wrong even in the current spec before removing status.
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
@@ -35,7 +35,6 @@ status of the feature is not known. | |||
|End | + | + | + | + | + | + | + | + | | + | | |||
|End with timestamp | + | + | + | + | + | + | + | - | | + | | |||
|IsRecording | + | + | + | + | + | + | + | | | + | | |||
|Set status | + | + | + | + | + | + | + | + | | + | |
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.
We could keep this and add something like (REMOVED)
to keep track of its removal process (until all SIGs are done).
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
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.
Approving in the sense of "I (and Dynatrace) would be OK with this solution" (even without replacement, as http.status_code et al is enough for us).
Replacing status with unspecified/ok/error would also be fine with me (we would probably still not use it at Dynatrace though as we prefer more specific error information).
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed in favor of open-telemetry/oteps#136 |
Fixes #706
Changes
Remove
Span.Status
as per OTEP-134