Add managed variables #1691
Add managed variables #1691
Conversation
Add comprehensive tests covering uncovered lines across the variables module: is_resolve_function edge cases, variant deserialization errors, remote provider change detection, force refresh, variant diffing, variable type operations, push_variable_types, and more. Add pragmas for genuinely untestable code paths (fork handler, unreachable fallback).
Covers: resolve function default without type, duplicate variable name, on_config_change with unknown variable, and multiple keyword-only params.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add a `timeout` field to `RemoteVariablesConfig` (default `(10, 10)`) so that both the polling thread and the blocking first-resolve path use a bounded `Session.get` call. Also propagate `timeout_millis` through `VariableProvider.shutdown()` so thread joins respect the shutdown budget.
Deploying logfire-docs with
|
| Latest commit: |
410da24
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://98a177bf.logfire-docs.pages.dev |
| Branch Preview URL: | https://managed-variables.logfire-docs.pages.dev |
…sion optional - Remove `enabled` field from VariableConfig (disabled variables are now modeled by pointing labels to code_default) - Add `code_default` handling in `follow_ref` - returns (None, None) to trigger code default fallthrough - Make `LabelRef.version` optional (None for code_default and label-to-label refs) - Update `_validate_labels` to skip code_default refs in label validation - Update tests to use code_default pattern instead of enabled=False
- Remove `enabled` field from VariableConfig reference table - Document `code_default` as a LabelRef target - Note that LabelRef.version is optional - Add explanations for when to use each ref target type
|
the docs page is by far the biggest, please can it be split up into multiple pages? |
There was a problem hiding this comment.
Pull request overview
Adds a “managed variables” feature to Logfire, providing typed runtime-config variables backed by local config or remote Logfire API with polling/SSE updates, plus supporting CLI-friendly sync/validation workflows.
Changes:
- Introduces
logfire.variablesmodule with config models, providers (local/remote), and variable resolution utilities (targeting, overrides, on-change callbacks). - Adds public
logfire.var()+variables_*helper APIs and wires provider lifecycle intoconfigure()/shutdown(). - Adds extensive tests and documentation for managed variables and provider sync tooling.
Reviewed changes
Copilot reviewed 22 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/type_checking.py | Adds typing regression/behavior documentation for var() defaults. |
| tests/test_push_variables.py | Adds unit tests for schema generation, diffing, push/validate formatting, and registration. |
| tests/test_logfire_api.py | Ensures managed-variables symbols are present in the public API surface. |
| tests/test_configure.py | Extends config serialization test to include variables + remote provider mocking. |
| tests/otel_integrations/test_requests.py | Makes request metric assertions tolerant of numeric totals. |
| tests/otel_integrations/test_openai.py | Fixes IsNumeric usage to matcher instance. |
| tests/conftest.py | Clears variable registry between tests. |
| pyproject.toml | Adds variables extra dependency on Pydantic. |
| mkdocs.yml | Adds Managed Variables doc page to nav. |
| logfire/variables/variable.py | Implements Variable, targeting context, overrides, resolution, and on-change callbacks. |
| logfire/variables/remote.py | Implements remote provider with polling, SSE listener, and CRUD APIs. |
| logfire/variables/local.py | Implements in-memory local provider with CRUD + change notification. |
| logfire/variables/config.py | Adds Pydantic config models for labels/rollouts/conditions and resolution logic. |
| logfire/variables/abstract.py | Defines provider ABC, resolved details, diff/push/validate utilities, and variable-type push. |
| logfire/variables/init.py | Exposes managed variables public surface via lazy imports. |
| logfire/_internal/main.py | Adds variable registry + public variable APIs and integrates provider shutdown. |
| logfire/_internal/config_params.py | Adds LOGFIRE_API_KEY config param for public API calls (variables). |
| logfire/_internal/config.py | Adds variables options/config, provider creation, and provider start lifecycle integration. |
| logfire/_internal/client.py | Adds internal PUT helper (currently unused in shown diff). |
| logfire/init.py | Exposes var + variables_* helpers at top-level and adds VariablesOptions. |
| logfire-api/logfire_api/init.py | Updates logfire-api stubs to include managed variables APIs. |
| docs/reference/advanced/managed-variables.md | Adds full managed variables guide and reference documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extract on_change callback functionality into a separate PR to keep the managed-variables branch focused on the core feature.
- Fix lost-wakeup race in remote provider worker by using wait(timeout) then clear() pattern instead of clear() before wait() - Pass timeout budget to variable provider shutdown to prevent exceeding total shutdown timeout - Validate variable names at registration time against VariableName pattern to fail early with a clear error - Fix deserialization cache race by re-checking key presence under lock before inserting to prevent duplicate _cache_order entries - Fix attribute merge order so user-provided attributes have highest priority over baggage and resource attributes - Fix docs referencing non-existent latest_weight field on Rollout model
Update test to use underscore-separated name (`my_variable_2`) instead of hyphenated name (`my-variable-2`) to match the new variable name validation added in the previous commit.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| _cache: dict[str, T_co] = {} | ||
| _cache_order: list[str] = [] | ||
| _cache_maxsize = 128 | ||
| _cache_lock = threading.Lock() | ||
|
|
||
| def _deserialize_cached(serialized_value: str) -> T_co | Exception: | ||
| with _cache_lock: | ||
| if serialized_value in _cache: | ||
| return _cache[serialized_value] | ||
| try: | ||
| result = self.type_adapter.validate_json(serialized_value) | ||
| except Exception as e: | ||
| return e | ||
| with _cache_lock: | ||
| # Re-check after acquiring lock to avoid duplicate entries from concurrent misses | ||
| if serialized_value not in _cache: | ||
| if len(_cache) >= _cache_maxsize: | ||
| oldest = _cache_order.pop(0) | ||
| _cache.pop(oldest, None) | ||
| _cache[serialized_value] = result | ||
| _cache_order.append(serialized_value) | ||
| return result | ||
|
|
||
| self._deserialize_cached = _deserialize_cached |
There was a problem hiding this comment.
🚩 Deserialization cache returns shared mutable instances
The deserialization cache in _deserialize_cached (logfire/variables/variable.py:168-184) caches the result of self.type_adapter.validate_json(serialized_value) and returns the same instance for repeated lookups of the same serialized value. For mutable types like Pydantic models, this means all callers share the same object. If any caller mutates the returned value (e.g., config.value.temperature = 1.0), it silently corrupts the cache for all future callers.
This is a known tradeoff in caching (similar to functools.lru_cache), and the documentation examples only show reading properties, not mutating them. However, it's worth noting that Pydantic models are not frozen by default, so mutation is easy to do accidentally. Consider documenting this expectation or using model_copy() on cache hits for model types.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Added a comment documenting that cached results are shared instances (like functools.lru_cache) and callers should not mutate returned values. Adding model_copy() on every cache hit would negate the performance benefit of caching, so documenting the expectation is the right tradeoff here.
There was a problem hiding this comment.
This isn't documented anywhere for users, it's just a comment. And this seems like a significant problem. Is caching really needed here?
- Add test for invalid variable name validation in main.py - Add pragma: no branch for race-condition guard in variable cache - Add pragma: no branch for worker awaken timeout branch
- Add polling_interval validation (minimum 10s) since polling is only a fallback for SSE, and increase default from 30s to 60s - Pass timeout_millis=200 to variable provider shutdown during reconfiguration to avoid holding the lock too long - Document that deserialization cache returns shared mutable instances
…ariables from logfire-api - Add local import of VariablesConfig inside runtime branch in config.py - Remove all variables-related APIs from logfire-api (var, variables_*, VariablesOptions, _VariablesModule) — variables users should use the full SDK - Update test_logfire_api.py to skip variables names in __all__ check
| @model_validator(mode='after') | ||
| def _validate_labels(self): | ||
| # Validate rollout label references | ||
| for k in self.rollout.labels: | ||
| if k not in self.labels: | ||
| raise ValueError(f'Label {k!r} present in `rollout.labels` is not present in `labels`.') | ||
|
|
||
| # Validate rollout override label references | ||
| for i, override in enumerate(self.overrides): # pragma: no branch | ||
| for k in override.rollout.labels: # pragma: no branch | ||
| if k not in self.labels: # pragma: no branch | ||
| raise ValueError(f'Label {k!r} present in `overrides[{i}].rollout` is not present in `labels`.') | ||
|
|
||
| # Validate ref chains don't reference non-existent labels | ||
| for k, labeled_value in self.labels.items(): | ||
| if isinstance(labeled_value, LabelRef) and labeled_value.ref not in ('latest', 'code_default'): | ||
| if labeled_value.ref not in self.labels: | ||
| raise ValueError(f'Label {k!r} has ref {labeled_value.ref!r} which is not present in `labels`.') | ||
|
|
||
| return self |
There was a problem hiding this comment.
🚩 Documentation claims 'latest' can be used as rollout label key, but validator rejects it
The documentation at docs/reference/advanced/managed-variables.md:211 states:
You can explicitly assign a percentage to latest alongside labels
But VariableConfig._validate_labels at logfire/variables/config.py:316-318 validates that ALL keys in rollout.labels must exist in self.labels (the label definitions dict). Since 'latest' is a special ref value (not a label definition), putting it in rollout labels would fail validation.
The system handles the "latest" concept differently: when rollout weights sum to less than 1.0, the remainder probability implicitly goes to None (which maps to latest_version or code_default). So the feature described in the docs may be partially supported via the implicit remainder, but the explicit 'latest' key in rollout labels would not work as documented.
Was this helpful? React with 👍 or 👎 to provide feedback.
Split the old `VariablesOptions(config=...)` into two distinct types: - `VariablesOptions` (formerly `RemoteVariablesConfig`): flat dataclass for remote API configuration with fields like `block_before_first_resolve`, `polling_interval`, `api_key`, `timeout`, `instrument`, etc. - `LocalVariablesOptions`: for local in-memory variable configs (dev/testing) - `VariableProvider` can also be passed directly to `variables=` The `configure(variables=...)` parameter now accepts `VariablesOptions | LocalVariablesOptions | VariableProvider | None`. Add lazy variable provider initialization: when `variables=` is not passed to `configure()` but `LOGFIRE_API_KEY` exists in the environment, the first call to `get_variable_provider()` lazily creates a `LogfireRemoteVariableProvider` with default options. Uses double-checked locking with the existing RLock for thread safety. Also export `LocalVariablesOptions` from `logfire.__init__` and update docs with an "Automatic Remote Variables" tip explaining the lazy init behavior.
| To roll out v3 to everyone, just move the `production` label from v2 to v3. To roll back, move it back to v2. No new versions need to be created — the label is just a pointer. | ||
|
|
||
| !!! tip "Latest version" | ||
| If no labels are configured in the rollout, or if rollout weights sum to less than 1.0, the remaining traffic automatically receives the **latest version** (the most recently created version). This is the simplest setup: just keep creating new versions and all traffic gets the latest one. |
There was a problem hiding this comment.
The UI shows the remaining traffic going to the code default, not the latest version
| ``` | ||
|
|
||
| !!! tip "Automatic Remote Variables" | ||
| If `LOGFIRE_API_KEY` is set in your environment, variable APIs will **automatically** use the remote provider without needing `variables=VariablesOptions()` in `configure()`. The first time a variable is resolved, the SDK detects the API key and lazily initializes the remote provider with default options. You only need to pass `variables=VariablesOptions(...)` explicitly if you want to customize options like `polling_interval` or `block_before_first_resolve`. |
There was a problem hiding this comment.
This spits out errors if the LOGFIRE_API_KEY doesn't have permission
|
|
||
| ```python skip="true" | ||
| logfire.variables_push_types([ | ||
| (FeatureConfig, 'my-feature-config'), |
There was a problem hiding this comment.
names should have to be valid identifiers
2 participants
Summary
Adds a managed variables feature to Logfire, enabling dynamic configuration management with feature flags, A/B testing, and targeted rollouts. Variables can be resolved from local in-memory configuration or from the remote Logfire API with background polling and SSE-based real-time updates.
This is a replacement of #1548 to reduce review noise.
Key additions
Unresolved review comments from #1548 to revisit
These are comments that were noted but not actioned yet in this PR:
doesn't seem to exist. Docs may need a cleanup pass.