fix: complete urllib3 instrumentation (#1059) — address review from #1744

[alloevil]

Summary

Completes the urllib3 instrumentation integration for logfire, addressing review feedback from PR #1744.

Fixes #1059

Changes

Based on PR #1744 by @Br1an67, with the following fixes:

1. Realistic VCR cassette: Replaced fake example.org:8080 cassette with httpbin.org — the previous cassette had a fake URL returning 200 OK with an empty body, which didn't represent a real HTTP interaction.

2. Updated test assertions: Test URL, host, port, and path now match the cassette (httpbin.org:443/get instead of example.org:8080/foo).

3. VCR instead of monkeypatching: The test uses @pytest.mark.vcr() as requested by @alexmojaki.

What's preserved from #1744

• logfire/_internal/integrations/urllib3.py — core integration wrapping opentelemetry-instrumentation-urllib3
• logfire/_internal/main.py — instrument_urllib3 method on Logfire class
• logfire/__init__.py and logfire-api/ — exports
• pyproject.toml — urllib3 optional dependency
• docs/integrations/http-clients/urllib3.md — documentation
• mkdocs.yml — nav entry

Testing

The test verifies that:

• urllib3 requests are automatically instrumented with spans
• Span attributes include correct HTTP method, URL, status code, and metrics
• The integration works within a parent span context

Closes #1059

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds urllib3 instrumentation support to Logfire. A new logfire/_internal/integrations/urllib3.py module wraps URLLib3Instrumentor from opentelemetry-instrumentation-urllib3, with a runtime check that raises a RuntimeError if the package is absent. A Logfire.instrument_urllib3() method is added to logfire/_internal/main.py and exposed as a module-level export in logfire/__init__.py. Type stubs in logfire-api are updated with fully-typed hook signatures for request and response callbacks. The new optional dependency is declared in pyproject.toml, tests cover both the happy path via a recorded cassette and the missing-dependency error scenario, and documentation is added under docs/integrations/http-clients/urllib3.md with navigation wired into mkdocs.yml.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: completing urllib3 instrumentation and addressing review feedback. It's concise and specific.
Description check	✅ Passed	The description comprehensively explains the changes made, including realistic cassette replacement, test updates, and preserved components from prior work.
Linked Issues check	✅ Passed	The PR fully addresses issue #1059 by providing native urllib3 instrumentation through logfire's public API, eliminating the need for direct OpenTelemetry configuration.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with implementing urllib3 instrumentation: integration code, exports, documentation, tests, dependencies, and configuration.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

2 issues found across 13 files

Confidence score: 3/5

• In logfire/_internal/integrations/urllib3.py, the broad except ImportError: can catch unrelated import failures, so real dependency breakages may be misclassified as “urllib3 missing” and hide a concrete runtime problem—narrow this to ModuleNotFoundError and check exc.name before merging.
• The same import-handling path in logfire/_internal/integrations/urllib3.py drops explicit chaining, which makes stack traces harder to interpret and slows diagnosis if import failures reach users—use raise ... from err to preserve original context and de-risk support/debugging.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="logfire/_internal/integrations/urllib3.py">

<violation number="1" location="logfire/_internal/integrations/urllib3.py:11">
P2: Broad `except ImportError:` masks unrelated dependency/import failures; discriminate `ModuleNotFoundError` by `exc.name` instead.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Broad except ImportError: masks unrelated dependency/import failures; discriminate ModuleNotFoundError by exc.name instead.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At logfire/_internal/integrations/urllib3.py, line 11:

<comment>Broad `except ImportError:` masks unrelated dependency/import failures; discriminate `ModuleNotFoundError` by `exc.name` instead.</comment>

<file context>
@@ -0,0 +1,37 @@
+
+try:
+    from opentelemetry.instrumentation.urllib3 import RequestInfo, URLLib3Instrumentor
+except ImportError:
+    raise RuntimeError(
+        '`logfire.instrument_urllib3()` requires the `opentelemetry-instrumentation-urllib3` package.\n'
</file context>

[alloevil]

Hi! I'm having trouble identifying the lint failure — the CI log just shows "Process completed with exit code 1" without details. Could you help me understand what's failing?

I've fixed:

1. ✅ except ImportError as _err: raise ... from _err (explicit exception chaining)
2. ✅ Callable imported from collections.abc instead of typing (UP035)
3. ✅ uv.lock updated with opentelemetry-instrumentation-urllib3

Is there a way to see the detailed pre-commit output, or is there a specific check I'm missing?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cubic-dev-ai]

You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment @cubic-dev-ai review.

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

_{Re-trigger cubic}

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🚩 Integrations doc table missing urllib3 row

The bullet list at docs/integrations/index.md:35 was updated to include urllib3, but the detailed integrations table (lines 43–80) was not updated with a corresponding row for urllib3. Every other integration listed in the bullet points has a matching row in the table (e.g., Requests at line 72, HTTPX at line 59). This is a documentation completeness gap rather than a code bug.

---

Was this helpful? React with 👍 or 👎 to provide feedback.