From b2657f10c17b80227fbcc9d8d3e0aedb40408b90 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 3 Sep 2020 11:24:36 +0300 Subject: [PATCH 1/6] Remove Span.Status per OTEP-134 --- CHANGELOG.md | 2 + spec-compliance-matrix.md | 2 - specification/trace/api.md | 112 +----------------- specification/trace/sdk_exporters/jaeger.md | 4 - specification/trace/sdk_exporters/zipkin.md | 15 --- .../trace/semantic_conventions/http.md | 48 -------- .../trace/semantic_conventions/rpc.md | 14 +-- 7 files changed, 10 insertions(+), 187 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8125eb209f..8b2cd6a59b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ New: - Add Span API and semantic conventions for recording exceptions ([#697](https://github.com/open-telemetry/opentelemetry-specification/pull/697), [#874](https://github.com/open-telemetry/opentelemetry-specification/pull/874)) +- Remove `Span.Status` as per [OTEP-134](https://github.com/open-telemetry/oteps/pull/134) + (#918[https://github.com/open-telemetry/opentelemetry-specification/pull/918]) Updates: diff --git a/spec-compliance-matrix.md b/spec-compliance-matrix.md index 552b9cb79ab..0ca7088bc84 100644 --- a/spec-compliance-matrix.md +++ b/spec-compliance-matrix.md @@ -35,7 +35,6 @@ status of the feature is not known. |End | + | + | + | + | + | + | + | + | | + | |End with timestamp | + | + | + | + | + | + | + | - | | + | |IsRecording | + | + | + | + | + | + | + | | | + | -|Set status | + | + | + | + | + | + | + | + | | + | |Safe for concurrent calls | + | + | + | | + | + | + | + | | + | |[Span attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-attributes)| |SetAttribute | + | + | + | + | + | + | + | + | | + | @@ -137,7 +136,6 @@ status of the feature is not known. |InstrumentationLibrary mapping | | + | - | - | | - | - | - | | | |Boolean attributes | + | + | + | + | | + | + | + | | | |Array attributes | + | + | + | - | | + | + | + | | | -|Status mapping | + | - | + | - | | + | + | + | | | |Event attributes mapping to Annotations | + | | + | + | | + | + | + | | | |Fractional microseconds in timestamps | + | - | + | - | | - | - | - | | | |Jaeger| diff --git a/specification/trace/api.md b/specification/trace/api.md index bfcfb6d4f66..c1a12838068 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -26,17 +26,10 @@ Table of Contents * [IsRecording](#isrecording) * [Set Attributes](#set-attributes) * [Add Events](#add-events) - * [Set Status](#set-status) * [UpdateName](#updatename) * [End](#end) * [Record Exception](#record-exception) * [Span lifetime](#span-lifetime) -* [Status](#status) - * [StatusCanonicalCode](#statuscanonicalcode) - * [Status creation](#status-creation) - * [GetCanonicalCode](#getcanonicalcode) - * [GetDescription](#getdescription) - * [GetIsOk](#getisok) * [SpanKind](#spankind) * [Concurrency](#concurrency) * [Included Propagators](#included-propagators) @@ -226,7 +219,6 @@ the entire operation and, optionally, one or more sub-spans for its sub-operatio - [`Attributes`](../common/common.md#attributes) - A list of [`Link`s](#add-links) to other `Span`s - A list of timestamped [`Event`s](#add-events) -- A [`Status`](#set-status). The _span name_ concisely identifies the work represented by the Span, for example, an RPC method name, a function name, @@ -371,8 +363,8 @@ Links SHOULD preserve the order in which they're set. ### Span operations -With the exception of the function to retrieve the `Span`'s `SpanContext` and -recording status, none of the below may be called after the `Span` is finished. +With the exception of the function to retrieve the `Span`'s `SpanContext`, +none of the below may be called after the `Span` is finished. #### Get Context @@ -385,8 +377,7 @@ The Span interface MUST provide: #### IsRecording Returns true if this `Span` is recording information like events with the -`AddEvent` operation, attributes using `SetAttributes`, status with `SetStatus`, -etc. +`AddEvent` operation, attributes using `SetAttributes`, etc. There should be no parameter. @@ -454,19 +445,6 @@ keys"](semantic_conventions/README.md) which have prescribed semantic meanings. Note that [`RecordException`](#record-exception) is a specialized variant of `AddEvent` for recording exception events. -#### Set Status - -Sets the [`Status`](#status) of the `Span`. If used, this will override the -default `Span` status, which is `OK`. - -Only the value of the last call will be recorded, and implementations are free -to ignore previous calls. - -The Span interface MUST provide: - -- An API to set the `Status` where the new status is the only argument. This - SHOULD be called `SetStatus`. - #### UpdateName Updates the `Span` name. Upon this update, any sampling behavior based on `Span` @@ -536,90 +514,6 @@ timestamps to the Span object: Start and end time as well as Event's timestamps MUST be recorded at a time of a calling of corresponding API. -## Status - -`Status` interface represents the status of a finished `Span`. It's composed of -a canonical code in conjunction with an optional descriptive message. - -### StatusCanonicalCode - -`StatusCanonicalCode` represents the canonical set of status codes of a finished -`Span`, following the [Standard GRPC -codes](https://github.com/grpc/grpc/blob/master/doc/statuscodes.md): - -- `Ok` - - The operation completed successfully. -- `Cancelled` - - The operation was cancelled (typically by the caller). -- `Unknown` - - An unknown error. -- `InvalidArgument` - - Client specified an invalid argument. Note that this differs from - `FailedPrecondition`. `InvalidArgument` indicates arguments that are problematic - regardless of the state of the system. -- `DeadlineExceeded` - - Deadline expired before operation could complete. For operations that change the - state of the system, this error may be returned even if the operation has - completed successfully. -- `NotFound` - - Some requested entity (e.g., file or directory) was not found. -- `AlreadyExists` - - Some entity that we attempted to create (e.g., file or directory) already exists. -- `PermissionDenied` - - The caller does not have permission to execute the specified operation. - `PermissionDenied` must not be used if the caller cannot be identified (use - `Unauthenticated1` instead for those errors). -- `ResourceExhausted` - - Some resource has been exhausted, perhaps a per-user quota, or perhaps the - entire file system is out of space. -- `FailedPrecondition` - - Operation was rejected because the system is not in a state required for the - operation's execution. -- `Aborted` - - The operation was aborted, typically due to a concurrency issue like sequencer - check failures, transaction aborts, etc. -- `OutOfRange` - - Operation was attempted past the valid range. E.g., seeking or reading past end - of file. Unlike `InvalidArgument`, this error indicates a problem that may be - fixed if the system state changes. -- `Unimplemented` - - Operation is not implemented or not supported/enabled in this service. -- `Internal` - - Internal errors. Means some invariants expected by underlying system has been - broken. -- `Unavailable` - - The service is currently unavailable. This is a most likely a transient - condition and may be corrected by retrying with a backoff. -- `DataLoss` - - Unrecoverable data loss or corruption. -- `Unauthenticated` - - The request does not have valid authentication credentials for the operation. - -### Status creation - -API MUST provide a way to create a new `Status`. - -Required parameters - -- `StatusCanonicalCode` of this `Status`. - -Optional parameters - -- Description of this `Status`. - -### GetCanonicalCode - -Returns the `StatusCanonicalCode` of this `Status`. - -### GetDescription - -Returns the description of this `Status`. -Languages should follow their usual conventions on whether to return `null` or an empty string here if no description was given. - -### GetIsOk - -Returns true if the canonical code of this `Status` is `Ok`, otherwise false. - ## SpanKind `SpanKind` describes the relationship between the Span, its parents, diff --git a/specification/trace/sdk_exporters/jaeger.md b/specification/trace/sdk_exporters/jaeger.md index d5159730278..5e4b1cef89a 100644 --- a/specification/trace/sdk_exporters/jaeger.md +++ b/specification/trace/sdk_exporters/jaeger.md @@ -34,10 +34,6 @@ OpenTelemetry Span's `InstrumentationLibrary` MUST be reported as span `tags` to TBD -### Status - -TBD - ### Events TBD diff --git a/specification/trace/sdk_exporters/zipkin.md b/specification/trace/sdk_exporters/zipkin.md index 1afa4d7e1bc..6b6819b107f 100644 --- a/specification/trace/sdk_exporters/zipkin.md +++ b/specification/trace/sdk_exporters/zipkin.md @@ -22,7 +22,6 @@ and Zipkin. | Span.Attributes | Span.Tags | See [Attributes](../../common/common.md#attributes) for data types for the mapping. | | Span.Events | Span.Annotations | See [Events](#events) for the mapping format. | | Span.Links | TBD | TBD | -| Span.Status | Add to Span.Tags | See [Status](#status) for tag names to use. | | Span.LocalChildSpanCount | TBD | TBD | TBD : This is work in progress document and it is currently doesn't specify @@ -136,20 +135,6 @@ Array values MUST be serialized to string like a JSON list as mentioned in TBD: add examples -### Status - -Span `Status` MUST be reported as a key-value pair in `tags` to Zipkin. - -The following table defines the OpenTelemetry `Status` to Zipkin `tags` mapping. - -| Status|Tag Key| Tag Value | -|--|--|--| -|Code | `ot.status_code` | Name of the code, for example: `OK` | -|Message *(optional)* | `ot.status_description` | `{message}` | - -The `ot.status_code` tag value MUST follow the [Standard GRPC Code -Names](https://github.com/grpc/grpc/blob/master/doc/statuscodes.md). - ### Events OpenTelemetry `Event` has an optional `Attribute`(s) which is not supported by diff --git a/specification/trace/semantic_conventions/http.md b/specification/trace/semantic_conventions/http.md index 9c0cd04a55f..64177e449d4 100644 --- a/specification/trace/semantic_conventions/http.md +++ b/specification/trace/semantic_conventions/http.md @@ -9,7 +9,6 @@ and various HTTP versions like 1.1, 2 and SPDY. - [Name](#name) -- [Status](#status) - [Common Attributes](#common-attributes) - [HTTP client](#http-client) - [HTTP server](#http-server) @@ -35,39 +34,6 @@ such as `"HTTP {METHOD_NAME}"`. Instrumentation MUST NOT default to using URI path as span name, but MAY provide hooks to allow custom logic to override the default span name. -## Status - -Implementations MUST set the [span status](../api.md#status) if the HTTP communication failed -or an HTTP error status code is returned (e.g. above 3xx). - -In the case of an HTTP redirect, the request should normally be considered successful, -unless the client aborts following redirects due to hitting some limit (redirect loop). -If following a (chain of) redirect(s) successfully, the status should be set according to the result of the final HTTP request. - -Don't set the span status description if the reason can be inferred from `http.status_code` and `http.status_text`. - -| HTTP code | Span status code | -|-------------------------|-----------------------| -| 100...299 | `Ok` | -| 3xx redirect codes | `DeadlineExceeded` in case of loop (see above) [1], otherwise `Ok` | -| 401 Unauthorized ⚠ | `Unauthenticated` ⚠ (Unauthorized actually means unauthenticated according to [RFC 7235][rfc-unauthorized]) | -| 403 Forbidden | `PermissionDenied` | -| 404 Not Found | `NotFound` | -| 429 Too Many Requests | `ResourceExhausted` | -| 499 Client Closed | `Cancelled` (Not an official HTTP status code, defined by [NGINX][nginx-http-499]) | -| Other 4xx code | `InvalidArgument` [1] | -| 501 Not Implemented | `Unimplemented` | -| 503 Service Unavailable | `Unavailable` | -| 504 Gateway Timeout | `DeadlineExceeded` | -| Other 5xx code | `Internal` [1] | -| Any status code the client fails to interpret (e.g., 093 or 573) | `Unknown` | - -Note that the items marked with [1] are different from the mapping defined in the [OpenCensus semantic conventions][oc-http-status]. - -[oc-http-status]: https://github.com/census-instrumentation/opencensus-specs/blob/master/trace/HTTP.md#mapping-from-http-status-codes-to-trace-status-codes -[rfc-unauthorized]: https://tools.ietf.org/html/rfc7235#section-3.1 -[nginx-http-499]: https://httpstatuses.com/499 - ## Common Attributes @@ -128,20 +94,6 @@ from the `net.peer.name` used to look up the `net.peer.ip` that is actually connected to. In that case it is strongly recommended to set the `net.peer.name` attribute in addition to `http.host`. -For status, the following special cases have canonical error codes assigned: - -| Client error | Trace status code | -|-----------------------------|--------------------| -| DNS resolution failed | `Unknown` | -| Request cancelled by caller | `Cancelled` | -| URL cannot be parsed | `InvalidArgument` | -| Request timed out | `DeadlineExceeded` | - -This is not meant to be an exhaustive list -but if there is no clear mapping for some error conditions, -instrumentation developers are encouraged to use `Unknown` -and open a PR or issue in the specification repository. - ## HTTP server To understand the attributes defined in this section, it is helpful to read the "Definitions" subsection. diff --git a/specification/trace/semantic_conventions/rpc.md b/specification/trace/semantic_conventions/rpc.md index 62df39f334e..f70bf3c8ed7 100644 --- a/specification/trace/semantic_conventions/rpc.md +++ b/specification/trace/semantic_conventions/rpc.md @@ -20,7 +20,7 @@ This document defines how to describe remote procedure calls ## Common remote procedure call conventions -A remote procedure calls is described by two separate spans, one on the client-side and one on the server-side. +A remote procedure calls are described by two separate spans, one on the client-side and one on the server-side. For outgoing requests, the `SpanKind` MUST be set to `CLIENT` and for incoming requests to `SERVER`. @@ -89,17 +89,13 @@ Note that _method_ in this context is about the called remote procedure and _not For remote procedure calls via [gRPC][], additional conventions are described in this section. -`rpc.system` MUST be set to `"grpc"`. +| Attribute name | Notes and examples | Required? | +| -------------- | ---------------------------------------------------------------------- | --------- | +| `rpc.system` | MUST be set to `"grpc"` | Yes | +| `rpc.status` | Canonical status code of the gRPC call's response | Yes | [gRPC]: https://grpc.io/ -### Status - -Implementations MUST set status which MUST be the same as the gRPC client/server -status. The mapping between gRPC canonical codes and OpenTelemetry status codes -is 1:1 as OpenTelemetry canonical codes is just a snapshot of grpc codes which -can be found [here](https://github.com/grpc/grpc-go/blob/master/codes/codes.go). - ### Events In the lifetime of a gRPC stream, an event for each message sent/received on From 0b6c065e887b3513c06080b038e567e40f3b5835 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 3 Sep 2020 13:32:34 +0300 Subject: [PATCH 2/6] Update specification/trace/semantic_conventions/rpc.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Christian Neumüller --- specification/trace/semantic_conventions/rpc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/trace/semantic_conventions/rpc.md b/specification/trace/semantic_conventions/rpc.md index f70bf3c8ed7..0d718fac8ea 100644 --- a/specification/trace/semantic_conventions/rpc.md +++ b/specification/trace/semantic_conventions/rpc.md @@ -92,7 +92,7 @@ For remote procedure calls via [gRPC][], additional conventions are described in | Attribute name | Notes and examples | Required? | | -------------- | ---------------------------------------------------------------------- | --------- | | `rpc.system` | MUST be set to `"grpc"` | Yes | -| `rpc.status` | Canonical status code of the gRPC call's response | Yes | +| `rpc.grpc.status` | Canonical status code of the gRPC call's response | Yes | [gRPC]: https://grpc.io/ From aea6201e8b73cf7c55084c4e55bba81c85ac1d2a Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 3 Sep 2020 14:12:53 +0300 Subject: [PATCH 3/6] Fix --- specification/trace/semantic_conventions/rpc.md | 1 - 1 file changed, 1 deletion(-) diff --git a/specification/trace/semantic_conventions/rpc.md b/specification/trace/semantic_conventions/rpc.md index 0d718fac8ea..957007a5202 100644 --- a/specification/trace/semantic_conventions/rpc.md +++ b/specification/trace/semantic_conventions/rpc.md @@ -13,7 +13,6 @@ This document defines how to describe remote procedure calls + [Service name](#service-name) * [Distinction from HTTP spans](#distinction-from-http-spans) - [gRPC](#grpc) - * [Status](#status) * [Events](#events) From 2c048318dcaa7dc1b52b03f272530fd690812f75 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 3 Sep 2020 15:34:16 +0300 Subject: [PATCH 4/6] Update CHANGELOG.md Co-authored-by: Armin Ruech --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2cd6a59b1..fe0ee017bbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ New: ([#697](https://github.com/open-telemetry/opentelemetry-specification/pull/697), [#874](https://github.com/open-telemetry/opentelemetry-specification/pull/874)) - Remove `Span.Status` as per [OTEP-134](https://github.com/open-telemetry/oteps/pull/134) - (#918[https://github.com/open-telemetry/opentelemetry-specification/pull/918]) + ([#918](https://github.com/open-telemetry/opentelemetry-specification/pull/918)) Updates: From 05cb1e1866bf942ac9748218e270adc291b321cc Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 3 Sep 2020 15:34:43 +0300 Subject: [PATCH 5/6] Update specification/trace/semantic_conventions/rpc.md Co-authored-by: Armin Ruech --- specification/trace/semantic_conventions/rpc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/trace/semantic_conventions/rpc.md b/specification/trace/semantic_conventions/rpc.md index 957007a5202..4dcf2bec3e1 100644 --- a/specification/trace/semantic_conventions/rpc.md +++ b/specification/trace/semantic_conventions/rpc.md @@ -19,7 +19,7 @@ This document defines how to describe remote procedure calls ## Common remote procedure call conventions -A remote procedure calls are described by two separate spans, one on the client-side and one on the server-side. +A remote procedure call is described by two separate spans, one on the client-side and one on the server-side. For outgoing requests, the `SpanKind` MUST be set to `CLIENT` and for incoming requests to `SERVER`. From 755370f13e275a5e6d006abb6ceb582d08bf47f4 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 3 Sep 2020 15:41:31 +0300 Subject: [PATCH 6/6] Marked changes in spec complience matrix --- spec-compliance-matrix.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec-compliance-matrix.md b/spec-compliance-matrix.md index 0ca7088bc84..adc618d8c29 100644 --- a/spec-compliance-matrix.md +++ b/spec-compliance-matrix.md @@ -35,6 +35,7 @@ status of the feature is not known. |End | + | + | + | + | + | + | + | + | | + | |End with timestamp | + | + | + | + | + | + | + | - | | + | |IsRecording | + | + | + | + | + | + | + | | | + | +|Set status (REMOVED) | + | + | + | + | + | + | + | + | | + | |Safe for concurrent calls | + | + | + | | + | + | + | + | | + | |[Span attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-attributes)| |SetAttribute | + | + | + | + | + | + | + | + | | + | @@ -136,6 +137,7 @@ status of the feature is not known. |InstrumentationLibrary mapping | | + | - | - | | - | - | - | | | |Boolean attributes | + | + | + | + | | + | + | + | | | |Array attributes | + | + | + | - | | + | + | + | | | +|Status mapping (REMOVED) | + | - | + | - | | + | + | + | | | |Event attributes mapping to Annotations | + | | + | + | | + | + | + | | | |Fractional microseconds in timestamps | + | - | + | - | | - | - | - | | | |Jaeger|