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

Remove Span.Status per OTEP-134 #918

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
iNikem marked this conversation as resolved.
Show resolved Hide resolved

Updates:

Expand Down
2 changes: 0 additions & 2 deletions spec-compliance-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ status of the feature is not known.
|End | + | + | + | + | + | + | + | + | | + |
|End with timestamp | + | + | + | + | + | + | + | - | | + |
|IsRecording | + | + | + | + | + | + | + | | | + |
|Set status | + | + | + | + | + | + | + | + | | + |
Copy link
Member

Choose a reason for hiding this comment

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

We could keep this and add something like (REMOVED) to keep track of its removal process (until all SIGs are done).

|Safe for concurrent calls | + | + | + | | + | + | + | + | | + |
|[Span attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-attributes)|
|SetAttribute | + | + | + | + | + | + | + | + | | + |
Expand Down Expand Up @@ -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|
Expand Down
112 changes: 3 additions & 109 deletions specification/trace/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Copy link
Member

@Oberon00 Oberon00 Sep 3, 2020

Choose a reason for hiding this comment

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

Note: This is wrong even in the current spec before removing status.

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

Expand All @@ -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.

Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions specification/trace/sdk_exporters/jaeger.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ OpenTelemetry Span's `InstrumentationLibrary` MUST be reported as span `tags` to

TBD

### Status

TBD

### Events

TBD
15 changes: 0 additions & 15 deletions specification/trace/sdk_exporters/zipkin.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
48 changes: 0 additions & 48 deletions specification/trace/semantic_conventions/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ and various HTTP versions like 1.1, 2 and SPDY.
<!-- toc -->

- [Name](#name)
- [Status](#status)
- [Common Attributes](#common-attributes)
- [HTTP client](#http-client)
- [HTTP server](#http-server)
Expand All @@ -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

<!-- semconv http -->
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 5 additions & 9 deletions specification/trace/semantic_conventions/rpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
iNikem marked this conversation as resolved.
Show resolved Hide resolved

For outgoing requests, the `SpanKind` MUST be set to `CLIENT` and for incoming requests to `SERVER`.

Expand Down Expand Up @@ -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 |
iNikem marked this conversation as resolved.
Show resolved Hide resolved

[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
Expand Down