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

What to do with non well-known http.method values? #3470

Closed
trask opened this issue May 3, 2023 · 6 comments
Closed

What to do with non well-known http.method values? #3470

trask opened this issue May 3, 2023 · 6 comments
Assignees
Labels
area:semantic-conventions Related to semantic conventions semconv:HTTP spec:metrics Related to the specification/metrics directory spec:trace Related to the specification/trace directory

Comments

@trask
Copy link
Member

trask commented May 3, 2023

http.method is a required field in both traces and metrics, but it's not clear what instrumentation should do with non well-known values, e.g. if someone sends a request with http method XYZ, should instrumentation preserve that value? or normalize it to something like OTHER?

on the tracing side it seems to make sense to preserve the XYZ value, but on the metrics side this could leave you open to cardinality problems, so it seems to make sense to normalize it to OTHER on the metrics side.

but having different values captured for traces and metrics seems bad, so I think it makes sense to normalize it to OTHER for both.

@trask trask added spec:metrics Related to the specification/metrics directory spec:trace Related to the specification/trace directory labels May 3, 2023
@trask trask assigned trask and unassigned jmacd May 3, 2023
@Oberon00
Copy link
Member

Oberon00 commented May 4, 2023

I don't think there are "invalid" HTTP method values. Using custom HTTP verbs seems unusual but valid. Special considerations may apply to span names (and maybe metric attributes?) but otherwise I see no reason to scrub non-well-known values.

https://www.rfc-editor.org/rfc/rfc9110.html#section-9.1-5:

The method token is case-sensitive because it might be used as a gateway to object-based systems with case-sensitive method names. By convention, standardized methods are defined in all-uppercase US-ASCII letters.

https://www.rfc-editor.org/rfc/rfc9110.html#section-9.1-11:

Additional methods, outside the scope of this specification, have been specified for use in HTTP. All such methods ought to be registered within the "Hypertext Transfer Protocol (HTTP) Method Registry", as described in Section 16.1.

@trask trask changed the title What to do with invalid http.method values? What to do with non well-known http.method values? May 4, 2023
@trask
Copy link
Member Author

trask commented May 4, 2023

I don't think there are "invalid" HTTP method values

good point, thx, I've updated the title and description

Special considerations may apply to span names (and maybe metric attributes?) but otherwise I see no reason to scrub non-well-known values.

I think it's a bit awkward to have different definition of http.method for traces and metrics, so I guess my preference would be to only capture well-known values by default and scrub others to OTHER. And instrumentation could offer an option to expand the list of "well-known values" if needed.

@Oberon00
Copy link
Member

Oberon00 commented May 4, 2023

I'm torn on this. I think the safe option is to add the on-by-default optional special case for non-well-known verbs. But it comes with some information loss especially in "interesting" cases and also you'll have to add the list of verbs.

Also we might break certain cases we don't know of. And I am not sure a "on/off" switch is good enough here, because if you need the normalization to be protected from flooding, this is independent from whether you use custom verbs or not. So you'd rather have a configurable list of accepted verbs.

Maybe this is better scrubbed/normalized on the collector?

If still possible, we may want to remove the assumption that http.method is low-cardinality. E.g. remove it from the span name suggestions and whatever is required in the metrics.

@trask
Copy link
Member Author

trask commented Jun 19, 2023

@open-telemetry/technical-committee can you post the results of the vote here? thx!

@reyang
Copy link
Member

reyang commented Jun 19, 2023

@open-telemetry/technical-committee met and voted last Wednesday (Jun. 14th, 2023) and here are the results:

Please close other options and push Option A through the finishing line.

@reyang
Copy link
Member

reyang commented Jun 21, 2023

I think this is resolved by open-telemetry/semantic-conventions#17.

@reyang reyang closed this as completed Jun 21, 2023
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 semconv:HTTP spec:metrics Related to the specification/metrics directory spec:trace Related to the specification/trace directory
Projects
5 participants