Skip to content

Fix init logger state isolation#1332

Open
stretpjc wants to merge 5 commits intomainfrom
fix-init-logger-state-isolation
Open

Fix init logger state isolation#1332
stretpjc wants to merge 5 commits intomainfrom
fix-init-logger-state-isolation

Conversation

@stretpjc
Copy link

@stretpjc stretpjc commented Feb 3, 2026

When passing state=BraintrustState() to init_logger(), the logger was not fully isolated because _compute_logger_metadata() was still using the global _state instead of the passed state parameter.

Changes:

  • Modified _compute_logger_metadata() to accept a required state parameter and use it for org_id and app_conn() instead of the global _state
  • Updated init_logger() to pass the state to _compute_logger_metadata()
  • Updated _span_components_to_object_id_lambda() to explicitly call login() and pass _state when computing project IDs for deserialized span components
  • Updated state so that if api_key or app_url exists then BraintrustState() is passed implicitly
  • Added tests

This enables multi-tenant logging where separate loggers can be created for different organizations, each with their own isolated BraintrustState.

When passing `state=BraintrustState()` to `init_logger()`, the logger was not
fully isolated because `_compute_logger_metadata()` was still using the global
`_state` instead of the passed state parameter.

Changes:
- Modified `_compute_logger_metadata()` to accept a required `state` parameter
  and use it for `org_id` and `app_conn()` instead of the global `_state`
- Updated `init_logger()` to pass the state to `_compute_logger_metadata()`
- Updated `_span_components_to_object_id_lambda()` to explicitly call `login()`
  and pass `_state` when computing project IDs for deserialized span components

This enables multi-tenant logging where separate loggers can be created for
different organizations, each with their own isolated BraintrustState.
@stretpjc stretpjc requested a review from ibolmo February 3, 2026 17:30
state = state or _state
# If no explicit state is provided but api_key or app_url is specified,
# create an isolated state to avoid conflicts with the global state
if state is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

i feel like this should go into a helper function

Copy link
Author

Choose a reason for hiding this comment

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

Added _resolve_state to address this suggestion

Add _resolve_state in to centralize logic for choosing a BraintrustState: return an explicit state if provided, create an isolated state when api_key or app_url is passed, otherwise use the global state.
@clutchski
Copy link
Collaborator

I think (a) the tests need to pass (b) we should somehow have tests verify the correct behaviour (e.g. data is sent to more than one org) vs just asserting internal detals about the logger

Tests were failing because the _compute_logger_metadata function now takes  state as the first positional argument, but the test helper  init_test_logger has a mock that didn't account for this. Updated fake_compute_logger_metadata arguments to include state in order to address this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants