Propagate variable version in baggage alongside label#1927
Merged
Conversation
`ResolvedVariable.__enter__` now sets a second baggage entry `logfire.variables.<name>.version` (string-formatted integer) whenever the resolved variable has a version, in addition to the existing `logfire.variables.<name>` entry that carries the label. Why: downstream spans inside the resolution context now carry both attributes, so observability queries that compare variants of an A/B test can `GROUP BY (label, version)` directly. Previously the version was only on the `Resolve variable <name>` span itself, and that span isn't in the same trace as user-created spans (it opens and closes inside `_get_result_and_record_span` before the context manager body runs), so there was no SQL-only way to associate a downstream span with the version that served it. Code-default resolutions have `version=None` and don't emit the new baggage entry — only versioned resolutions do. Two test additions in `test_variables.py`: - The existing context-manager-sets-label test now also asserts the version baggage entry is present and matches the resolved version. - A new test asserts that code-default resolutions don't emit a version baggage entry (no key, not just an empty value).
Deploying logfire-docs with
|
| Latest commit: |
4578860
|
| Status: | ⚡️ Build in progress... |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
alexmojaki
reviewed
May 14, 2026
alexmojaki
reviewed
May 14, 2026
alexmojaki
reviewed
May 14, 2026
- Drop the redundant logfire.configure(**config_kwargs) call; the
autouse `config` fixture in conftest.py already configures the
default logfire instance and clears variables between tests.
- Replace the two separate `assert ... in/== baggage` checks with a
single `assert baggage == snapshot({...})`, matching the project's
testing convention.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ResolvedVariable.__enter__now sets two baggage entries instead of one:logfire.variables.<name>— the resolved label (unchanged).logfire.variables.<name>.version— the resolved version, string-formatted (new).The version entry is only set when a version actually resolved; code-default resolutions (where
version is None) emit just the label entry, same as before.Why
Today, downstream spans inside a variable's resolution context carry the label via baggage but not the version. The version is available on the SDK-emitted
Resolve variable <name>span as an attribute, but that span is opened and closed entirely inside_get_result_and_record_span— before the context manager body runs — so it does not share atrace_idwith any user-created spans. There is no SQL-only path to attach the version to a downstream span.That gap is hit hardest by A/B-test analytics. The marquee SQL query for
src/walkthroughs/managed-variables/ab-testing-rollout/(in the platform repo) aggregates request metrics bylogfire.variables.ranking_config(the label). After a label gets moved to a new version mid-experiment — the canonical "promote canary v2 → v3" flow — aggregating by label alone mixes data from the old and new versions, which is precisely the failure mode an A/B harness is supposed to surface. The walkthrough today works around it by settingspan.set_attribute('config_version', resolved.version)in application code. With this change, that workaround becomes unnecessary:GROUP BY label, attributes->>'logfire.variables.ranking_config.version'works directly.Backwards compatibility
Strictly additive — existing SQL queries that filter or group on
logfire.variables.<name>(the label entry) continue to work unchanged. New queries can pull the version off the new key when they need it.The new baggage value is a stringified integer (e.g.
'3'); OTel baggage values are strings.Test plan
test_context_manager_sets_label_in_baggagetest that the version entry is present and matches the resolved version ('1'in that fixture).test_context_manager_omits_version_for_code_defaultconfirming that resolutions with no remote config (soversion is None) emit only the label entry, not a version entry.Follow-up in platform repo
Once this lands and a release goes out (and the platform's pinned
logfirerevision bumps), the walkthrough's02_send_queries.pyworkaround can be removed and the SQL query simplifies. That cleanup is referenced from the platform PR that bundles the rename work and the walkthrough's interim workaround.