Skip to content

opentelemetry-util-http: don't raise on unparsable request targets - #4965

Open
henry3260 wants to merge 1 commit into
open-telemetry:mainfrom
henry3260:parse-url-query-invalid-url
Open

opentelemetry-util-http: don't raise on unparsable request targets#4965
henry3260 wants to merge 1 commit into
open-telemetry:mainfrom
henry3260:parse-url-query-invalid-url

Conversation

@henry3260

Copy link
Copy Markdown
Contributor

Description

_parse_url_query in opentelemetry-util-http called urlparse on request targets that come
straight off the request line. urlparse raises ValueError ("Invalid IPv6 URL") on targets such
as //exa[mple, and because the exception propagated out of instrumentation, tornado turned those
requests into 500s. It now returns empty components instead.

The components are deliberately not recovered by splitting on ?: url.query is optional under the
semantic conventions, and every current caller sources url.path from the framework
(request.path in tornado, the route template in asgi), so nothing downstream regresses. Preserving
the query for unparsable targets would be a separate change.

Related to #4447, which was closed by #4551 by avoiding _parse_url_query in the WSGI
instrumentation; the same root cause remained in the shared helper for the other callers.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • New util/opentelemetry-util-http/tests/test_parse_url_query.py covers absolute urls,
    request-target-only urls, no query, empty url, and unparsable urls.
  • New tornado regression test test_invalid_request_uri_does_not_break_request asserts that
    //exa[mple still answers 404 with http.target and http.status_code set. Reverting the
    helper makes it fail with 500 != 404.
  • uv run tox -e py313-test-util-http — 66 passed.
  • uv run tox -e py313-test-instrumentation-tornado — 46 passed. test_remove_sensitive_params
    fails locally because http_server_mock cannot bind port 5000; it fails the same way without
    this change.
  • uv run pre-commit run ruff --all-files and ruff-format pass.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@henry3260
henry3260 requested a review from a team as a code owner August 17, 2026 12:55
@henry3260
henry3260 force-pushed the parse-url-query-invalid-url branch from 5999209 to 4c956ac Compare August 17, 2026 12:56
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 17, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on maintainers · refreshed 2026-08-21 01:51 UTC

Merge when ready.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

…le request target so tornado no longer turns such requests into 500s
@henry3260
henry3260 force-pushed the parse-url-query-invalid-url branch from 4c956ac to d45a910 Compare August 17, 2026 16:32
@github-project-automation github-project-automation Bot moved this to Approved PRs in Python PR digest Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved PRs

Development

Successfully merging this pull request may close these issues.

3 participants