Capture HTTP request/response headers as span attributes

[mateuszrzeszutek]

Fixes #1061

Changes

• Add semantic conventions for capturing HTTP request and response headers.

[Oberon00]

This convention is breaking new ground, because it only defines a prefix, not a full semantic convention. We don't really have support for that in the semantic convention generator (yet?), so it's fine to only have it as a markdown table.

There are a few questions that arise from this convention, that I'll post as separate comments.

[jsuereth]

Should HTTP Request/Response be an "Event" with these as attributes on that event?

[yurishkuro]

Should HTTP Request/Response be an "Event" with these as attributes on that event?

I'd say no, because there are no meaningful timestamps one could attach to such events that are different from the span start/end.

[Oberon00]

After seeing #1907, I wonder if my proposal to preserve casing was so good after all, or if we need something additional. IMHO a good goal would be to be able to capture header information in such a way that #1907 is resolved as well. Some ideas:

1. Normalize header names to lowercase, - to _ (like lowercased CGI envvars), do not preserve casing.
2. Optionally capture a list of configured headers normalized (in addition to case-preserving or only). The prefix could be the same or http.(request|response).normalized_header or similar. Or we could rename the current capturing to raw_header. The normalization could be done by the instrumentation, a span processor (if we have Add BeforeEnd to have a callback where the span is still writeable #1089), the exporter, the collector or even the backend.
3. Just state that normalization / accessing keys in a case-insensitive way is the responsibility of whoever processes the data (maybe in addition to defining the normalization but define nothing more. I don't think that would be good enough to satisfy proposals like Capture request/response media type #1907 in the future though.

CC @Blacksmoke16

[Blacksmoke16]

@Oberon00 HTTP/2 requires headers be lowercase while in HTTP/1 it doesn't matter. Normalizing them to all lowercase would be backwards compatible, while also being future proof.

[mateuszrzeszutek]

1. Normalize header names to lowercase, - to _ (like lowercased CGI envvars), do not preserve casing.

I like this idea because it's the simplest out of the three you suggested - and the most intuitive one ("why don't we lowercase headers" was the first question I asked in the linked issue 😄 ). We do lose a little bit of information that way, but since HTTP headers are supposed to be case insensitive by design it's probably not worth that much.
I'll rework my PR with idea 1. in mind.

[carlosalberto]

Why not have this as attributes with "type" map[string][]string

Since we don't support this now (and probably it will take a little while before we do so), should we proceed with this approach for now? @bogdandrutu @yurishkuro

[Oberon00]

and probably it will take a little while before we do so

You are assuming that we will support it 😉 That's not decided at all.

[carlosalberto]

That's not decided at all.

Yeah, definitely. More the reason to go with the current approach.

[Oberon00]

My thoughts: I'm somewhat strongly against adding support for more complex attributes, like maps. I think this PR should be merged as-is, and then we should close #376 because the only use case that came up so far has been solved differently. What would the purpose of adding maps after this PR be? The added-value of maps is already very small (negative IMHO), and if we add maps after this PR, there is one more negative to it, namely the disruption it causes for this semantic convention.

[Oberon00]

I'm against having the value be set as a single string instead of single-element array, as it makes typing more complicated. See #1898 (comment)

[carlosalberto]

Will merge by EOD unless somebody opposes.

[mateuszrzeszutek]

@open-telemetry/specs-approvers @open-telemetry/specs-trace-approvers please take a look at this PR 🙏

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Mateusz Rzeszutek (9bdf1e6, b11f7af, fb11220, 5b2f882, ad335c1, 19823de, d39e727, bfb35dd, 12151ec, 749a9cb)
• ✅ Yuri Shkuro (05e3149)