fix(instrumentation): http fallback method to _OTHER by Rahul-Baradol · Pull Request #8868 · open-telemetry/opentelemetry-go-contrib (original) (raw)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct to me for the behavior change in #8866, but I think there’s still one worthwhile test gap before merge.

The PR updates the unknown/empty-method fallback in every generated client.go, while the new assertions added here are server-side only. internal/shared/semconv/client_test.go.tmpl still doesn’t exercise the new client fallback path, which also matches the remaining Codecov gap on this PR.

I’d suggest adding two client-path assertions in the shared client test template so the coverage applies across all generated instrumentations:

• req.Method == "" should produce http.request.method = "_OTHER" with no http.request.method_original
• an unknown mixed-case method like "Unknown" should produce http.request.method = "_OTHER" and http.request.method_original = "Unknown"

With that and the changelog entry already mentioned on the PR, I don’t have other concerns.