Skip to content

feat: surface X-Logfire-Warning header#1906

Merged
alexmojaki merged 8 commits into
mainfrom
add-server-response-headers
May 13, 2026
Merged

feat: surface X-Logfire-Warning header#1906
alexmojaki merged 8 commits into
mainfrom
add-server-response-headers

Conversation

@adriangb
Copy link
Copy Markdown
Member

@adriangb adriangb commented May 5, 2026

Gives the Logfire backend an out-of-band channel — independent of response bodies — to deprecate endpoints or signal hard failures to any client running this SDK.

A new logfire/_internal/server_response.py module installs a requests response hook on every Logfire-bound HTTP session (OTLP exporters, token validation, CRUD client, variables provider, CLI). Each Logfire response is checked for the custom headers X-Logfire-Warning: <message> which calls warnings.warn(message, LogfireServerWarning). Python's default filter dedupes identical messages within a process, so a chatty server only warns once.

@adriangb adriangb requested review from Viicos and alexmojaki May 5, 2026 14:24
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 5, 2026

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: ee581a9
Status: ✅  Deploy successful!
Preview URL: https://a713b4fc.logfire-docs.pages.dev
Branch Preview URL: https://add-server-response-headers.logfire-docs.pages.dev

View logs

Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

No issues found across 7 files

Confidence score: 5/5

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

adriangb added a commit that referenced this pull request May 5, 2026
Per review (#1905#discussion_r3189043209), the X-Logfire-Warning /
X-Logfire-Error response-hook handling is logically independent from
the X-Logfire-Telemetry request header — they just happened to be
introduced together. Moved that code (plus its exceptions, install
calls, and tests) to a separate PR (#1906) so each can be reviewed
and landed on its own.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread logfire/_internal/server_response.py Outdated
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

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/config.py">

<violation number="1" location="logfire/_internal/config.py:1333">
P2: `_lazy_init_variable_provider` (line 1462) creates a `LogfireRemoteVariableProvider` without passing `transport_response_hook`, so lazily-initialized providers won't surface server warning/error headers. Add `transport_response_hook=self.advanced.transport_response_hook` there as well.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread logfire/_internal/config.py Outdated
@adriangb adriangb requested a review from alexmojaki May 6, 2026 17:37
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

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

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/config.py">

<violation number="1" location="logfire/_internal/config.py:222">
P3: The new callback example uses an undefined variable (`response`) instead of the `helper` parameter.</violation>
</file>

<file name="logfire/_internal/server_response.py">

<violation number="1" location="logfire/_internal/server_response.py:42">
P2: Use an explicit `None` check for `hook`; truthiness can skip valid custom callbacks and unexpectedly run the default hook.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Comment thread logfire/_internal/server_response.py Outdated
Comment thread logfire/_internal/config.py Outdated
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

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/types.py">

<violation number="1" location="logfire/types.py:291">
P1: `default_hook()` no longer processes `X-Logfire-Error`, so server-signaled hard failures are ignored.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Comment thread logfire/types.py
adriangb and others added 8 commits May 13, 2026 00:53
Adds a `requests` response hook that the SDK installs on every
Logfire-bound HTTP session (OTLP exporters, token validation, CRUD
client, variables provider, CLI). The hook reads two custom headers
the server attaches to responses:

* `X-Logfire-Warning` -> `warnings.warn(..., LogfireServerWarning)`.
  Python's default filter dedupes identical messages within a process,
  so a chatty server only warns once.
* `X-Logfire-Error` -> raises `LogfireServerError`. OTLP/variables
  paths already swallow exceptions from their HTTP calls; CRUD/CLI
  propagate the error to the user.

This gives the backend an out-of-band channel to deprecate endpoints
or signal hard failures without piggy-backing on response bodies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lets users intercept every Logfire API response (OTLP exports, credential
init, variables provider). Default keeps the existing X-Logfire-Warning /
X-Logfire-Error handling; pass `lambda response: None` to opt out, or
compose around `process_logfire_response_headers`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The example references `logfire`/`my_metric` without imports, which
test_docs runs through pytest-examples and trips on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`_lazy_init_variable_provider` was constructing `LogfireRemoteVariableProvider`
without the configured hook, so providers spun up via the lazy path bypassed
the user-supplied response handling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop LogfireServerError and X-Logfire-Error handling; default hook now only
emits LogfireServerWarning from X-Logfire-Warning. Also fixes cubic review
comments: explicit None check on the hook callback, and fix the docstring
example to use helper.response.status_code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adriangb added a commit that referenced this pull request May 13, 2026
Per review (#1905#discussion_r3189043209), the X-Logfire-Warning /
X-Logfire-Error response-hook handling is logically independent from
the X-Logfire-Telemetry request header — they just happened to be
introduced together. Moved that code (plus its exceptions, install
calls, and tests) to a separate PR (#1906) so each can be reviewed
and landed on its own.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adriangb adriangb force-pushed the add-server-response-headers branch from 80334c4 to ee581a9 Compare May 13, 2026 04:55
@alexmojaki alexmojaki changed the title feat: surface X-Logfire-Warning / X-Logfire-Error response headers feat: surface X-Logfire-Warning header May 13, 2026
@alexmojaki alexmojaki merged commit 26a5bf1 into main May 13, 2026
16 checks passed
@alexmojaki alexmojaki deleted the add-server-response-headers branch May 13, 2026 11:00
adriangb added a commit that referenced this pull request May 13, 2026
Adds an `X-Logfire-Telemetry: key=val,...` header to every request the SDK
sends (OTLP exports, token validation, CRUD calls, variable provider, CLI),
carrying the SDK version and a curated set of non-sensitive `_LogfireConfigData`
fields so the backend can drive product analytics and deprecation decisions.
Tokens, API keys, service name, environment, etc. are explicitly excluded.

Also installs a response hook on every Logfire SDK session that surfaces
`X-Logfire-Warning` headers via `warnings.warn` (deduplicated by Python's
default filter) and raises `LogfireServerError` on `X-Logfire-Error` headers,
so the server can deprecate endpoints or signal hard failures out-of-band.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

trim telemetry pairs, add service.instance.id, document rationale per field

* Limit `_LogfireConfigData`-derived pairs to `code_source_set`,
  `variables_set`, and `token_count` — the only ones with a concrete product
  question they answer. Each remaining field now has an inline comment
  explaining why we send it.
* Carry the resolved `service.instance.id` (UUID generated inside `_initialize`,
  shared with OTLP resource attributes) so the backend can correlate the
  header on non-OTLP requests with the spans this SDK instance exports.
* Drop the `None` branch of `_format_value` and the idempotency guard in
  `install_logfire_response_hook` — both were unreachable, which was tripping
  the coverage gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

move service_instance_id into _config_telemetry_pairs

It only ships when a LogfireConfig is available, so it belongs in the
config-derived group rather than the always-sent base group.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Revert "Add user agent to query client (#1875)"

This reverts commit ff5c802. With X-Logfire-Telemetry now carrying SDK
version, Python version, implementation and OS, the custom User-Agent on
the query client is duplicate plumbing for the same product analytics
question. Letting httpx send its default UA again, and the next commit
adds the telemetry header here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

address review: JSON header value, implementation field, cache base pairs, query client

- Encode the X-Logfire-Telemetry value as compact JSON instead of a
  comma-separated key=val list. Removes a silent footgun if any value
  ever contains "," or "=", and lets us send native bool/int instead of
  formatting them as strings.
- Rename the `runtime` field to `implementation` to match Python's own
  wording for `sys.implementation.name`.
- Cache `_base_telemetry_pairs()` with `functools.cache` since the value
  is constant per-process.
- Wire the same telemetry header into the experimental query client now
  that the previous custom User-Agent has been reverted.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

split out response-header handling into its own PR

Per review (#1905#discussion_r3189043209), the X-Logfire-Warning /
X-Logfire-Error response-hook handling is logically independent from
the X-Logfire-Telemetry request header — they just happened to be
introduced together. Moved that code (plus its exceptions, install
calls, and tests) to a separate PR (#1906) so each can be reviewed
and landed on its own.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants