Send spec-compliant User-Agent: logfire-python/<version>, retaining the OTLP exporter identifier on exports#2106
Send spec-compliant User-Agent: logfire-python/<version>, retaining the OTLP exporter identifier on exports#2106dmontagu wants to merge 5 commits into
logfire-python/<version>, retaining the OTLP exporter identifier on exports#2106Conversation
…porter identifier on OTLP Claude-Session: https://claude.ai/code/session_0159PYpdaBhM35pcPFiVcotC
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStandardizes the Logfire HTTP User-Agent as 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
|
|
||
| _ACCEPT = Literal['application/json', 'application/vnd.apache.arrow.stream', 'text/csv'] | ||
| _USER_AGENT = f'logfire-sdk-python/{VERSION} (Python {platform.python_version()}, os {platform.platform()}, arch {platform.machine()})' |
There was a problem hiding this comment.
It's not super clear what the harm of keeping the other info in the UA is. I guess it makes parsing and querying harder? But if we're going to remove info then it seems important to merge some version of #1905 first so that we don't lose it entirely.
There was a problem hiding this comment.
I'd say it's fine to leave the telemetry in this header, and we rename logfire-sdk-python/... to logfire-python/.... I initially had concerns with parsing/querying because I have a dashboard where I keep track of SDK queries by UA, but if we properly follow the RFC 9110 grammar and have the correct UDF to do to the parsing then it should be fine.
cc @jirikuncar
There was a problem hiding this comment.
I just want to follow the otel spec here and not rely on this too much for extra info. I feel like we don't depend on the python/os/arch stuff so much that we can't afford a gap in awareness of this.
There was a problem hiding this comment.
Are we currently using the info in this UA? Is there pressure to change this UA soon?
| headers = Headers(client_kwargs.pop('headers', None)) | ||
| headers['authorization'] = read_token | ||
| headers.setdefault('user-agent', _USER_AGENT) | ||
| headers.setdefault('user-agent', UA_HEADER) |
There was a problem hiding this comment.
why do we even support custom headers? and should our UA be appended to a custom UA?
There was a problem hiding this comment.
Unfortunately removing the option to set it would be breaking in some way, but I agree it's questionable. I'd also add the httpx one as a component after ours
There was a problem hiding this comment.
Agreed with Victorien that removing custom-header support would be breaking, so I've kept it, including the existing semantics where a caller-supplied UA fully replaces ours (it doubles as an opt-out). On appending python-httpx/<v> after ours: I'd rather not. The spec guidance we're following only asks that the exporter's identifier be retained; httpx is just the transport, and consistency would then also suggest python-requests/<v> on the API client and CLI, re-growing the UA right after this PR trimmed it to one low-cardinality token per surface. If we want richer client-runtime detail later, I think #1905's telemetry header is the right home for it.
There was a problem hiding this comment.
The spec is irrelevant here, this isn't the exporter.
httpx/requests does naturally seem like it fits in the UA. I think it fits there better than in the telemetry header. And it's not currently there anyway.
3 participants
Summary
Aligns the SDK's
User-Agenthandling with the OpenTelemetry exporter specification and with the other Logfire SDKs, and unifies it across every HTTP surface:logfire/<v>(upstream exporter identifier dropped)logfire-python/<v> OTel-OTLP-Exporter-Python/<exporter-v>logfire/<v>logfire-python/<v>logfire-sdk-python/<v> (Python ..., os ..., arch ...)logfire-python/<v>python-requests/<v>)logfire-python/<v>Why
The OTel exporter spec's User Agent section recommends that when a distribution adds its identifier to the
User-Agent, the exporter's default identifier is retained, typically by prepending:That guidance was added in open-telemetry/opentelemetry-specification#4560 (Sept 2025, motivated by #4479). Our current behavior predates it: the
logfire/<version>header dates to #250 (Sept 2023) and, once reused for the OTLP exporters, silently replaced the upstreamOTel-OTLP-Exporter-Python/<v>identifier because the Python exporter applies custom headers over its defaults. Since the Python exporter does not (yet) implement the spec'suser-agentenrichment option, we rebuild its default identifier from the installed package version and prepend ours; a test compares against a freshly constructed exporter's actual default so we notice if the upstream format ever changes.Retaining the exporter identifier also has direct debugging value: it tells the backend which upstream exporter version produced a request without having to ask the user.
Choices and their grounding
logfire-python/<v>: matches the sibling SDKs (logfire-js/<v>,logfire-rust/<v>) and the spec'sMyDistribution/x.y.zproduct-token form (RFC 9110).process.runtime.*,os.*,host.*, see Pre-populatehost.*andos.*resource attributes #2013), and a single low-cardinality product token keeps the UA usable for filtering and aggregating by SDK version.UA_HEADER, with OTLP exports additionally retaining the upstream identifier viaOTLP_EXPORTER_UA_HEADER.This complements (and does not replace) the request-telemetry-header discussion in #1905; richer runtime/config signals belong there, not in the UA.
Compatibility notes
logfire/<version>(e.g. proxy rules) should match thelogfire-python/prefix instead. Older SDK versions keep sending the old formats, so consumers should expect both during the transition.user-agentheaders on the query client are still respected (setdefault).Test plan
tests/test_user_agent.py: product-token format; OTLP UA exactly equals<our token> <upstream exporter default>(drift guard against the installed exporter's real default); end-to-endlogfire.configure()asserting the UA per endpoint (/v1/infovs/v1/traces) viarequests_mock; API client, query client, and CLI request headers.make format lint typecheckclean;tests/test_configure.py,tests/test_cli.py,tests/test_query_client.py,tests/exporters/test_otlp_session.py,tests/test_variables.pyall pass (625 tests).https://claude.ai/code/session_0159PYpdaBhM35pcPFiVcotC