Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions logfire/variables/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import TYPE_CHECKING

from logfire.variables.abstract import (
ResolutionReason,
ResolvedVariable,
SyncMode,
ValidationReport,
Expand Down Expand Up @@ -71,6 +72,7 @@
# Context managers and utilities
'targeting_context',
# Types
'ResolutionReason',
'SyncMode',
'ValidationReport',
# Exceptions
Expand Down
47 changes: 31 additions & 16 deletions logfire/variables/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

__all__ = (
'ResolvedVariable',
'ResolutionReason',
'SyncMode',
'ValidationReport',
'VariableProvider',
Expand All @@ -42,6 +43,28 @@
T = TypeVar('T')
T_co = TypeVar('T_co', covariant=True)

ResolutionReason = Literal[
'resolved',
'context_override',
'missing_config',
'unrecognized_variable',
'validation_error',
'other_error',
'no_provider',
'code_default',
]
"""Why a variable (or a composed reference) resolved to its final value.

- `resolved`: provider returned a value that was used as-is.
- `context_override`: a value set via `Variable.override(...)` was used.
- `missing_config`: the variable exists on the provider but the targeting/rollout produced no value.
- `unrecognized_variable`: the provider has no entry for the variable.
- `validation_error`: the serialized value failed deserialization.
- `other_error`: composition, rendering or other error during resolution.
- `no_provider`: no provider is configured.
- `code_default`: the variable's code-default was used because the provider had no value.
"""

if not TYPE_CHECKING: # pragma: no branch
if sys.version_info < (3, 10): # pragma: no cover
_dataclass = dataclass
Expand Down Expand Up @@ -96,18 +119,10 @@ class ResolvedVariable(Generic[T_co]):
"""The name of the variable."""
value: T_co
"""The resolved value of the variable."""
_reason: Literal[
'resolved',
'context_override',
'missing_config',
'unrecognized_variable',
'validation_error',
'other_error',
'no_provider',
] # we might eventually make this public, but I didn't want to yet
"""Internal field indicating how the value was resolved."""
# Note: I had to put _reason before fields with defaults due to lack of kw_only
# Note: When we drop support for python 3.9, move _reason to the end
reason: ResolutionReason
"""How the variable was resolved (see `ResolutionReason` for possible values)."""
# Note: `reason` is declared before fields with defaults because we don't use kw_only=True
# on Python<3.10; move it to the end when 3.9 support is dropped.
label: str | None = None
"""The name of the selected label, if any."""
version: int | None = None
Expand Down Expand Up @@ -680,19 +695,19 @@ def get_serialized_value_for_label(
"""
config = self.get_variable_config(variable_name)
if config is None:
return ResolvedVariable(name=variable_name, value=None, _reason='unrecognized_variable')
return ResolvedVariable(name=variable_name, value=None, reason='unrecognized_variable')

labeled_value = config.labels.get(label)
if labeled_value is None:
return ResolvedVariable(name=variable_name, value=None, _reason='resolved')
return ResolvedVariable(name=variable_name, value=None, reason='resolved')

serialized, version = config.follow_ref(labeled_value)
return ResolvedVariable(
name=variable_name,
value=serialized,
label=label,
version=version,
_reason='resolved',
reason='resolved',
)

def refresh(self, force: bool = False):
Expand Down Expand Up @@ -1422,7 +1437,7 @@ def get_serialized_value(
Returns:
A ResolvedVariable with value=None.
"""
return ResolvedVariable(name=variable_name, value=None, _reason='no_provider')
return ResolvedVariable(name=variable_name, value=None, reason='no_provider')

def get_variable_config(self, name: str) -> VariableConfig | None:
"""Return None for all variable lookups.
Expand Down
4 changes: 2 additions & 2 deletions logfire/variables/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ def resolve_serialized_value(
"""
variable_config = self._get_variable_config(name)
if variable_config is None:
return ResolvedVariable(name=name, value=None, _reason='unrecognized_variable')
return ResolvedVariable(name=name, value=None, reason='unrecognized_variable')

serialized_value, selected_label, version = variable_config.resolve_value(
targeting_key, attributes, label=label
Expand All @@ -517,7 +517,7 @@ def resolve_serialized_value(
value=serialized_value,
label=selected_label,
version=version,
_reason='resolved',
reason='resolved',
)

def _get_variable_config(self, name: VariableName) -> VariableConfig | None:
Expand Down
2 changes: 1 addition & 1 deletion logfire/variables/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ def get_serialized_value(
self.refresh()

if self._config is None:
return ResolvedVariable(name=variable_name, value=None, _reason='missing_config')
return ResolvedVariable(name=variable_name, value=None, reason='missing_config')

return self._config.resolve_serialized_value(variable_name, targeting_key, attributes)

Expand Down
110 changes: 69 additions & 41 deletions logfire/variables/variable.py
Comment thread
alexmojaki marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from collections.abc import Generator, Mapping, Sequence
from contextlib import ExitStack, contextmanager
from contextvars import ContextVar
from dataclasses import dataclass, field, replace
from dataclasses import dataclass, field
from importlib.util import find_spec
from typing import TYPE_CHECKING, Any, Generic, Protocol, TypeVar

Expand All @@ -27,6 +27,7 @@
__all__ = (
'ResolveFunction',
'is_resolve_function',
'_BaseVariable',
'Variable',
'targeting_context',
)
Expand All @@ -37,6 +38,15 @@
_VARIABLE_OVERRIDES: ContextVar[dict[str, Any] | None] = ContextVar('_VARIABLE_OVERRIDES', default=None)


def _record_exception(exception: BaseException, span: logfire.LogfireSpan) -> None:
"""Record an exception on a span, ignoring a CPython traceback extraction bug."""
try:
span.record_exception(exception)
except RuntimeError as exc:
if 'generator raised StopIteration' not in str(exc):
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
raise


@dataclass
class _TargetingContextData:
"""Internal data structure for targeting context."""
Expand Down Expand Up @@ -115,8 +125,8 @@ def is_resolve_function(f: Any) -> TypeIs[ResolveFunction[Any]]:
return required_positional <= 2 and total_positional >= 2


class Variable(Generic[T_co]):
"""A managed variable that can be resolved dynamically based on configuration."""
class _BaseVariable(Generic[T_co]):
"""Base class for managed variables with shared resolution infrastructure."""

name: str
"""Unique name identifying this variable."""
Expand Down Expand Up @@ -187,28 +197,12 @@ def refresh_sync(self, force: bool = False):
"""Synchronously refresh the variable."""
self.logfire_instance.config.get_variable_provider().refresh(force=force)

def get(
def _get_result_and_record_span(
self,
targeting_key: str | None = None,
attributes: Mapping[str, Any] | None = None,
*,
label: str | None = None,
) -> ResolvedVariable[T_co]:
"""Resolve the variable and return full details including label, version, and any errors.

Args:
targeting_key: Optional key for deterministic label selection (e.g., user ID).
If not provided, falls back to contextvar targeting key (set via targeting_context),
then to the current trace ID if there is an active trace.
attributes: Optional attributes for condition-based targeting rules.
label: Optional explicit label name to select. If provided, bypasses rollout
weights and targeting, directly selecting the specified label. If the label
doesn't exist in the configuration, falls back to default resolution.

Returns:
A ResolvedVariable object containing the resolved value, selected label,
version, and any errors that occurred.
"""
merged_attributes = self._get_merged_attributes(attributes)

# Targeting key resolution: call-site > contextvar > trace_id
Expand Down Expand Up @@ -248,13 +242,11 @@ def get(
'value': serialized_value,
'label': result.label,
'version': result.version,
'reason': result._reason, # pyright: ignore[reportPrivateUsage]
'reason': result.reason,
}
)
if result.exception:
span.record_exception(
result.exception,
)
_record_exception(result.exception, span)
return result

def _resolve(
Expand All @@ -270,7 +262,7 @@ def _resolve(
context_value = context_overrides[self.name]
if is_resolve_function(context_value):
context_value = context_value(targeting_key, attributes)
return ResolvedVariable(name=self.name, value=context_value, _reason='context_override')
return ResolvedVariable(name=self.name, value=context_value, reason='context_override')

provider = self.logfire_instance.config.get_variable_provider()

Expand All @@ -286,21 +278,35 @@ def _resolve(
span.set_attribute('invalid_serialized_value', serialized_result.value)
default = self._get_default(targeting_key, attributes)
reason: str = 'validation_error' if isinstance(value_or_exc, ValidationError) else 'other_error'
return ResolvedVariable(name=self.name, value=default, exception=value_or_exc, _reason=reason)
return ResolvedVariable(
name=self.name,
value=default,
exception=value_or_exc,
reason=reason,
label=serialized_result.label,
version=serialized_result.version,
)
return ResolvedVariable(
name=self.name,
value=value_or_exc,
label=serialized_result.label,
version=serialized_result.version,
_reason='resolved',
reason='resolved',
)
# Label not found - fall through to default resolution

serialized_result = provider.get_serialized_value(self.name, targeting_key, attributes)

if serialized_result.value is None:
default = self._get_default(targeting_key, attributes)
return _with_value(serialized_result, default)
# Provider had no value; surface that the code default was used.
Comment on lines +246 to 247
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot May 21, 2026

Choose a reason for hiding this comment

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

P2: Provider exception from serialized_result.exception is lost when _resolve_serialized_default returns a non-None result (i.e., when render_fn is provided). The _resolve_serialized_default method creates a fresh ResolvedVariable without propagating the provider's exception. This means if a provider returns value=None with a meaningful exception (e.g., reason='unrecognized_variable'), that diagnostic information is silently dropped in the render_fn code path, whereas the non-render_fn path at lines 248-255 correctly preserves it via exception=serialized_result.exception.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At logfire/variables/variable.py, line 246:

<comment>Provider exception from `serialized_result.exception` is lost when `_resolve_serialized_default` returns a non-None result (i.e., when `render_fn` is provided). The `_resolve_serialized_default` method creates a fresh `ResolvedVariable` without propagating the provider's exception. This means if a provider returns `value=None` with a meaningful exception (e.g., `reason='unrecognized_variable'`), that diagnostic information is silently dropped in the render_fn code path, whereas the non-render_fn path at lines 248-255 correctly preserves it via `exception=serialized_result.exception`.</comment>

<file context>
@@ -278,34 +227,23 @@ def _resolve(
+                    render_fn=render_fn,
+                )
+                if default_result is not None:
+                    return default_result
                 # Provider had no value; surface that the code default was used.
                 return ResolvedVariable(
</file context>
Suggested change
return default_result
# Provider had no value; surface that the code default was used.
if default_result is not None:
default_result.exception = default_result.exception or serialized_result.exception
return default_result

✅ Addressed in 1147a16

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(AI) Addressed in stacked PR #1949.

return ResolvedVariable(
name=self.name,
value=self._get_default(targeting_key, attributes),
exception=serialized_result.exception,
label=serialized_result.label,
version=serialized_result.version,
reason='code_default',
)

# Deserialize - returns T | Exception
value_or_exc = self._deserialize(serialized_result.value)
Expand All @@ -310,22 +316,29 @@ def _resolve(
span.set_attribute('invalid_serialized_value', serialized_result.value)
default = self._get_default(targeting_key, attributes)
reason: str = 'validation_error' if isinstance(value_or_exc, ValidationError) else 'other_error'
return ResolvedVariable(name=self.name, value=default, exception=value_or_exc, _reason=reason)
return ResolvedVariable(
name=self.name,
value=default,
exception=value_or_exc,
reason=reason,
label=serialized_result.label,
version=serialized_result.version,
)

return ResolvedVariable(
name=self.name,
value=value_or_exc,
label=serialized_result.label,
version=serialized_result.version,
_reason='resolved',
reason='resolved',
)

except Exception as e:
if span and serialized_result is not None: # pragma: no cover
span.set_attribute('invalid_serialized_label', serialized_result.label)
span.set_attribute('invalid_serialized_value', serialized_result.value)
default = self._get_default(targeting_key, attributes)
return ResolvedVariable(name=self.name, value=default, exception=e, _reason='other_error')
return ResolvedVariable(name=self.name, value=default, exception=e, reason='other_error')

def _get_default(
self, targeting_key: str | None = None, merged_attributes: Mapping[str, Any] | None = None
Expand Down Expand Up @@ -385,17 +398,32 @@ def to_config(self) -> VariableConfig:
)


def _with_value(details: ResolvedVariable[Any], new_value: T_co) -> ResolvedVariable[T_co]:
"""Return a copy of the provided resolution details, just with a different value.
class Variable(_BaseVariable[T_co]):
"""A managed variable that can be resolved dynamically based on configuration."""

Args:
details: Existing resolution details to modify.
new_value: The new value to use.
def get(
self,
targeting_key: str | None = None,
attributes: Mapping[str, Any] | None = None,
*,
label: str | None = None,
) -> ResolvedVariable[T_co]:
"""Resolve the variable and return full details including label, version, and any errors.

Returns:
A new ResolvedVariable with the given value.
"""
return replace(details, value=new_value)
Args:
targeting_key: Optional key for deterministic label selection (e.g., user ID).
If not provided, falls back to contextvar targeting key (set via targeting_context),
then to the current trace ID if there is an active trace.
attributes: Optional attributes for condition-based targeting rules.
label: Optional explicit label name to select. If provided, bypasses rollout
weights and targeting, directly selecting the specified label. If the label
doesn't exist in the configuration, falls back to default resolution.

Returns:
A ResolvedVariable object containing the resolved value, selected label,
version, and any errors that occurred.
"""
return self._get_result_and_record_span(targeting_key, attributes, label)


@contextmanager
Expand Down
Loading
Loading