do instr-fetch and instr-xhr *need* to provide configurability of "HTTP known methods" for `http.request.method`?

[trentm]

trafficstars

Good point. It'll "work" but get undefined everytime, because it'll get this implementation: https://github.com/open-telemetry/opentelemetry-js/blob/v2.0.0/packages/opentelemetry-core/src/platform/browser/environment.ts#L29-L31

So this should probably change. BTW, the same code exists in instrumentation-fetch currently.

The spec says:

If the HTTP instrumentation could end up converting valid HTTP request methods to _OTHER, then it MUST provide a way to override the list of known HTTP methods. If this override is done via environment variable, then the environment variable MUST be named OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS and support a comma-separated list of case-sensitive known HTTP methods (this list MUST be a full override of the default known method, it is not a list of known methods in addition to the defaults).

I'm not sure if this instrumentation qualifies for If the HTTP instrumentation could end up converting valid HTTP request methods to _OTHER. Thoughts?

The alternative would be to have a httpKnownMethods config option to this instrumentation.

Originally posted by @trentm in https://github.com/open-telemetry/opentelemetry-js/pull/5662#discussion_r2082368675

the issue

When updating instr-fetch and instr-xhr to support HTTP semconv migration (https://opentelemetry.io/docs/specs/semconv/non-normative/http-migration/) I revisited how most of the HTTP-related span attributes are specified. Surprisingly subtle is http.request.method. Semconv requires this attr to be a normalized value and includes the quoted paragraph above about "MUST provide a way to override the list of known HTTP methods". That's conditional on "If the HTTP instrumentation could end up converting valid HTTP request methods to _OTHER,".

It isn't clear to me if that applies to instr-fetch and instr-xhr. What is "valid HTTP request methods" here? The implementation I wrote includes all the methods covered by this spec statement: "By default, this convention defines "known" methods as the ones listed in RFC9110 and the PATCH method defined in RFC5789."

Being uncertain, I implemented support for the OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS envvar to override. However, instr-fetch and instr-xhr are web instrumentations, so of course envvars aren't a thing. So something should change here.

Related:

• OTel JS's instr-http and instr-undici do not support configurability of HTTP known methods.
• OTel Java does (via envvar and system property).

Options:

1. Drop the OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS handling in instr-fetch and instr-xhr and hence do not provide configurability of known HTTP methods.
2. Add something like knownHttpMethods config option to these instrumentations as an in-code method to configure this.

TODO:

• Dig into the history when this was added to semconv to try to glean the intent.
• What do other OTel lang SDKs do?
• Ask for clarification in a spec/semconv issue.