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

Capture request/response media type #1907

Closed
Blacksmoke16 opened this issue Sep 7, 2021 · 6 comments
Closed

Capture request/response media type #1907

Blacksmoke16 opened this issue Sep 7, 2021 · 6 comments
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory

Comments

@Blacksmoke16
Copy link

What are you trying to achieve?

Capture media type information for a request/response.

Additional context.

Currently the semantic conventions for HTTP spans do not include any attributes related to the media type of the request/response. This makes it impossible to distinguish a request of one type from another of a different type to the same endpoint without manual instrumentation. E.g. how OTLP supports both JSON and protobuf payloads or an API returning JSON or XML encoded data based on the request's Accept header.

Partially related to #1898, as this would be another case of the content-type header being already captured.

The main questions that need figured out before a PR:

  1. What should it be called? (request|response)_content_type or (request|response)_media_type
  2. Should it also contain charset and/or boundary directives? Or should those be separate nested attributes?
  3. Any reason to breakup the type/subtype?
@Blacksmoke16 Blacksmoke16 added the spec:trace Related to the specification/trace directory label Sep 7, 2021
@Oberon00
Copy link
Member

Oberon00 commented Sep 7, 2021

Wouldn't it be enough to represent this just as http.(request|response).header.Content-Type? I guess not normalizing the header casing is annoying for that use case 😕

@Oberon00
Copy link
Member

Oberon00 commented Sep 7, 2021

If we already have header capturing we should try to fulfill this use case as well. I would prefer not having to define a new semantic convention for every header. But I agree that the current header proposal may be insufficient.

@Blacksmoke16
Copy link
Author

@Oberon00 My thought with that is if you're searching for that value wouldn't that treat application/json and application/json; charset=UTF-8 as separate values?

@Oberon00
Copy link
Member

Oberon00 commented Sep 7, 2021

That is true. But I don't think instrumentations should start parsing HTTP header values. This should rather be done on the backend. I wonder if OpenTelemetry should define semantic conventions for such a use case nonetheless

@Blacksmoke16
Copy link
Author

@Oberon00 That's fair. Some backends have the ability to create derived columns which could be a good way to handle it. I.e. regex match out the charset and stuff from the singular http.headers.content-type header.

This is also similar to #640 as the majority of that usecase could also be solved via http.headers.content-length, at least the size used for that value. I.e. might not be able to differentiate from uncompressed size versus compressed size.

@Blacksmoke16
Copy link
Author

Probably good to close this with the introduction of #1898. Feel free to re-open if you disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory
Projects
None yet
Development

No branches or pull requests

3 participants