Conversation
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>
Deploying logfire-docs with
|
| Latest commit: |
78808fa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://43ce670c.logfire-docs.pages.dev |
| Branch Preview URL: | https://add-telemetry-header.logfire-docs.pages.dev |
… 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>
|
service_instance_id belongs in the second table under "Sent when a LogfireConfig is available (_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>
|
Done |
There was a problem hiding this comment.
I think it would make sense to revert #1875 for the query client (let it use the httpx UA), and add the same telemetry header?
Regarding the format, it might be a footgun for the future to use comma-separated; what about using a JSON object encoded as a string?
| 'python_version': platform.python_version(), | ||
| # Runtime: spotting non-CPython users (pypy, graalpy) before changing | ||
| # anything that depends on CPython-specific behaviour. | ||
| 'runtime': sys.implementation.name, |
There was a problem hiding this comment.
I feel like implementation better matches the Python documentation wording:
| 'runtime': sys.implementation.name, | |
| 'implementation': sys.implementation.name, |
|
|
||
| def _base_telemetry_pairs() -> dict[str, str]: |
There was a problem hiding this comment.
I think it doesn't hurt to:
| def _base_telemetry_pairs() -> dict[str, str]: | |
| @functools.cache | |
| def _base_telemetry_pairs() -> dict[str, str]: |
No strong feelings on my end but why is json better than comma seperated / query param format? |
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>
…airs, 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>
There was a problem hiding this comment.
1 issue found across 3 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/telemetry_header.py">
<violation number="1" location="logfire/_internal/telemetry_header.py:53">
P2: `runtime` was renamed to `implementation`, which breaks the declared telemetry field contract and can silently drop backend analytics for this dimension.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
|
why not have everything in _base_telemetry_pairs in the user-agent? we already put the SDK version in there in at least some places |
| return response | ||
|
|
||
|
|
||
| def install_logfire_response_hook(session: requests.Session) -> None: |
There was a problem hiding this comment.
this seems like it should be a separate PR
Your PR body mentioned any value with a comma in it would break parsing |
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>
|
yes but you can still have commas inside of a string inside of json... |
|
@Viicos @alexmojaki I moved the response handling into #1906. I think #1906 stands on it's own and we should move forward with that (in some way). This PR I leave up to @alexmojaki @Kludex @dmontagu to decide what to do with. I agree with @Viicos that we should consider the overlap with #1875. Maybe we should do this and #1904, they're slightly orthogonal. Certainly for our Terraform client and whatnot we cannot send a span so it will have to be headers/user agent. |
Summary
Adds an
X-Logfire-Telemetry: {...}header (compact JSON) to every Logfire SDK request (OTLP exports, token validation, CRUD client, variable provider, CLI, query client). The value is deliberately small; every field has a concrete product question it answers.Linear: PYD-3579
What we send and why
Always sent (
_base_telemetry_pairs):sdk_versionsdk_languagepython_versionimplementationsys.implementation.name.osSent when a
LogfireConfigis available (_config_telemetry_pairs):code_source_setcode_source=option (newer feature) — informs whether the source-link UI is worth investing in.variables_settoken_countservice_instance_idservice.instance.idOTLP resource attribute (same UUID), letting the backend correlate header-only requests (token validation, variables fetch, CRUD) with the spans this SDK instance exports.Explicitly excluded:
token,api_key,service_name,environment,service_version,data_dir,code_source.repository|revision|root_path, and any other free-form string the user supplies. There is also nomin_level,console_enabled, etc.: nothing was added that didn't have a clear product use, and adding new fields is gated by the inline comment in_config_telemetry_pairs.The value is encoded as compact JSON (
json.dumps(..., separators=(',', ':'))) so future fields with arbitrary string values can't silently break parsing.Query client
#1875 added a custom User-Agent on the experimental query client to surface SDK version / Python version / OS to the backend. With this header in place that became duplicate plumbing, so it has been reverted as its own commit and the same
X-Logfire-Telemetryheader is now attached on query client construction instead.Test plan
tests/test_telemetry_header.pycovers header construction (with/without config), secret exclusion, OTLP export carrying the sameservice.instance.idas the resource attribute,LogfireCredentials.from_token, andLogfireClient.make format lint typecheckclean.uv run pytest tests/test_telemetry_header.py tests/test_query_client.py tests/test_configure.py tests/test_client.py tests/test_variables.py tests/test_cli.pyall pass.🤖 Generated with Claude Code