Add error to span, deprecate status #174
Conversation
This changes the mechanism to report an error on a Span from using status.code to using error. As per the discussion in this spec issue: open-telemetry/opentelemetry-specification#706 (comment)
|
+1, but I think we also need a corresponding Spec change (which technically should precede the proto change). |
There was a problem hiding this comment.
I think this can be reprented using status in a backwards compatible way. The trace protocol is declared stable, and I think here we can easily avoid a brwaking change by only losing a little elegance. I see now, you just deprecated the old field. But I have two concerns: (1) Should error actually be a tribool success/error/no hint given? This should be discussed in spec. (2) Add explicit guidance that status MUST be set to an error code if error=true (for compatibility with consumers that don't know the new error field).
Definitely wait for spec discussion before doing anything in proto.
|
Completely agree on waiting for the spec change, hoping this PR can help driving that conversation. |
codeboten
changed the title
|
I see this was already mentioned but out of caution I want to reiterate one more time: we declared the Trace portion of the protocol stable. We will need to make sure we don't break this promise. Any implementation of v0.4.0 of the protocol will need to stay compatible (i.e. be able to interoperate) with the implementations of the future versions of the protocol. |
|
Closing this PR as Status will stay. |
This changes the mechanism to report an error on a Span from using status.code to using error. As per the discussion in this spec issue: open-telemetry/opentelemetry-specification#706 (comment)