From 20df2ace8b263c850004a88704b31267c16f4ede Mon Sep 17 00:00:00 2001 From: hyeonsang010716 Date: Mon, 14 Sep 2026 21:40:50 +0900 Subject: [PATCH] fix(tracing): do not cache a missing OPENAI_API_KEY `BackendSpanExporter.api_key` was a `cached_property` over `self._api_key or os.environ.get("OPENAI_API_KEY")`. Resolving it before the variable existed cached `None`, so a key set afterwards, for example by `load_dotenv()`, never took effect and every export kept logging "OPENAI_API_KEY is not set, skipping trace export". `api_key` is now a property that reads the explicit key once and falls back to the environment only when it is missing. A key found in the environment is kept, so a later change to the variable does not reroute exports. A lookup that finds nothing stores nothing, so it cannot discard a key that `set_api_key()` sets on another thread meanwhile. A setter keeps `exporter.api_key = "..."` working, since public code relies on assigning the attribute. --- src/agents/tracing/processors.py | 23 ++++--- tests/tracing/test_processor_api_key.py | 79 +++++++++++++++++++++++++ tests/tracing/test_set_api_key_fix.py | 4 +- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/agents/tracing/processors.py b/src/agents/tracing/processors.py index b61f3e7976..9599835379 100644 --- a/src/agents/tracing/processors.py +++ b/src/agents/tracing/processors.py @@ -96,16 +96,25 @@ def set_api_key(self, api_key: str): api_key: The OpenAI API key to use. This is the same key used by the OpenAI Python client. """ - # Clear the cached property if it exists - if "api_key" in self.__dict__: - del self.__dict__["api_key"] - - # Update the private attribute self._api_key = api_key - @cached_property + @property def api_key(self): - return self._api_key or os.environ.get("OPENAI_API_KEY") + # Keep a key from the environment once it is found, but do not remember a missing one, so a + # key that appears after an export without one, such as from a later `load_dotenv()`, is + # still used. A lookup that finds nothing writes nothing, so it cannot discard a key that + # `set_api_key()` stores while the lookup runs. + api_key = self._api_key + if not api_key: + api_key = os.environ.get("OPENAI_API_KEY") + if api_key: + self._api_key = api_key + return api_key + + @api_key.setter + def api_key(self, api_key: str | None): + # Assigning the attribute worked while it was a cached property, and callers rely on it. + self._api_key = api_key @cached_property def organization(self): diff --git a/tests/tracing/test_processor_api_key.py b/tests/tracing/test_processor_api_key.py index 69e4c3cc5e..91fa858569 100644 --- a/tests/tracing/test_processor_api_key.py +++ b/tests/tracing/test_processor_api_key.py @@ -5,6 +5,7 @@ import pytest +from agents.tracing import processors from agents.tracing.processors import BackendSpanExporter from agents.tracing.spans import Span from agents.tracing.traces import Trace @@ -75,3 +76,81 @@ def fake_post(*, url, headers, json): assert auth_by_first_item[("a",)] == "Bearer key-a" assert auth_by_first_item[("c",)] == "Bearer key-b" assert auth_by_first_item[("b",)] == "Bearer global-key" + + +class _KeylessItem: + tracing_api_key = None + + def export(self) -> dict[str, str]: + return {"id": "item"} + + +def _record_authorization_headers(monkeypatch, exporter: BackendSpanExporter) -> list[str]: + authorization_headers: list[str] = [] + + def fake_post(*, url, headers, json): + authorization_headers.append(headers["Authorization"]) + return SimpleNamespace(status_code=200, text="ok") + + monkeypatch.setattr(exporter, "_client", SimpleNamespace(post=fake_post)) + return authorization_headers + + +def _export_keyless_item(exporter: BackendSpanExporter) -> None: + exporter.export(cast(list[Trace | Span[Any]], [_KeylessItem()])) + + +def test_exporter_uses_env_api_key_set_after_an_export_without_one(monkeypatch): + """A key missing at the first export must not disable tracing for the rest of the process.""" + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + exporter = BackendSpanExporter() + authorization_headers = _record_authorization_headers(monkeypatch, exporter) + + _export_keyless_item(exporter) + assert authorization_headers == [] + + monkeypatch.setenv("OPENAI_API_KEY", "sk-set-later") + _export_keyless_item(exporter) + + assert authorization_headers == ["Bearer sk-set-later"] + assert exporter.api_key == "sk-set-later" + + +def test_exporter_keeps_env_api_key_once_resolved(monkeypatch): + """Changing the variable later must not reroute exports that did not call `set_api_key`.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-first") + exporter = BackendSpanExporter() + authorization_headers = _record_authorization_headers(monkeypatch, exporter) + + _export_keyless_item(exporter) + monkeypatch.setenv("OPENAI_API_KEY", "sk-rotated") + _export_keyless_item(exporter) + + assert authorization_headers == ["Bearer sk-first", "Bearer sk-first"] + + +def test_exporter_uses_an_assigned_api_key(monkeypatch): + """Assigning `api_key` directly must keep working as it did with the cached property.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-env") + exporter = BackendSpanExporter() + authorization_headers = _record_authorization_headers(monkeypatch, exporter) + + exporter.api_key = "sk-assigned" + _export_keyless_item(exporter) + + assert authorization_headers == ["Bearer sk-assigned"] + + +def test_keyless_lookup_does_not_discard_a_key_set_while_it_runs(monkeypatch): + """`set_api_key` can run on another thread while the export worker looks up the variable.""" + exporter = BackendSpanExporter() + + class EnvironmentWithoutKey: + def get(self, name: str) -> str | None: + exporter.set_api_key("sk-explicit") + return None + + monkeypatch.setattr(processors, "os", SimpleNamespace(environ=EnvironmentWithoutKey())) + + assert exporter.api_key in (None, "sk-explicit") + assert exporter.api_key == "sk-explicit" diff --git a/tests/tracing/test_set_api_key_fix.py b/tests/tracing/test_set_api_key_fix.py index f8843bcb80..cfeb7f9e9e 100644 --- a/tests/tracing/test_set_api_key_fix.py +++ b/tests/tracing/test_set_api_key_fix.py @@ -17,7 +17,5 @@ def test_set_api_key_preserves_env_fallback(monkeypatch: pytest.MonkeyPatch): assert exporter.api_key == "explicit-key" # Clear explicit key and verify env fallback works - exporter._api_key = None - if "api_key" in exporter.__dict__: - del exporter.__dict__["api_key"] + exporter.set_api_key("") assert exporter.api_key == "env-key"