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

http.response.header.content-length and http.request.header.content-length attributes are missing #1439

Closed
pellared opened this issue Sep 30, 2024 · 5 comments · Fixed by #1444
Assignees
Labels

Comments

@pellared
Copy link
Member

Area(s)

area:http

What happened?

The http.response.header.content-length and http.request.header.content-length attributes are missing or the deprecated attributes have improper description.

See: https://github.com/search?q=repo%3Aopen-telemetry%2Fsemantic-conventions%20http.response.header.content-length&type=code

CC @IdlePhysicist

Semantic convention version

v1.27.0

Additional context

Discussed in open-telemetry/opentelemetry-go#5853

@trask
Copy link
Member

trask commented Sep 30, 2024

http.response.header.content-length is a specialization of http.response.header.<key>

maybe we can make this clearer in the deprecation note

@trask
Copy link
Member

trask commented Oct 2, 2024

@pellared let me know if #1444 addresses the confusion, thanks!

@IdlePhysicist
Copy link

Hi @trask, I am the one who opened the discussion that lead to this issue being filed.

Firstly thanks for putting up your PR, but your clarification doesn't have much contextual impact for me. I understand that there are potentially many keys under http.response.header & http.request.header.

What I think I am missing is that why there is no HTTPRequestHeaderContentLengthKey or anything prefixed with HTTPRequestHeader? For every other deprecation that I encountered, when updating from semconv v1.24.0 to semconv v1.26.0, there was a corresponding replacement constant in the Go package (the example that I gave in my original post was semconv.HTTPURLKey -> semconv.URLFullKey). I suspect that the issue is where does one stop? Having a constant for each and every key is likely a path to madness.

I am wondering if we should be using attribute.Key("<key>") in general going forward and defining our own constants in code if or as needed?

@trask
Copy link
Member

trask commented Oct 3, 2024

hi @IdlePhysicist!

Having a constant for each and every key is likely a path to madness.

http.request.header.<key> is a special kind of semantic attribute because there are infinite number of different HTTP headers:

| `http.request.header.<key>` | string[] | HTTP request headers, `<key>` being the normalized HTTP Header name (lowercase), the value being the header values. [1] | `http.request.header.content-type=["application/json"]`; `http.request.header.x-forwarded-for=["1.2.3.4", "1.2.3.5"]` | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |

I'm not sure if/how Go encodes these special semantic attributes, probably @pellared can guide you better there. Here's how they work in Java:

https://github.com/open-telemetry/semantic-conventions-java/blob/dd1d40c6be372bc73e203307d4a02c24c5ce89f2/semconv/src/main/java/io/opentelemetry/semconv/HttpAttributes.java#L33-L34

@jsuereth
Copy link
Contributor

jsuereth commented Oct 3, 2024

Go (and the collector) do not support template attribute types in their semconv gen. There's actually a decent class of things not supported in Go that have evolved in Semconv.

We had a discussion withing the WG recently (about this). @MrAlias and @dashpole may be able to give a better estimate on when those may be supported in Go. I haven't been able to keep up to date on the status there myself.

References open-telemetry/opentelemetry-go#5668 so this context is available for whomever updates semconv for Go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants