From 5cdce911b52921c7af3ecb2af783fac0d5a8df4c Mon Sep 17 00:00:00 2001 From: Ted Young Date: Thu, 17 Sep 2020 13:52:42 -0700 Subject: [PATCH 1/7] StatusSource definition --- specification/trace/api.md | 86 ++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 49 deletions(-) diff --git a/specification/trace/api.md b/specification/trace/api.md index 4d4c3a399c6..558e798e30d 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -249,6 +249,7 @@ the entire operation and, optionally, one or more sub-spans for its sub-operatio - A list of [`Link`s](#add-links) to other `Span`s - A list of timestamped [`Event`s](#add-events) - A [`Status`](#set-status). +- A [`Status Source`](#set-status-source). The _span name_ concisely identifies the work represented by the Span, for example, an RPC method name, a function name, @@ -573,7 +574,7 @@ 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. +a canonical code, an optional descriptive message, and a source. ### StatusCanonicalCode @@ -581,53 +582,36 @@ a canonical code in conjunction with an optional descriptive message. `Span`, following the [Standard GRPC codes](https://github.com/grpc/grpc/blob/master/doc/statuscodes.md): +- `Unset` + - The default status. +- `Error` + - The operation contains an error. - `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. + - The operation has been validated to have completed successfully. + +The status code SHOULD remain unset, except for the following circumstances: + +### StatusSource +The `StatusSource` represents the origin of the status code, and which rules +apply for setting it. + +- `Instrumentation` + - Spans generated by Instrumentation Libraries. +- `User` + - Spans generated or modified by the Application Developer or Operator. + +When the status source is `Instrumentation`, status codes SHOULD be documented +and predictable. The status code should only be set to `ERROR` according to the rules +defined within the semantic conventions. For operations not covered by the semantic +conventions, Instrumentation Libraries SHOULD publish their own conventions, +including status codes. + +When the status source is set to `Instrumentation`, there is no need to ever +set the status code to `Ok`. `Unset` and `Ok` SHOULD be treated as equivalent +when the status source is `Instrumentation`. + +When the status source is set to `User`, the statuses `Error` and `Ok` SHOULD be +treated as an override for any other error analysis. ### Status creation @@ -640,6 +624,7 @@ Required parameters Optional parameters - Description of this `Status`. +- A `StatusSource` value. ### GetCanonicalCode @@ -650,9 +635,12 @@ Returns the `StatusCanonicalCode` of this `Status`. 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 +### GetIsError + +Returns true if the canonical code of this `Status` is `Error`, otherwise false. -Returns true if the canonical code of this `Status` is `Ok`, otherwise false. +### GetStatusSource +Returns the `StatusSource`. ## SpanKind From 2dff27e893ce1584ef00df5a812b8367b4092e3e Mon Sep 17 00:00:00 2001 From: Ted Young Date: Thu, 17 Sep 2020 13:54:31 -0700 Subject: [PATCH 2/7] Update HTTP status code mapping --- .../trace/semantic_conventions/http.md | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/specification/trace/semantic_conventions/http.md b/specification/trace/semantic_conventions/http.md index 9c0cd04a55f..581179088b7 100644 --- a/specification/trace/semantic_conventions/http.md +++ b/specification/trace/semantic_conventions/http.md @@ -49,22 +49,19 @@ Don't set the span status description if the reason can be inferred from `http.s | 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 +| 3xx redirect codes | `Error` in case of loop (see above), otherwise `Ok` | +| 401 Unauthorized ⚠ | `Error` ⚠ (Unauthorized actually means unauthenticated according to [RFC 7235][rfc-unauthorized]) | +| 403 Forbidden | `Error` | +| 404 Not Found | `Error` | +| 429 Too Many Requests | `Error` | +| 499 Client Closed | `Error` (Not an official HTTP status code, defined by [NGINX][nginx-http-499]) | +| Other 4xx code | `Error` | +| 501 Not Implemented | `Error` | +| 503 Service Unavailable | `Error` | +| 504 Gateway Timeout | `Error` | +| Other 5xx code | `Error` | +| Any status code the client fails to interpret (e.g., 093 or 573) | `Error` | + [rfc-unauthorized]: https://tools.ietf.org/html/rfc7235#section-3.1 [nginx-http-499]: https://httpstatuses.com/499 From cdb0f0447592519321102c8aaa3fb16163a1cf0e Mon Sep 17 00:00:00 2001 From: Ted Young Date: Thu, 17 Sep 2020 21:58:14 -0700 Subject: [PATCH 3/7] Remove span status --- specification/trace/api.md | 39 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/specification/trace/api.md b/specification/trace/api.md index 558e798e30d..82f37639e44 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -249,7 +249,6 @@ the entire operation and, optionally, one or more sub-spans for its sub-operatio - A list of [`Link`s](#add-links) to other `Span`s - A list of timestamped [`Event`s](#add-events) - A [`Status`](#set-status). -- A [`Status Source`](#set-status-source). The _span name_ concisely identifies the work represented by the Span, for example, an RPC method name, a function name, @@ -574,7 +573,7 @@ calling of corresponding API. ## Status `Status` interface represents the status of a finished `Span`. It's composed of -a canonical code, an optional descriptive message, and a source. +a canonical code, and an optional descriptive message. ### StatusCanonicalCode @@ -587,31 +586,25 @@ codes](https://github.com/grpc/grpc/blob/master/doc/statuscodes.md): - `Error` - The operation contains an error. - `Ok` - - The operation has been validated to have completed successfully. + - The operation has been validated by an Application developers or Operator to + have completed successfully, or contain The status code SHOULD remain unset, except for the following circumstances: -### StatusSource -The `StatusSource` represents the origin of the status code, and which rules -apply for setting it. +When the status is set to `ERROR` by Instrumentation Libraries, the status codes +SHOULD be documented and predictable. The status code should only be set to `ERROR` +according to the rules defined within the semantic conventions. For operations +not covered by the semantic conventions, Instrumentation Libraries SHOULD +publish their own conventions, including status codes. -- `Instrumentation` - - Spans generated by Instrumentation Libraries. -- `User` - - Spans generated or modified by the Application Developer or Operator. +Generally, Instrumentation Libraries SHOULD NOT set the status code to `Ok`, +unless explicitly configured to do so. Instrumention libraries SHOULD leave the +status code as `Unset` unless there is an error, as described above. -When the status source is `Instrumentation`, status codes SHOULD be documented -and predictable. The status code should only be set to `ERROR` according to the rules -defined within the semantic conventions. For operations not covered by the semantic -conventions, Instrumentation Libraries SHOULD publish their own conventions, -including status codes. +Application developers and Operators may set the status code to `Ok`. -When the status source is set to `Instrumentation`, there is no need to ever -set the status code to `Ok`. `Unset` and `Ok` SHOULD be treated as equivalent -when the status source is `Instrumentation`. - -When the status source is set to `User`, the statuses `Error` and `Ok` SHOULD be -treated as an override for any other error analysis. +Analysis tools SHOULD respond to an `Ok` status by supressing any errors they +would otherwise generate. For example, to supress noisy errors such as 404s. ### Status creation @@ -624,7 +617,6 @@ Required parameters Optional parameters - Description of this `Status`. -- A `StatusSource` value. ### GetCanonicalCode @@ -639,9 +631,6 @@ Languages should follow their usual conventions on whether to return `null` or a Returns true if the canonical code of this `Status` is `Error`, otherwise false. -### GetStatusSource -Returns the `StatusSource`. - ## SpanKind `SpanKind` describes the relationship between the Span, its parents, From 573c06f8741898541124442581117deb407e49a8 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Cortez Date: Thu, 24 Sep 2020 15:43:11 +0200 Subject: [PATCH 4/7] Apply feedback. --- specification/trace/api.md | 7 +--- .../trace/semantic_conventions/http.md | 34 ++++--------------- 2 files changed, 8 insertions(+), 33 deletions(-) diff --git a/specification/trace/api.md b/specification/trace/api.md index 82f37639e44..fd58d0d87eb 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -578,8 +578,7 @@ a canonical code, and 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): +`Span`. - `Unset` - The default status. @@ -627,10 +626,6 @@ Returns the `StatusCanonicalCode` of this `Status`. 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. -### GetIsError - -Returns true if the canonical code of this `Status` is `Error`, otherwise false. - ## SpanKind `SpanKind` describes the relationship between the Span, its parents, diff --git a/specification/trace/semantic_conventions/http.md b/specification/trace/semantic_conventions/http.md index 581179088b7..e97aa3b9eb4 100644 --- a/specification/trace/semantic_conventions/http.md +++ b/specification/trace/semantic_conventions/http.md @@ -37,33 +37,13 @@ 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 | `Error` in case of loop (see above), otherwise `Ok` | -| 401 Unauthorized ⚠ | `Error` ⚠ (Unauthorized actually means unauthenticated according to [RFC 7235][rfc-unauthorized]) | -| 403 Forbidden | `Error` | -| 404 Not Found | `Error` | -| 429 Too Many Requests | `Error` | -| 499 Client Closed | `Error` (Not an official HTTP status code, defined by [NGINX][nginx-http-499]) | -| Other 4xx code | `Error` | -| 501 Not Implemented | `Error` | -| 503 Service Unavailable | `Error` | -| 504 Gateway Timeout | `Error` | -| Other 5xx code | `Error` | -| Any status code the client fails to interpret (e.g., 093 or 573) | `Error` | - -[rfc-unauthorized]: https://tools.ietf.org/html/rfc7235#section-3.1 -[nginx-http-499]: https://httpstatuses.com/499 +[Span Status](../api.md#status) MUST be left unset if HTTP status code was in the +1xx, 2xx or 3xx ranges, unless there was another error (e.g., network error receiving +the response body; or 3xx codes with max redirects exceeded), in which case status +MUST be set to `Error`. + +For HTTP status codes in the 4xx and 5xx ranges, as well as any other code the client +failed to interpret, status MUST be set to `Error`. ## Common Attributes From ff6d57c7e8fe0e57313bacf26f75f19648a61ab4 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Cortez Date: Thu, 24 Sep 2020 16:26:55 +0200 Subject: [PATCH 5/7] Remove unused link. --- specification/trace/api.md | 1 - 1 file changed, 1 deletion(-) diff --git a/specification/trace/api.md b/specification/trace/api.md index fd58d0d87eb..c504fa6d69f 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -36,7 +36,6 @@ Table of Contents * [Status creation](#status-creation) * [GetCanonicalCode](#getcanonicalcode) * [GetDescription](#getdescription) - * [GetIsOk](#getisok) * [SpanKind](#spankind) * [Concurrency](#concurrency) * [Included Propagators](#included-propagators) From 39a23f0bd6c927597a7579ca233c112fe19c0562 Mon Sep 17 00:00:00 2001 From: Ted Young Date: Thu, 24 Sep 2020 11:01:21 -0700 Subject: [PATCH 6/7] markdownlint --- specification/trace/api.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/specification/trace/api.md b/specification/trace/api.md index 215626969ab..29b9c5723b3 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -592,24 +592,24 @@ a canonical code, and an optional descriptive message. - `Error` - The operation contains an error. - `Ok` - - The operation has been validated by an Application developers or Operator to - have completed successfully, or contain + - The operation has been validated by an Application developers or Operator to + have completed successfully, or contain The status code SHOULD remain unset, except for the following circumstances: -When the status is set to `ERROR` by Instrumentation Libraries, the status codes -SHOULD be documented and predictable. The status code should only be set to `ERROR` -according to the rules defined within the semantic conventions. For operations -not covered by the semantic conventions, Instrumentation Libraries SHOULD -publish their own conventions, including status codes. +When the status is set to `ERROR` by Instrumentation Libraries, the status codes +SHOULD be documented and predictable. The status code should only be set to `ERROR` +according to the rules defined within the semantic conventions. For operations +not covered by the semantic conventions, Instrumentation Libraries SHOULD +publish their own conventions, including status codes. -Generally, Instrumentation Libraries SHOULD NOT set the status code to `Ok`, -unless explicitly configured to do so. Instrumention libraries SHOULD leave the +Generally, Instrumentation Libraries SHOULD NOT set the status code to `Ok`, +unless explicitly configured to do so. Instrumention libraries SHOULD leave the status code as `Unset` unless there is an error, as described above. -Application developers and Operators may set the status code to `Ok`. +Application developers and Operators may set the status code to `Ok`. -Analysis tools SHOULD respond to an `Ok` status by supressing any errors they +Analysis tools SHOULD respond to an `Ok` status by supressing any errors they would otherwise generate. For example, to supress noisy errors such as 404s. ### Status creation From 150be61dd36a79f01bd240bc32d6fd24b2f818ae Mon Sep 17 00:00:00 2001 From: Ted Young Date: Thu, 24 Sep 2020 11:03:35 -0700 Subject: [PATCH 7/7] spelling --- specification/trace/api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specification/trace/api.md b/specification/trace/api.md index 29b9c5723b3..b7816e3920c 100644 --- a/specification/trace/api.md +++ b/specification/trace/api.md @@ -609,8 +609,8 @@ status code as `Unset` unless there is an error, as described above. Application developers and Operators may set the status code to `Ok`. -Analysis tools SHOULD respond to an `Ok` status by supressing any errors they -would otherwise generate. For example, to supress noisy errors such as 404s. +Analysis tools SHOULD respond to an `Ok` status by suppressing any errors they +would otherwise generate. For example, to suppress noisy errors such as 404s. ### Status creation