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

Update Status to meet with the latest specification #1311

Closed
reyang opened this issue Sep 28, 2020 · 5 comments · Fixed by #1313
Closed

Update Status to meet with the latest specification #1311

reyang opened this issue Sep 28, 2020 · 5 comments · Fixed by #1313
Assignees
Labels
bug Something isn't working pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package

Comments

@reyang
Copy link
Member

reyang commented Sep 28, 2020

The OpenTelemetry specification has been changed to reduce the complexity of Span Status https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#status.

Need to take this breaking change and update the

public readonly struct Status : System.IEquatable<Status>
file.

@reyang reyang added bug Something isn't working pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package priority:p1 labels Sep 28, 2020
@reyang reyang added this to the 0.7.0-beta (Beta 4) milestone Sep 28, 2020
@eddynaka
Copy link
Contributor

I'll take a look today/tomorrow

@eddynaka eddynaka self-assigned this Sep 28, 2020
@eddynaka
Copy link
Contributor

Should we change the StatusCanonicalCode as well?

It contains the same list from the Status.

@reyang
Copy link
Member Author

reyang commented Sep 28, 2020

Should we change the StatusCanonicalCode as well?

It contains the same list from the Status.

Yes, and we probably can have 1 class instead of two.

@eddynaka
Copy link
Contributor

Should we change the StatusCanonicalCode as well?

It contains the same list from the Status.

Yes, and we probably can have 1 class instead of two.

I started to look at this and we might have a problem. Grpc uses the StatusCanonicalCode. With that, it's mapped to the Status itself.
I'm opening a draft PR to get other comments.

@eddynaka
Copy link
Contributor

eddynaka commented Oct 7, 2020

FYI, opened an issue in the spec: open-telemetry/opentelemetry-specification#1044

Problems: we don't have a current map from the rpc status code to otel status code.

The current mapping in the PR is based on assumptions...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants