Skip to content

Fix return type annotation of _parse_trace_id_header in Jaeger propagator#5440

Open
Eason09053360 wants to merge 3 commits into
open-telemetry:mainfrom
Eason09053360:fix-jaeger-parse-trace-id-header-annotation
Open

Fix return type annotation of _parse_trace_id_header in Jaeger propagator#5440
Eason09053360 wants to merge 3 commits into
open-telemetry:mainfrom
Eason09053360:fix-jaeger-parse-trace-id-header-annotation

Conversation

@Eason09053360

@Eason09053360 Eason09053360 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Description

_parse_trace_id_header in the Jaeger propagator is annotated as returning tuple[int] (a tuple of exactly one element), but every return path yields a 3-tuple (trace_id, span_id, flags) and the caller unpacks three values. This misleads type checkers.

This PR fixes the annotation to tuple[int, int, int]. Annotation-only change, no runtime impact.
_extract_baggage — Add complete type annotation

Type of change

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

How Has This Been Tested?

  • Annotation-only change; existing propagator/opentelemetry-propagator-jaeger tests cover the extraction behavior.

  • tox -e typecheck

typecheck: commands[0]> pyright --version
WARNING: there is a new pyright version available (v1.1.405 -> v1.1.411).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest`

pyright 1.1.405
typecheck: commands[1]> pyright
WARNING: there is a new pyright version available (v1.1.405 -> v1.1.411).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest`

0 errors, 0 warnings, 0 informations
  typecheck: OK (2.88=setup[0.01]+cmd[0.52,2.35] seconds)
  congratulations :) (2.99 seconds)

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated

Eason09053360 added a commit to Eason09053360/opentelemetry-python that referenced this pull request Jul 19, 2026
@Eason09053360
Eason09053360 marked this pull request as ready for review July 19, 2026 13:57
@Eason09053360
Eason09053360 requested a review from a team as a code owner July 19, 2026 13:57
…ator

The function returns a 3-tuple (trace_id, span_id, flags) on every code
path and callers unpack three values, but the annotation declared
tuple[int] (a one-element tuple), which misleads type checkers.
@Eason09053360
Eason09053360 force-pushed the fix-jaeger-parse-trace-id-header-annotation branch from 75f22c1 to 9fcea30 Compare July 24, 2026 01:00
@xrmx

xrmx commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Change looks good, any chance you can fix all the typing issue in the package so we can enable typechecking? Your PR already fixes half of them. Add the following and then uv run tox -e typecheck

diff --git a/pyproject.toml b/pyproject.toml
index 51bc40983..302fc593c 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -133,7 +133,8 @@ include = [
   "exporter/opentelemetry-exporter-otlp-proto-http",
   "exporter/opentelemetry-exporter-otlp-json-common",
   "exporter/opentelemetry-exporter-otlp-common",
-  "codegen/opentelemetry-codegen-json"
+  "codegen/opentelemetry-codegen-json",
+  "propagator/opentelemetry-propagator-jaeger",
 ]
 
 exclude = [
@@ -154,6 +155,7 @@ exclude = [
   "exporter/opentelemetry-exporter-otlp-json-common/tests",
   "exporter/opentelemetry-exporter-otlp-common/tests",
   "exporter/opentelemetry-exporter-otlp-json-common/benchmarks",
+  "propagator/opentelemetry-propagator-jaeger/tests",
 ]
 
 # When packages are correct typed add them to the strict list

@xrmx xrmx moved this to Easy to review / merge / close in Python PR digest Jul 24, 2026
Parameterize Getter/Setter with CarrierT to match the base class
signatures, guard against None baggage values before unquoting, type
header values as str instead of the unbound CarrierT, and make
_int_from_hex_str return int | None with explicit fallbacks at the call
sites. Add the package to the pyright include list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Easy to review / merge / close

Development

Successfully merging this pull request may close these issues.

2 participants