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.

Sign up for GitHub

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

Map Error Status to error tag in Zipkin & Jaeger #1257

Merged
merged 16 commits into from
Dec 10, 2020
Merged

Map Error Status to error tag in Zipkin & Jaeger #1257

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d283999
Added mapping of Error Status to error flag in Zipkin & Jaeger export…
CodeBlanch Nov 23, 2020
2133e6b
Merge remote-tracking branch 'upstream/master' into zipkin-jaeger-err…
CodeBlanch Nov 23, 2020
4d618e2
Zipkin error flag is now an empty string.
CodeBlanch Nov 24, 2020
cd5c3c7
Restored uppercase for status enum values.
CodeBlanch Nov 24, 2020
1ea97b2
Merge branch 'master' into zipkin-jaeger-error-flag
cijothomas Nov 24, 2020
1fba8f2
Removed the error text under attributes.
CodeBlanch Nov 25, 2020
f3d4176
Casing fix.
CodeBlanch Nov 25, 2020
f16cc91
Merging latest from master.
CodeBlanch Dec 3, 2020
b62b651
Review feedback.
CodeBlanch Dec 3, 2020
8534795
Added back information about removing error flag when false.
CodeBlanch Dec 4, 2020
0bf7fa8
Merge branch 'master' into zipkin-jaeger-error-flag
CodeBlanch Dec 7, 2020
91f9c1f
Updated CHANGELOG.
CodeBlanch Dec 7, 2020
70fd808
Update specification/trace/sdk_exporters/jaeger.md
CodeBlanch Dec 9, 2020
de9a65f
Merge branch 'master' into zipkin-jaeger-error-flag
CodeBlanch Dec 9, 2020
a6ebf3d
Merge branch 'master' into zipkin-jaeger-error-flag
CodeBlanch Dec 10, 2020
2891d74
Review feedback.
CodeBlanch Dec 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Updates:
([#1219](https://github.com/open-telemetry/opentelemetry-specification/pull/1219))
- Add RPC semantic conventions for metrics
([#1162](https://github.com/open-telemetry/opentelemetry-specification/pull/1162))
- Clarify `Description` usage on `Status` API
([#1257](https://github.com/open-telemetry/opentelemetry-specification/pull/1257))
- Add/Update `Status` + `error` mapping for Jaeger & Zipkin Exporters
([#1257](https://github.com/open-telemetry/opentelemetry-specification/pull/1257))

## v0.7.0 (11-18-2020)

Expand Down
3 changes: 3 additions & 0 deletions spec-compliance-matrix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ status of the feature is not known.
|Boolean attributes | + | + | + | + | | + | + | + | | + |
|Array attributes | + | + | + | [-](https://github.com/open-telemetry/opentelemetry-python/issues/1110) | | + | + | + | | + |
|Status mapping | + | + | + | + | | + | + | + | | + |
|Error Status mapping | | | | | | | | | | + |
|Event attributes mapping to Annotations | + | + | + | + | | + | + | + | | + |
|Integer microseconds in timestamps | | + | | + | | | | | | + |
|[Jaeger](specification/trace/sdk_exporters/jaeger.md)|
Expand All @@ -167,6 +168,8 @@ status of the feature is not known.
|Service name mapping | | | | + | | | | | | + |
|Resource to Process mapping | | | | [-](https://github.com/open-telemetry/opentelemetry-python/issues/1436) | | | | | | + |
|InstrumentationLibrary mapping | | | | + | | | | | | + |
|Status mapping | | | | | | | | | | + |
|Error Status mapping | | | | | | | | | | + |
|Events converted to Logs | | | | + | | | | | | + |
|OpenCensus|
|TBD|
Expand Down
10 changes: 6 additions & 4 deletions specification/trace/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ status, which is `Unset`.

- `StatusCode`, one of the values listed below.
- Optional `Description` that provides a descriptive message of the `Status`.
`Description` MUST only be used with the `Error` `StatusCode` value.

`StatusCode` is one of the following values:

Expand All @@ -529,10 +530,11 @@ status, which is `Unset`.

The Span interface MUST provide:

- An API to set the `Status`. This SHOULD be called `SetStatus`.
This API takes the `StatusCode`, and an optional `Description`, either as
individual parameters or as an immutable object encapsulating them, whichever
is most appropriate for the language.
- An API to set the `Status`. This SHOULD be called `SetStatus`. This API takes
the `StatusCode`, and an optional `Description`, either as individual
parameters or as an immutable object encapsulating them, whichever is most
appropriate for the language. `Description` MUST be IGNORED for `StatusCode`
Copy link
Member

Choose a reason for hiding this comment

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

either as individual parameters or as an immutable object encapsulating them, whichever is most appropriate for the language.

This sounds like a general guideline that would apply to any API function, we don't need to be cluttering individual function descriptions with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was there before my addition you want me to update here?

Copy link
Member

Choose a reason for hiding this comment

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

could be done in another PR

`Ok` & `Unset` values.

The status code SHOULD remain unset, except for the following circumstances:

Expand Down
16 changes: 15 additions & 1 deletion specification/trace/sdk_exporters/jaeger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,21 @@ TBD

### Status

TBD
Span `Status` MUST be reported as a key-value pair in `tags` to Jaeger, unless it is `UNSET`.
In the latter case it MUST NOT be reported.

The following table defines the OpenTelemetry `Status` to Jaeger `tags` mapping.

| Status|Tag Key| Tag Value |
|--|--|--|
|Code | `otel.status_code` | Name of the code, either `OK` or `ERROR`. MUST NOT be set if the code is `UNSET`. |
|Description | `otel.status_description` | Description of the `Status` if it has a value otherwise not set. |

### Error flag

When Span `Status` is set to `ERROR` an `error` tag SHOULD be added with the
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
`BOOL` value (`vBool`) set to `true`. The added `error` tag MAY override any
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
previous value.

### Events

Expand Down
10 changes: 6 additions & 4 deletions specification/trace/sdk_exporters/zipkin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ convention](../semantic_conventions/README.md)
document maps to the strongly-typed fields of Zipkin spans.

Primitive types MUST be converted to string using en-US culture settings.
Boolean values must use lower case strings `"true"` and `"false"`, except an
Copy link

Choose a reason for hiding this comment

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

this special case is still true. For example, you have an opentracing bridge which has in the past littered folks stuff with "error"->"false" data. can you please verify that false should never be sent as a value of "error" regardless of if being mapped from the grpc status or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I added it back with different words as part of the Status section.

attribute named `error`. In case if value of the attribute is `false`, Zipkin
tag needs to be omitted.
Boolean values MUST use lower case strings `"true"` and `"false"`.

Array values MUST be serialized to string like a JSON list as mentioned in
[semantic conventions](../../overview.md#semantic-conventions).
Expand All @@ -146,7 +144,11 @@ The following table defines the OpenTelemetry `Status` to Zipkin `tags` mapping.
| Status|Tag Key| Tag Value |
|--|--|--|
|Code | `otel.status_code` | Name of the code, either `OK` or `ERROR`. MUST NOT be set if the code is `UNSET`. |
|Message *(optional)* | `otel.status_description` | `{message}` |
|Description| `error` | Description of the `Status`. MUST be set if the code is `ERROR`, use an empty string if Description has no value. MUST NOT be set for `OK` and `UNSET` codes. |

Note: The `error` tag should only be set if `Status` is `Error`. If a boolean
version (`{"error":false}` or `{"error":"false"}`) is present, it SHOULD be
removed. Zipkin will treat any span with `error` sent as failed.

### Events

Expand Down