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

API: Describe Status class #59

Closed
SergeyKanzhelev opened this issue Jun 3, 2019 · 10 comments · Fixed by #150
Closed

API: Describe Status class #59

SergeyKanzhelev opened this issue Jun 3, 2019 · 10 comments · Fixed by #150
Assignees
Labels
area:api Cross language API specification issue

Comments

@SergeyKanzhelev
Copy link
Member

Describe Status class and canonical codes

@SergeyKanzhelev SergeyKanzhelev added the area:api Cross language API specification issue label Jun 3, 2019
@SergeyKanzhelev SergeyKanzhelev added this to the API complete milestone Jun 3, 2019
@tedsuo tedsuo self-assigned this Jun 7, 2019
@rochdev
Copy link
Member

rochdev commented Jun 17, 2019

As described in open-telemetry/opentelemetry-js#31 (comment), I'm not a fan of this concept in general. It's basically impossible to have a range of values to cover every use case. There is also not enough information included to actually understand the source of the error if any.

I think it would be more flexible to have an error code as an arbitrary string, a message, and a stack trace if it applies. This would provide a lot more flexibility and details about the source of any error.

@SergeyKanzhelev
Copy link
Member Author

@rochdev can you please create a separate issue for this and we will mark it with the next milestone for the discussion

@draffensperger
Copy link

IMO one of the goals here is to try to standardize the different status codes for RPC transports (gRPC, HTTP, Thrift, etc.) so that trace analysis and visualization tools could have a standardized least-common-denominator concept of status to know that e.g. something is a user error vs. a server timeout vs. a permissions error even if it doesn't know the fine-grained original status reason.

Would it work for the different HTTP transports to have standardized span labels for their specific status codes? OpenCensus used that approach, see Mapping from HTTP status codes to Trace status codes and HTTP Span Attributes. (Note those are OpenTelemetry spec links but seem copied from OpenCensus and are marked WIP.)

And for the stack trace, could we just have a stack trace property of a Span, which then allows associating stack traces even to successful spans? I think Status does have a full description field so that at least is included there I believe.

@SergeyKanzhelev
Copy link
Member Author

@draffensperger I'm sorry I lost the sense of what you are proposing to change. In any case, this issue is to describe what was proposed in Java specs so we will have a baseline to start discussions from.

@tedsuo do you think you gonna finish with it soon?

@rochdev
Copy link
Member

rochdev commented Jun 18, 2019

@SergeyKanzhelev From the description of @draffensperger I think this may be useful in a few contexts. However, the most important error code (500 Internal Server Error) doesn't seem to be included. Which code should be used for exceptions?

@tedsuo
Copy link
Contributor

tedsuo commented Jun 19, 2019

Throwing in the towel on this one! Apologies, I'm unavailable for the next two weeks.

@carlosalberto
Copy link
Contributor

I can take this one.

@carlosalberto carlosalberto self-assigned this Jun 19, 2019
@SergeyKanzhelev
Copy link
Member Author

@rochdev good point that is was missing from the mapping table. Must be "Unknown" as it is an unknown error on server side.

@draffensperger
Copy link

I noticed there is also an INTERNAL status code. Could that be the right status for an internal server error?

@SergeyKanzhelev
Copy link
Member Author

@draffensperger 500 is about the error you simply don't know details for. You still made a successful communication. Internal is when library crashed attempting to establish communication or parse the response. In http world it will most probably be one of client library exceptions, not a status code based one.

TuckTuckFloof pushed a commit to TuckTuckFloof/opentelemetry-specification that referenced this issue Oct 15, 2020
* Add varint files

* Fill out variant backport

* Add type_pack_element

* Add variant_size

* Add variant_alternative

* Fill out variant

* Fill out variant backport

* Fill out variant back port

* Fill out variant backport

* Fill out variant

* Fill out variant

* Add variant test

* Add get tests

* Add visit test

* Add more tests

* Fix variant tests

* Rename

* Reformat

* Add copyrights

* Add more variant tests

* Reformat

* Add more tests

* Fix for gcc-4.8

* Fix gcc-4.8 issue

* Rename macro

* Handle bools better
rockb1017 pushed a commit to rockb1017/opentelemetry-specification that referenced this issue Nov 18, 2021
…owners

Improve maintainers and approvers rules
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 21, 2024
* Add OTLP Trace Data Format specification

This is a continuation of OTLP RFC proposal open-telemetry/oteps#35

This change defines the data format used by Span and Resource messages.

The data format is a result of research of prior art (primarily OpenCensus and Jaeger),
as well as experimentation and benchmarking done as part of OTLP RFC proposal.

Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto
(use `make benchmark-encoding` target). Benchmarking shows that depending on the payload
composition this data format is about 4x-5x faster in encoding and 2x-3x faster in
decoding equivalent data compared to OpenCensus data format (all benchmarks in Go).

Notable differences from OpenCensus:

- Attribute key/value pairs are represented as a list rather than as a map.
  This results in significant performance gains and at the same time changes
  the semantic of attributes because now it is possible to have multiple attributes
  with the same key. This is also in-line with Jaeger's tags representation.

- Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in
  significant performance improvements for certain payload compositions (e.g. lots of
  TimedEvents).

- Resource labels use the same data type as Span attributes which now allows
  to have labels with other data types (OpenCensus only allowed strings).

* Address review comments

* Add protobuf type qualifier to pre-formatted blocks

* Add a note about future goals for the protocol

* Address review comments

* Make sure all field comments start with field name

* Change status to approved

* Clarify start and end time expectations.

* Address Sergey's comments

* Remove string length requirement

* Add StatusCode enum
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 23, 2024
* Add OTLP Trace Data Format specification

This is a continuation of OTLP RFC proposal open-telemetry/oteps#35

This change defines the data format used by Span and Resource messages.

The data format is a result of research of prior art (primarily OpenCensus and Jaeger),
as well as experimentation and benchmarking done as part of OTLP RFC proposal.

Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto
(use `make benchmark-encoding` target). Benchmarking shows that depending on the payload
composition this data format is about 4x-5x faster in encoding and 2x-3x faster in
decoding equivalent data compared to OpenCensus data format (all benchmarks in Go).

Notable differences from OpenCensus:

- Attribute key/value pairs are represented as a list rather than as a map.
  This results in significant performance gains and at the same time changes
  the semantic of attributes because now it is possible to have multiple attributes
  with the same key. This is also in-line with Jaeger's tags representation.

- Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in
  significant performance improvements for certain payload compositions (e.g. lots of
  TimedEvents).

- Resource labels use the same data type as Span attributes which now allows
  to have labels with other data types (OpenCensus only allowed strings).

* Address review comments

* Add protobuf type qualifier to pre-formatted blocks

* Add a note about future goals for the protocol

* Address review comments

* Make sure all field comments start with field name

* Change status to approved

* Clarify start and end time expectations.

* Address Sergey's comments

* Remove string length requirement

* Add StatusCode enum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants