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

fix! implement new Span.Status spec #411

Merged
merged 8 commits into from
Sep 22, 2020
Merged

fix! implement new Span.Status spec #411

merged 8 commits into from
Sep 22, 2020

Conversation

fbogsany
Copy link
Contributor

This implements the new Span.Status spec: open-telemetry/opentelemetry-specification#966

We need the mapping to OTLP to be specified in order to complete this, however that is strictly additive and this implements the proposal for OK & Unset -> OK and Error -> UnknownError.

Copy link
Contributor

@robertlaurin robertlaurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me

Copy link
Member

@mwear mwear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We can and probably should set the error: true span attribute for the Jaeger exporter: https://github.com/open-telemetry/opentelemetry-ruby/blob/master/exporter/jaeger/lib/opentelemetry/exporter/jaeger/encoder.rb#L121-L124. This can be done in a follow up.

@fbogsany
Copy link
Contributor Author

Updated to encode an error status in Jaeger, and to default to UNSET. We have treated nil as UNSET, and we have that expectation in a bunch of places (particularly tests), so the exporters are resilient to span_data.status.nil?. The spec is clear that UNSET is the default, though.

@fbogsany fbogsany merged commit eaa06e1 into master Sep 22, 2020
@fbogsany fbogsany deleted the new-status branch September 22, 2020 14:40
@fbogsany fbogsany mentioned this pull request Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement error flagging with status codes (OTEP 136)
3 participants