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

Jump to bottom

Add semantic conventions for HTTP metrics #739

Merged
merged 22 commits into from
Aug 30, 2020
Merged

Add semantic conventions for HTTP metrics #739

Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
34eb0ba
add http metric label spec
grahamfuller1 Jul 16, 2020
18a3e67
typo
grahamfuller1 Jul 16, 2020
028d345
update attribute locations and clarify which to inlcude, alter and ex…
grahamfuller1 Jul 21, 2020
6f0e55d
add metric instruments list
grahamfuller1 Jul 21, 2020
d5d15cc
simplify intro sections, add requirement columns, clarify labels, fix…
grahamfuller1 Jul 22, 2020
09116fc
fix HTTP strings
grahamfuller1 Jul 22, 2020
14c0a98
add count metric instrument and shorten duration metric name
grahamfuller1 Jul 23, 2020
b3e8bf2
remove dependency on http.md, add more notes and examples and general…
grahamfuller1 Jul 24, 2020
7e0c306
replace span.kind with type
grahamfuller1 Jul 24, 2020
4d2ec7f
add missing labels and cleanup links
grahamfuller1 Jul 24, 2020
b87fcb8
substitution->alternatives and remove section not needed
grahamfuller1 Jul 24, 2020
e2ce1b3
make request count metric instrument plural
gfuller1 Jul 30, 2020
952d1ae
formatting
gfuller1 Jul 30, 2020
c27d66d
clarify type column
gfuller1 Aug 5, 2020
7b59ac0
update intro and fix units
gfuller1 Aug 6, 2020
1ba7a15
breakout metric instrument table
gfuller1 Aug 6, 2020
bdcbdd5
remove http.route since it is the same as http.target after http.targ…
gfuller1 Aug 13, 2020
7de9d45
update net labels to link to definition
gfuller1 Aug 20, 2020
d420059
add lowercase requirement to http.scheme
gfuller1 Aug 21, 2020
655a490
Merge branch 'master' into http-metrics-spec
bogdandrutu Aug 28, 2020
d607daf
formatting
gfuller1 Aug 28, 2020
da2a2de
Merge branch 'master' into http-metrics-spec
bogdandrutu Aug 30, 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
91 changes: 91 additions & 0 deletions specification/metrics/semantic_conventions/http-metrics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# General

The conventions described in this section are HTTP specific. When HTTP operations occur,
metric events about those operations will be generated and reported to provide insight into the
operations. By adding HTTP labels to metric events it allows for finely tuned filtering.

**Discaimer:** These are initial HTTP metric instruments and labels but more may be added in the future.

### Metric Instruments

The following metric instruments MUST be used to describe HTTP operations. They MUST be of the specified
type and units.

#### HTTP Server
Below is a table of HTTP server metric instruments.

| Name | Instrument | Units | Description |
|------------------------|---------------|--------------|-------------|
| `http.server.duration` | ValueRecorder | milliseconds | measures the duration of the inbound HTTP request |

#### HTTP Client
Below is a table of HTTP client metric instruments.

| Name | Instrument | Units | Description |
|------------------------|---------------|--------------|-------------|
| `http.client.duration` | ValueRecorder | milliseconds | measure the duration of the outbound HTTP request |

### Labels
jmacd marked this conversation as resolved.
Show resolved Hide resolved

Below is a table of the labels that SHOULD be included on metric events
gfuller1 marked this conversation as resolved.
Show resolved Hide resolved
and whether they should be on server, client, or both types of HTTP metric events:

| Name | Type | Recommended | Notes and examples |
|--------------------|---------------------|-------------------|--------------------|
| `http.method` | `client` & `server` | Yes | The HTTP request method. E.g. `"GET"` |
| `http.host` | `client` & `server` | see [label alternatives](#label-alternatives) | The value of the [HTTP host header][]. When the header is empty or not present, this label should be the same. |
| `http.scheme` | `client` & `server` | see [label alternatives](#label-alternatives) | The URI scheme identifying the used protocol: `"http"` or `"https"` |
gfuller1 marked this conversation as resolved.
Show resolved Hide resolved
| `http.status_code` | `client` & `server` | Optional | [HTTP response status code][]. E.g. `200` (integer) |
Copy link
Member

Choose a reason for hiding this comment

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

We don't support integers. Not sure how this should be recorded.

Copy link

Choose a reason for hiding this comment

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

as a string (from the well known integer value).

Copy link
Member

@tigrannajaryan tigrannajaryan Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This conflicts with semantic convention of the equivalently named http.status_code span attribute.

This can be a source of confusion.

I believe we should prohibit using the same attribute name and metric label name but give them slightly different meanings or value sets.

Either change the name of the label here, or change the span attribute name or make sure they both use the same values if the same attribute name and label name is used (i.e. both numbers or both text).

Related to #815

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. This seems like it'll be a lot bigger discussion and if I decide what it should look like in this PR it'll just bring this PR to a halt. I think this point should involve discussion about standards for this kind of scenario as it will most likely happen again in the future.

Because of that I think the best thing to do here is to leave out the http.status_code label for http metric events for now until we have come to a decision on best practices for cases like this and then we can add it in after. That way we can unblock this work and continue into the implementation side of things.

| `http.status_text` | `client` & `server` | Optional | [HTTP reason phrase][]. E.g. `"OK"` |
Copy link
Member

Choose a reason for hiding this comment

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

Is this useful?

Copy link

@ebullient ebullient Aug 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I am not a fan of status_text / reason phrase myself. It is Optional, so I let it lie.

Copy link
Member

Choose a reason for hiding this comment

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

https://tools.ietf.org/html/rfc7231#section-6.1 says that

reason phrases listed here are only recommendations -- they can be replaced by local equivalents

Not that I have seen this actually happening but it is a possibility. If source start sending different reason phrase for the same status code that would arguably make the status text less useful since aggregations would be difficult/impossible to do.

Since we already have http.status_code perhaps just drop this label?

Copy link
Member

Choose a reason for hiding this comment

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

Ugh. I highly dislike dealing with reason phrases.
I can see an argument for leaving it in. I have interacted with a REST service that encoded important stuff in the reason phrase (like the difference between a 404 caused by a route that doesn't exist and a 404 because the requested object doesn't exist).

I vote we leave it in. If we do, it will likely result in all HTTP client instrumentation including it. I guess this is good because in the case of meaningful reason codes, it will be possible to include them on the final metric events, and the user can facet or filter on them in the UI.
And in the general case, this label doesn't add any more cardinality (though it does add a few more bytes per instrument).

| `http.flavor` | `client` & `server` | Optional | Kind of HTTP protocol used: `"1.0"`, `"1.1"`, `"2"`, `"SPDY"` or `"QUIC"`. |
Copy link
Member

Choose a reason for hiding this comment

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

Is "flavor" the right term or "version"?

Also, I am not sure "SPDY" or "QUIC" belong to the same set.

| `net.peer.name` | `client` | see [1] in [label alternatives](#label-alternatives) | The name of the service the request is going to. |
| `net.peer.port` | `client` | see [1] in [label alternatives](#label-alternatives) | The port of the service the request is going to. E.g. `8080` |
| `net.peer.ip` | `client` | see [1] in [label alternatives](#label-alternatives) | The IP address of the service the request is going to. E.g. `255.255.255.0` |
| `http.server_name` | `server` | see [2] in [label alternatives](#label-alternatives) | The primary server name of the matched virtual host. This should be obtained via configuration. If no such configuration can be obtained, this label MUST NOT be set ( `net.host.name` should be used instead). |
| `net.host.name` | `server` | see [2] in [label alternatives](#label-alternatives) | The name of the host. |
| `net.host.port` | `server` | see [2] in [label alternatives](#label-alternatives) | The port of the host. |
Copy link
Member

Choose a reason for hiding this comment

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

I suggest copying the description verbatim from https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/span-general.md#general-network-connection-attributes and also link to them to avoid any subtle differences that may be introduced here. Even better don't have any description here, only link to that document.

Copy link
Member

Choose a reason for hiding this comment

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

See also #739 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

@justinfoote will propose lifting the semantic conventions out of the trace folder, as they are not specific to tracing in a meaningful way that prevents their use in other areas.

Copy link
Contributor

Choose a reason for hiding this comment

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

(We would prefer to merge this and then refactor.)

Copy link
Contributor Author

@gfuller1 gfuller1 Aug 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

7de9d45 until that the refactor happens

Copy link
Member

Choose a reason for hiding this comment

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

As @jmacd mentioned: #855


[HTTP host header]: https://tools.ietf.org/html/rfc7230#section-5.4
[HTTP response status code]: https://tools.ietf.org/html/rfc7231#section-6
[HTTP reason phrase]: https://tools.ietf.org/html/rfc7230#section-3.1.2

#### Parameterized labels

To avoid high cardinality the following labels SHOULD substitute any parameters when added as labels to http metric events as described below:

| Label name | Type | Recommended | Notes and examples |
|-------------------|---------------------|-------------|---------------------|
|`http.url` | `client` & `server` | see [label alternatives](#label-alternatives) | The originally requested URL |
|`http.target` | `client` & `server` | see [label alternatives](#label-alternatives) | The full request target as passed in a [HTTP request line][] or equivalent, e.g. `"/path/{id}/?q={}"`. |

[HTTP request line]: https://tools.ietf.org/html/rfc7230#section-3.1.1

Many REST APIs encode parameters into URI path, e.g. `/api/users/123` where `123`
is a user id, which creates high cardinality value space not suitable for labels on metric events.
In case of HTTP servers, these endpoints are often mapped by the server
frameworks to more concise _HTTP routes_, e.g. `/api/users/{user_id}`, which are
recommended as the low cardinality label values. However, the same approach usually
does not work for HTTP client labels, especially when instrumentation is provided
by a lower-level middleware that is not aware of the specifics of how the URIs
are formed. Therefore, HTTP client labels SHOULD be using conservative, low
cardinality names formed from the available parameters of an HTTP request,
such as `"HTTP {METHOD_NAME}"`. These labels MUST NOT default to using URI
path.

#### Label alternatives

**[1]** For client metric labels, one of the following sets of labels is RECOMMENDED (in order of usual preference unless for a particular web client/framework it is known that some other set is preferable for some reason; all strings must be non-empty):

* `http.url`
* `http.scheme`, `http.host`, `http.target`
* `http.scheme`, `net.peer.name`, `net.peer.port`, `http.target`
* `http.scheme`, `net.peer.ip`, `net.peer.port`, `http.target`

**[2]** For server metric labels, `http.url` is usually not readily available on the server side but would have to be assembled in a cumbersome and sometimes lossy process from other information (see e.g. <https://github.com/open-telemetry/opentelemetry-python/pull/148>).
It is thus preferred to supply the raw data that *is* available.
Namely, one of the following sets is RECOMMENDED (in order of usual preference unless for a particular web server/framework it is known that some other set is preferable for some reason; all strings must be non-empty):

* `http.scheme`, `http.host`, `http.target`
* `http.scheme`, `http.server_name`, `net.host.port`, `http.target`
* `http.scheme`, `net.host.name`, `net.host.port`, `http.target`
* `http.url`