Skip to content

Raise opentelemetry-sdk ceiling to <1.44.0 (allow 1.43)#2042

Open
hramezani wants to merge 2 commits into
pydantic:mainfrom
hramezani:raise-otel-sdk-ceiling
Open

Raise opentelemetry-sdk ceiling to <1.44.0 (allow 1.43)#2042
hramezani wants to merge 2 commits into
pydantic:mainfrom
hramezani:raise-otel-sdk-ceiling

Conversation

@hramezani

@hramezani hramezani commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Closes #2041

Summary

logfire pins opentelemetry-sdk<1.43.0 (and the matching opentelemetry-exporter-otlp-proto-http). That ceiling transitively blocks opentelemetry-instrumentation-fastapi 0.64b0, which requires opentelemetry-sdk>=1.43 and contains the fix for the FastAPI ≥0.137 _IncludedRouter regression — _get_route_details() raises AttributeError, 500-ing every CORS preflight (OPTIONS) request (open-telemetry/opentelemetry-python-contrib#4700).

Anyone on logfire.instrument_fastapi() is therefore effectively pinned to FastAPI <0.137.

This bumps the upper bound to <1.44.0 so the 1.43.0 line (and fastapi-instrumentation 0.64b0) can be installed.

-    "opentelemetry-sdk >= 1.39.0, < 1.43.0",
-    "opentelemetry-exporter-otlp-proto-http >= 1.39.0, < 1.43.0",
+    "opentelemetry-sdk >= 1.39.0, < 1.44.0",
+    "opentelemetry-exporter-otlp-proto-http >= 1.39.0, < 1.44.0",

Verification

Installed the exact combination from the issue (fastapi 0.138.x + opentelemetry-sdk 1.43.0 + opentelemetry-instrumentation-fastapi 0.64b0) and ran the relevant suites:

  • tests/otel_integrations/test_fastapi.py, test_starlette.py, test_asgi.pyall pass (22).
  • Core config/exporter tests compared on 1.42.1 vs 1.43.0 on the same interpreter: no functional regression, only test-snapshot churn from two upstream OTel changes (both handled below).

Test robustness for 1.43 (included in this PR)

OTel 1.43 makes two output changes that only surface in tests, addressed so CI stays green whether the lock resolves 1.42 or 1.43:

  1. service.instance.id is now a dashed UUID4 on the default resource (was the 32-char uuid4().hex form in ≤1.42). The TestExporter normaliser only recognised the hex form, so a random UUID leaked into every resource snapshot. Fixed centrally in logfire/_internal/exporters/test.py by normalising the dashed form to the same fixed value — no snapshot edits needed, and it works on both versions.
  2. Serialized protobuf body size changed (897045 → 897108 bytes), so test_max_body_size_bytes now asserts the message shape via a regex instead of a hardcoded byte count.

Both fixes were verified against opentelemetry-sdk 1.42.1 and 1.43.0 (affected tests pass on each).

Note: as of 2026-07-01 the 1.43.0 line has aged past uv's exclude-newer = "1 week" window, so a fresh uv lock --upgrade now resolves 1.43.0 / 0.64b0. The committed lock is intentionally left at 1.42.1 (this PR only raises the ceiling); the test-robustness changes above ensure CI won't break when the lock is next regenerated.

🤖 Generated with Claude Code

Review in cubic

logfire pinned `opentelemetry-sdk<1.43.0`, which transitively blocked
`opentelemetry-instrumentation-fastapi 0.64b0` — the release containing
the fix for the FastAPI >=0.137 `_IncludedRouter` regression that 500s
every CORS preflight (open-telemetry/opentelemetry-python-contrib#4700).

Raise the upper bound on `opentelemetry-sdk` and
`opentelemetry-exporter-otlp-proto-http` to `<1.44.0` so the 1.43.0 line
(and fastapi-instrumentation 0.64b0) can be installed.

Closes pydantic#2041

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@alexmojaki

Copy link
Copy Markdown
Collaborator

Today's lockfile still resolves 1.42.1 (1.43.0 is within uv's exclude-newer = "1 week" window), so CI is unaffected until it ages in.

This is the key problem

@hramezani hramezani requested a review from alexmojaki July 1, 2026 07:54
OpenTelemetry 1.43 changed the `service.instance.id` resource attribute
from the 32-char `uuid4().hex` form to a dashed UUID4. The TestExporter
normaliser only recognised the hex form, so on 1.43 a random UUID leaked
into every resource snapshot. Match the dashed form too and normalise it
to the same fixed value, keeping all existing snapshots valid on both
versions.

Also relax `test_max_body_size_bytes`: the exact serialised body size
depends on the OTel version (897045 on 1.42, 897108 on 1.43), so assert
the message shape via a regex rather than a hardcoded byte count.

Verified the affected tests pass on both opentelemetry-sdk 1.42.1 and
1.43.0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
return 1234
if name == 'service.instance.id':
if re.match(r'^[0-9a-f]{32}$', value):
# OpenTelemetry <=1.42 set this to `uuid4().hex` (32 hex chars); 1.43+ uses the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No, otel didn't set this before at all, logfire did. This points to the need for a change in config.py, not tests.

The otel change is here: https://github.com/open-telemetry/opentelemetry-python/pull/5259/changes#diff-d2c46fd8da8afdf67c6ad3dd42b0a243c5049f3c720d9bb3351f1d5522338b06R472

The spec is here: https://opentelemetry.io/docs/specs/semconv/registry/attributes/service/#service-instance-id

Also see the docs: https://pydantic.dev/docs/logfire/reference/sql/#service_instance_id

The otel change is causing a change in the documented logfire behaviour. logfire generates a new random UUID for each call to logfire.configure(), whereas it looks like otel holds onto a stable global value. A decision needs to be made about this.

cc @dmontagu who was recently involved in code changes in this area.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have two options:

  • Defer to OTel — drop our fallback, update docs
  • Preserve current behaviour — re-assert our own id above OTel's detector while still letting OTEL_RESOURCE_ATTRIBUTES/user detectors win.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise opentelemetry-sdk ceiling to allow >=1.43 (blocks otel-instrumentation-fastapi 0.64b0 FastAPI 0.137 fix)

2 participants