From aa190ef195cfd8bcaa7b21dd8231881c2ce62f4d Mon Sep 17 00:00:00 2001 From: Starfolk Date: Fri, 17 Jul 2026 16:22:14 +0000 Subject: [PATCH 1/4] fix(crewai): add provider metadata, route tools to metadata, drop eager serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align the CrewAI integration with .agents/skills/sdk-integrations/SKILL.md. - Add metadata.provider on crewai.llm spans, read from LLM.provider on the event source. Every llm span must carry both metadata.model AND metadata.provider per the instrumentation spec. - Route tool definitions into metadata.tools (both LLMCallStartedEvent tools and AgentExecutionStartedEvent tools). Tools no longer leak into input. - Drop eager _try_to_dict / _normalize_output / _normalize_tools passes in kickoff/task/agent/llm/tool output paths. bt_json at log time already handles Pydantic v2/v1 and dataclasses, so the eager pass is wasted work. - Narrow _agent_metadata's agent_llm read: previously ``str(llm)`` was a fallback, which dumps the pydantic repr including api_key / api_base / client_params. Now the allowlist reads only the model name. - Extend the existing direct-event tests: assert metadata.provider on the kickoff/llm tree and add a positive+negative check that tools route to metadata (not input). No new cassettes — the file docstring documents why VCR is impractical for CrewAI. Co-Authored-By: Claude Opus 4.7 --- .../integrations/crewai/test_crewai.py | 39 ++++++++- .../braintrust/integrations/crewai/tracing.py | 87 +++++++++++-------- 2 files changed, 87 insertions(+), 39 deletions(-) diff --git a/py/src/braintrust/integrations/crewai/test_crewai.py b/py/src/braintrust/integrations/crewai/test_crewai.py index 86f4d4a7..fc4c6e20 100644 --- a/py/src/braintrust/integrations/crewai/test_crewai.py +++ b/py/src/braintrust/integrations/crewai/test_crewai.py @@ -250,8 +250,23 @@ def test_kickoff_llm_event_tree_parents_and_shape(memory_logger): kickoff = _build_kickoff_started() _emit(kickoff) + + # Emit with a real CrewAI LLM as source so ``metadata.provider`` gets + # populated from ``LLM.provider`` — the spec requires every llm span + # to carry provider metadata. + from crewai import LLM + from crewai.events.event_bus import crewai_event_bus + + llm_source = LLM(model="gpt-4o-mini") llm_started = _build_llm_started(parent_event_id=kickoff.event_id) - _emit(llm_started) + future = crewai_event_bus.emit(llm_source, llm_started) + if future is not None: + try: + future.result(timeout=5.0) + except Exception: + pass + _flush_event_bus(crewai_event_bus, timeout=5.0) + _emit(_build_llm_completed(llm_started, usage={"prompt_tokens": 3, "completion_tokens": 5, "total_tokens": 8})) _emit(_build_kickoff_completed(kickoff)) @@ -277,12 +292,34 @@ def test_kickoff_llm_event_tree_parents_and_shape(memory_logger): # Shape assertions. assert llm_span["input"]["messages"] == llm_started.messages assert llm_span["metadata"]["model"] == "gpt-4o-mini" + # Every llm span must carry ``metadata.provider`` per the instrumentation spec. + assert llm_span["metadata"]["provider"] == "openai" assert llm_span["metadata"]["call_id"] == "call-1" assert llm_span["output"] == "4" assert kickoff_span["output"] == "final answer" assert kickoff_span["input"] == kickoff.inputs +def test_llm_tools_route_to_metadata_not_input(memory_logger): + """Tool definitions belong in ``metadata.tools`` per the spec, not in ``input``.""" + patch_crewai() + + tools = [ + {"type": "function", "function": {"name": "search", "description": "search the web"}}, + {"type": "function", "function": {"name": "sum", "description": "add two numbers"}}, + ] + started = _build_llm_started(tools=tools) + _emit(started) + _emit(_build_llm_completed(started)) + + span = memory_logger.pop()[0] + assert span["span_attributes"]["name"] == "crewai.llm" + # Positive: tools land in metadata untouched (no eager serialization). + assert span["metadata"]["tools"] == tools + # Negative: tools MUST NOT leak into input. + assert "tools" not in span["input"], span["input"] + + def test_llm_tokens_skipped_when_litellm_patched(memory_logger, monkeypatch): """Leaf-only rule: LiteLLM patched -> no token metrics on crewai.llm.""" # Pretend LiteLLM is patched without actually wrapping the module, so diff --git a/py/src/braintrust/integrations/crewai/tracing.py b/py/src/braintrust/integrations/crewai/tracing.py index baf4b046..160a9123 100644 --- a/py/src/braintrust/integrations/crewai/tracing.py +++ b/py/src/braintrust/integrations/crewai/tracing.py @@ -45,7 +45,6 @@ from braintrust.integrations.utils import ( _normalize_chat_messages, _parse_openai_usage_metrics, - _try_to_dict, ) from braintrust.logger import NOOP_SPAN, Span, current_span from braintrust.logger import start_span as _bt_start_span @@ -127,7 +126,12 @@ def _litellm_owns_leaf_span() -> bool: def _agent_metadata(agent: Any) -> dict[str, Any]: - """Extract identity + configuration metadata from a CrewAI agent object.""" + """Extract identity + configuration metadata from a CrewAI agent object. + + Only attributes on the explicit allowlist below are read, so an SDK-level + change that adds new fields (potentially containing API keys or other + secrets) does not silently leak into span metadata. + """ if agent is None: return {} meta: dict[str, Any] = {} @@ -137,7 +141,12 @@ def _agent_metadata(agent: Any) -> dict[str, Any]: meta[f"agent_{attr}"] = str(value) if attr == "id" else value llm = getattr(agent, "llm", None) if llm is not None: - meta["agent_llm"] = getattr(llm, "model", None) or str(llm) + # Only read the model name off the LLM object. Falling back to + # ``str(llm)`` would dump the full pydantic repr, which on CrewAI's + # ``LLM`` includes ``api_key`` / ``api_base`` / ``client_params``. + model_name = getattr(llm, "model", None) + if model_name: + meta["agent_llm"] = model_name return meta @@ -198,32 +207,20 @@ def _llm_config_metadata(source: Any) -> dict[str, Any]: return meta -def _normalize_tools(tools: Any) -> Any: - """Normalize a list of CrewAI tool descriptors for logging.""" - if tools is None: - return None - if isinstance(tools, (list, tuple)): - out = [] - for tool in tools: - if isinstance(tool, dict): - out.append(tool) - else: - out.append(_try_to_dict(tool)) - return out - return _try_to_dict(tools) - - -def _normalize_output(value: Any) -> Any: - """Coerce provider-owned output objects into plain dicts/strings for logging.""" - if value is None: +def _provider_from_source(source: Any) -> str | None: + """Best-effort extraction of the underlying provider for a CrewAI LLM call. + + CrewAI's :class:`LLM` object exposes a ``provider`` string + (``"openai"``, ``"anthropic"``, ``"bedrock"``, ...). Every ``llm`` span + must carry ``metadata.provider`` per the instrumentation spec, so we + pull it directly from the emitting source when possible. + """ + if source is None: return None - if isinstance(value, (str, int, float, bool, dict, list)): - return value - coerced = _try_to_dict(value) - if coerced is value and not isinstance(value, (str, int, float, bool, dict, list)): - # Last-resort: render as a string rather than log a raw SDK object. - return str(value) - return coerced + provider = getattr(source, "provider", None) + if isinstance(provider, str) and provider: + return provider + return None # --------------------------------------------------------------------------- @@ -366,7 +363,10 @@ def on_crew_kickoff_started(source: Any, event: CrewKickoffStartedEvent) -> None @crewai_event_bus.on(CrewKickoffCompletedEvent) def on_crew_kickoff_completed(_source: Any, event: CrewKickoffCompletedEvent) -> None: - self._end_span(event, output=_normalize_output(getattr(event, "output", None))) + # Pass provider objects through untouched — ``bt_json`` at log time + # handles Pydantic models, dataclasses, and stringly fallbacks, so + # eagerly ``model_dump``-ing here would just be wasted work. + self._end_span(event, output=getattr(event, "output", None)) # Kickoff is the outermost scope; drop any orphan entries left over # when an inner end-event was never delivered so state does not grow # unbounded in long-running services. @@ -399,7 +399,7 @@ def on_task_started(source: Any, event: TaskStartedEvent) -> None: @crewai_event_bus.on(TaskCompletedEvent) def on_task_completed(_source: Any, event: TaskCompletedEvent) -> None: - self._end_span(event, output=_normalize_output(getattr(event, "output", None))) + self._end_span(event, output=getattr(event, "output", None)) @crewai_event_bus.on(TaskFailedEvent) def on_task_failed(_source: Any, event: TaskFailedEvent) -> None: @@ -418,7 +418,9 @@ def on_agent_started(_source: Any, event: AgentExecutionStartedEvent) -> None: **_task_metadata(task), **_causal_metadata(event), } - metadata["tools"] = _normalize_tools(getattr(event, "tools", None)) + agent_tools = getattr(event, "tools", None) + if agent_tools: + metadata["tools"] = agent_tools self._open_span( event, name="crewai.agent", @@ -447,18 +449,27 @@ def on_llm_started(source: Any, event: LLMCallStartedEvent) -> None: } if getattr(event, "model", None): metadata["model"] = event.model + # Every ``llm`` span must carry ``metadata.provider`` per the + # instrumentation spec. CrewAI ``LLM`` objects expose ``.provider`` + # directly (``openai`` / ``anthropic`` / ``bedrock`` / ...). + provider = _provider_from_source(source) + if provider: + metadata["provider"] = provider if getattr(event, "call_id", None): metadata["call_id"] = event.call_id available_functions = getattr(event, "available_functions", None) if available_functions: + # Only the callable *names* — never the callables themselves. metadata["available_functions"] = list(available_functions) - span_input: dict[str, Any] = { - "messages": getattr(event, "messages", None), - } + # Tool definitions belong in ``metadata.tools``, not ``input``, + # per the spec. ``bt_json`` at log time handles Pydantic models + # and dataclasses, so pass through without eager serialization. tools = getattr(event, "tools", None) if tools: - span_input["tools"] = _normalize_tools(tools) - span_input["messages"] = _normalize_chat_messages(span_input["messages"]) + metadata["tools"] = tools + span_input: dict[str, Any] = { + "messages": _normalize_chat_messages(getattr(event, "messages", None)), + } self._open_span( event, name="crewai.llm", @@ -497,7 +508,7 @@ def on_llm_completed(_source: Any, event: LLMCallCompletedEvent) -> None: self._end_span( event, - output=_normalize_output(getattr(event, "response", None)), + output=getattr(event, "response", None), metadata=metadata or None, extra_metrics=extra_metrics, ) @@ -540,7 +551,7 @@ def on_tool_finished(_source: Any, event: ToolUsageFinishedEvent) -> None: extra_metadata["run_attempts"] = run_attempts self._end_span( event, - output=_normalize_output(getattr(event, "output", None)), + output=getattr(event, "output", None), metadata=extra_metadata or None, ) From da2e6c551d94b9f5a4c5464a8dcebfc35f49d14a Mon Sep 17 00:00:00 2001 From: Starfolk Date: Fri, 17 Jul 2026 16:32:30 +0000 Subject: [PATCH 2/4] fix(crewai): never emit token metrics on crewai.llm (leaf provider owns them) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous behavior only skipped token metrics when the LiteLLM integration was patched. That missed the native-provider paths — CrewAI 1.x routes gpt-* to a native openai client, claude-* to a native anthropic client, etc. When those integrations are patched (as ``auto_instrument()`` does by default), they emit a leaf span with tokens and ``crewai.llm`` double-counted at every ancestor in the trace-tree rollup. The sdk-integrations SKILL is explicit here: "Do not add 'if OpenAI is patched, skip metrics' checks — define clear ownership instead." CrewAI is always an orchestration layer that delegates to a provider SDK, so the clear rule is: ``crewai.llm`` never owns tokens. Same pattern pydantic_ai already uses for its wrapper spans. - Drop ``_litellm_owns_leaf_span`` + ``_TOKEN_NAME_MAP`` / ``_TOKEN_PREFIX_MAP``. - ``on_llm_completed`` no longer computes ``extra_metrics``; timing and ``time_to_first_token`` remain. - Drop the now-unused ``extra_metrics`` parameter from ``_end_span`` / ``_end_span_by_event_id``. - Replace the two conditional token tests with a single ``test_llm_never_emits_token_metrics`` that asserts no token key (``tokens``, ``prompt_tokens``, ``completion_tokens``, ``prompt_cached_tokens``, ``completion_reasoning_tokens``) leaks onto ``crewai.llm`` regardless of which provider integrations are patched. Co-Authored-By: Claude Opus 4.7 --- .../integrations/crewai/test_crewai.py | 57 ++++-------- .../braintrust/integrations/crewai/tracing.py | 92 ++++--------------- 2 files changed, 39 insertions(+), 110 deletions(-) diff --git a/py/src/braintrust/integrations/crewai/test_crewai.py b/py/src/braintrust/integrations/crewai/test_crewai.py index fc4c6e20..1ccddca3 100644 --- a/py/src/braintrust/integrations/crewai/test_crewai.py +++ b/py/src/braintrust/integrations/crewai/test_crewai.py @@ -71,11 +71,6 @@ def _reset_listener(): """Force a clean CrewAI listener for each test. The module-level ``_LISTENER`` singleton leaks between tests otherwise. - We deliberately do *not* touch LiteLLM patch markers here: stripping them - would confuse ``is_patched`` and let a subsequent ``patch_litellm()`` - double-wrap the module. The leaf-only token-metric tests below instead - monkeypatch ``is_litellm_patched`` directly when they need to pretend - LiteLLM is (or is not) patched. """ _reset_for_testing() yield @@ -320,14 +315,17 @@ def test_llm_tools_route_to_metadata_not_input(memory_logger): assert "tools" not in span["input"], span["input"] -def test_llm_tokens_skipped_when_litellm_patched(memory_logger, monkeypatch): - """Leaf-only rule: LiteLLM patched -> no token metrics on crewai.llm.""" - # Pretend LiteLLM is patched without actually wrapping the module, so - # tests later in the suite that rely on a clean LiteLLM still see it. - monkeypatch.setattr( - "braintrust.integrations.crewai.tracing._litellm_owns_leaf_span", - lambda: True, - ) +def test_llm_never_emits_token_metrics(memory_logger): + """Wrapper-only rule: crewai.llm MUST NOT log token metrics. + + CrewAI always delegates the actual API call to a provider SDK + (LiteLLM, or a native openai / anthropic / bedrock / gemini + client). Whichever of those is patched owns the leaf span with + tokens. If crewai.llm also logged them, trace-tree rollup would + double-count at every ancestor. This must hold regardless of which + (if any) provider integration is patched — the "clear ownership" + pattern from the sdk-integrations SKILL. + """ patch_crewai() llm_started = _build_llm_started() @@ -345,31 +343,14 @@ def test_llm_tokens_skipped_when_litellm_patched(memory_logger, monkeypatch): metrics = by_name["crewai.llm"][0]["metrics"] assert "start" in metrics and "end" in metrics - for token_key in ("tokens", "prompt_tokens", "completion_tokens"): - assert token_key not in metrics, f"crewai.llm leaked {token_key}={metrics[token_key]} while litellm is patched" - - -def test_llm_tokens_emitted_when_litellm_not_patched(memory_logger, monkeypatch): - """Leaf: LiteLLM unpatched -> crewai.llm owns token metrics.""" - monkeypatch.setattr( - "braintrust.integrations.crewai.tracing._litellm_owns_leaf_span", - lambda: False, - ) - patch_crewai() - - llm_started = _build_llm_started() - _emit(llm_started) - _emit( - _build_llm_completed( - llm_started, - usage={"prompt_tokens": 11, "completion_tokens": 22, "total_tokens": 33}, - ) - ) - - metrics = memory_logger.pop()[0]["metrics"] - assert metrics["prompt_tokens"] == 11 - assert metrics["completion_tokens"] == 22 - assert metrics["tokens"] == 33 + for token_key in ( + "tokens", + "prompt_tokens", + "completion_tokens", + "prompt_cached_tokens", + "completion_reasoning_tokens", + ): + assert token_key not in metrics, f"crewai.llm leaked {token_key}={metrics.get(token_key)}" def test_llm_call_failed_logs_error(memory_logger): diff --git a/py/src/braintrust/integrations/crewai/tracing.py b/py/src/braintrust/integrations/crewai/tracing.py index 160a9123..12a8c69b 100644 --- a/py/src/braintrust/integrations/crewai/tracing.py +++ b/py/src/braintrust/integrations/crewai/tracing.py @@ -20,20 +20,21 @@ end events for the same scope can race across threads. A listener-level :class:`threading.Lock` guards the ``event_id -> Span`` map. -Token-metric rule (leaf-only): - -CrewAI delegates LLM calls to LiteLLM. When the LiteLLM integration is -also patched, the ``Completion`` span LiteLLM produces is the leaf span -and already owns token accounting. Emitting tokens on the enclosing -``crewai.llm`` span in that configuration would make trace-tree rollup -double-count the same tokens at every ancestor. - -We therefore follow the same pattern as the pydantic_ai integration -(see ``_wrapper_span_metrics`` in ``pydantic_ai/tracing.py``): -``crewai.llm`` emits timing + ``time_to_first_token`` unconditionally, and -token metrics only when LiteLLM is *not* patched. The check happens at -log time via the internal ``_is_litellm_patched`` helper on the LiteLLM -integration. +Token-metric rule (wrapper-only): + +CrewAI is always an orchestration layer: every LLM call it emits is +routed through an underlying provider SDK (LiteLLM, or a native +``openai`` / ``anthropic`` / ``bedrock`` / ``google-genai`` client). +Whichever of those the user has patched — commonly all of them via +``auto_instrument()`` — owns the leaf span with real token accounting. +If ``crewai.llm`` also logged tokens, trace-tree rollup would +double-count them at every ancestor. + +We therefore follow the same rule ``pydantic_ai`` uses for its wrapper +spans (see ``_wrapper_span_metrics`` in ``pydantic_ai/tracing.py``): +``crewai.llm`` emits timing + ``time_to_first_token`` only. Tokens live +on the leaf provider span. This is the "clear ownership" pattern the +sdk-integrations SKILL prefers over "if X is patched, skip" checks. """ # pylint: disable=import-error @@ -42,10 +43,7 @@ import time from typing import TYPE_CHECKING, Any -from braintrust.integrations.utils import ( - _normalize_chat_messages, - _parse_openai_usage_metrics, -) +from braintrust.integrations.utils import _normalize_chat_messages from braintrust.logger import NOOP_SPAN, Span, current_span from braintrust.logger import start_span as _bt_start_span @@ -67,24 +65,6 @@ def start_span(*args, **kwargs): from crewai.events.event_bus import CrewAIEventsBus -# LiteLLM / LiteLLM-over-OpenAI usage field translation. Mirrors the -# mapping in ``braintrust.integrations.litellm.tracing`` so ``crewai.llm`` -# spans use identical metric names when LiteLLM is not the leaf. -_TOKEN_NAME_MAP: dict[str, str] = { - "total_tokens": "tokens", - "prompt_tokens": "prompt_tokens", - "completion_tokens": "completion_tokens", - "tokens": "tokens", - "input_tokens": "prompt_tokens", - "output_tokens": "completion_tokens", -} - -_TOKEN_PREFIX_MAP: dict[str, str] = { - "input": "prompt", - "output": "completion", -} - - # Provider parameters we want to surface on `crewai.llm` metadata. _LLM_CONFIG_FIELDS: tuple[str, ...] = ( "temperature", @@ -105,21 +85,6 @@ def start_span(*args, **kwargs): ) -def _litellm_owns_leaf_span() -> bool: - """Return True when LiteLLM's completion entry points have been patched. - - Late-imported so that importing the CrewAI integration never forces a - LiteLLM import. Errors are swallowed so a broken LiteLLM install - never prevents CrewAI tracing from running. - """ - try: - from braintrust.integrations.litellm import _is_litellm_patched - - return _is_litellm_patched() - except Exception: - return False - - # --------------------------------------------------------------------------- # Payload normalization helpers # --------------------------------------------------------------------------- @@ -493,24 +458,13 @@ def on_llm_completed(_source: Any, event: LLMCallCompletedEvent) -> None: if getattr(event, "model", None): metadata["model"] = event.model - # Timing metrics are always safe. Token metrics are only emitted - # when LiteLLM is NOT patching the completion entry points, to - # avoid double-counting with the downstream ``Completion`` span. - extra_metrics: dict[str, Any] = {} - if not _litellm_owns_leaf_span(): - usage = getattr(event, "usage", None) - if usage is not None: - extra_metrics = _parse_openai_usage_metrics( - usage, - token_name_map=_TOKEN_NAME_MAP, - token_prefix_map=_TOKEN_PREFIX_MAP, - ) - + # Timing metrics only. Token metrics belong on the leaf provider + # span (openai / anthropic / bedrock / litellm / ...) so trace-tree + # rollup does not double-count. See the module docstring. self._end_span( event, output=getattr(event, "response", None), metadata=metadata or None, - extra_metrics=extra_metrics, ) @crewai_event_bus.on(LLMCallFailedEvent) @@ -659,7 +613,6 @@ def _end_span( output: Any = None, error: Any = None, metadata: dict[str, Any] | None = None, - extra_metrics: dict[str, Any] | None = None, ) -> None: """Close the span opened by the matching start event. @@ -670,9 +623,7 @@ def _end_span( started_event_id = getattr(event, "started_event_id", None) or getattr(event, "event_id", None) if not started_event_id: return - self._end_span_by_event_id( - started_event_id, output=output, error=error, metadata=metadata, extra_metrics=extra_metrics - ) + self._end_span_by_event_id(started_event_id, output=output, error=error, metadata=metadata) def _end_span_by_event_id( @@ -682,7 +633,6 @@ def _end_span_by_event_id( output: Any = None, error: Any = None, metadata: dict[str, Any] | None = None, - extra_metrics: dict[str, Any] | None = None, ) -> None: with self._lock: span = self._spans.pop(started_event_id, None) @@ -695,8 +645,6 @@ def _end_span_by_event_id( metrics: dict[str, Any] = {"end": end_time} if first_token_time is not None and start_time is not None: metrics["time_to_first_token"] = first_token_time - start_time - if extra_metrics: - metrics.update(extra_metrics) log_payload: dict[str, Any] = {"metrics": metrics} if output is not None: From a94d55c4ab0c8bf63987d9baac919cc9a5c98282 Mon Sep 17 00:00:00 2001 From: Starfolk Date: Fri, 17 Jul 2026 16:39:56 +0000 Subject: [PATCH 3/4] chore(crewai): trim redundant comments The module docstring already documents the wrapper-only token rule and the allowlist rationale; per-callsite restatements added noise. Keeps the load-bearing WHY comments (str(llm) API-key leak; ``list(dict)`` yielding names only) and drops the rest. Co-Authored-By: Claude Opus 4.7 --- .../integrations/crewai/test_crewai.py | 19 ++--------- .../braintrust/integrations/crewai/tracing.py | 34 +++---------------- 2 files changed, 8 insertions(+), 45 deletions(-) diff --git a/py/src/braintrust/integrations/crewai/test_crewai.py b/py/src/braintrust/integrations/crewai/test_crewai.py index 1ccddca3..50c798e5 100644 --- a/py/src/braintrust/integrations/crewai/test_crewai.py +++ b/py/src/braintrust/integrations/crewai/test_crewai.py @@ -246,9 +246,8 @@ def test_kickoff_llm_event_tree_parents_and_shape(memory_logger): kickoff = _build_kickoff_started() _emit(kickoff) - # Emit with a real CrewAI LLM as source so ``metadata.provider`` gets - # populated from ``LLM.provider`` — the spec requires every llm span - # to carry provider metadata. + # A real CrewAI LLM is needed as ``source`` so ``metadata.provider`` + # gets populated from ``LLM.provider``. from crewai import LLM from crewai.events.event_bus import crewai_event_bus @@ -287,7 +286,6 @@ def test_kickoff_llm_event_tree_parents_and_shape(memory_logger): # Shape assertions. assert llm_span["input"]["messages"] == llm_started.messages assert llm_span["metadata"]["model"] == "gpt-4o-mini" - # Every llm span must carry ``metadata.provider`` per the instrumentation spec. assert llm_span["metadata"]["provider"] == "openai" assert llm_span["metadata"]["call_id"] == "call-1" assert llm_span["output"] == "4" @@ -309,23 +307,12 @@ def test_llm_tools_route_to_metadata_not_input(memory_logger): span = memory_logger.pop()[0] assert span["span_attributes"]["name"] == "crewai.llm" - # Positive: tools land in metadata untouched (no eager serialization). assert span["metadata"]["tools"] == tools - # Negative: tools MUST NOT leak into input. assert "tools" not in span["input"], span["input"] def test_llm_never_emits_token_metrics(memory_logger): - """Wrapper-only rule: crewai.llm MUST NOT log token metrics. - - CrewAI always delegates the actual API call to a provider SDK - (LiteLLM, or a native openai / anthropic / bedrock / gemini - client). Whichever of those is patched owns the leaf span with - tokens. If crewai.llm also logged them, trace-tree rollup would - double-count at every ancestor. This must hold regardless of which - (if any) provider integration is patched — the "clear ownership" - pattern from the sdk-integrations SKILL. - """ + """crewai.llm is a wrapper span; the leaf provider owns tokens (see module docstring).""" patch_crewai() llm_started = _build_llm_started() diff --git a/py/src/braintrust/integrations/crewai/tracing.py b/py/src/braintrust/integrations/crewai/tracing.py index 12a8c69b..9757e5f2 100644 --- a/py/src/braintrust/integrations/crewai/tracing.py +++ b/py/src/braintrust/integrations/crewai/tracing.py @@ -91,12 +91,7 @@ def start_span(*args, **kwargs): def _agent_metadata(agent: Any) -> dict[str, Any]: - """Extract identity + configuration metadata from a CrewAI agent object. - - Only attributes on the explicit allowlist below are read, so an SDK-level - change that adds new fields (potentially containing API keys or other - secrets) does not silently leak into span metadata. - """ + """Extract identity + configuration metadata from a CrewAI agent object.""" if agent is None: return {} meta: dict[str, Any] = {} @@ -106,9 +101,8 @@ def _agent_metadata(agent: Any) -> dict[str, Any]: meta[f"agent_{attr}"] = str(value) if attr == "id" else value llm = getattr(agent, "llm", None) if llm is not None: - # Only read the model name off the LLM object. Falling back to - # ``str(llm)`` would dump the full pydantic repr, which on CrewAI's - # ``LLM`` includes ``api_key`` / ``api_base`` / ``client_params``. + # ``str(llm)`` on CrewAI's ``LLM`` dumps the full pydantic repr, + # which includes ``api_key`` / ``api_base`` / ``client_params``. model_name = getattr(llm, "model", None) if model_name: meta["agent_llm"] = model_name @@ -173,13 +167,7 @@ def _llm_config_metadata(source: Any) -> dict[str, Any]: def _provider_from_source(source: Any) -> str | None: - """Best-effort extraction of the underlying provider for a CrewAI LLM call. - - CrewAI's :class:`LLM` object exposes a ``provider`` string - (``"openai"``, ``"anthropic"``, ``"bedrock"``, ...). Every ``llm`` span - must carry ``metadata.provider`` per the instrumentation spec, so we - pull it directly from the emitting source when possible. - """ + """Return the ``provider`` string off the emitting CrewAI ``LLM`` object.""" if source is None: return None provider = getattr(source, "provider", None) @@ -328,9 +316,6 @@ def on_crew_kickoff_started(source: Any, event: CrewKickoffStartedEvent) -> None @crewai_event_bus.on(CrewKickoffCompletedEvent) def on_crew_kickoff_completed(_source: Any, event: CrewKickoffCompletedEvent) -> None: - # Pass provider objects through untouched — ``bt_json`` at log time - # handles Pydantic models, dataclasses, and stringly fallbacks, so - # eagerly ``model_dump``-ing here would just be wasted work. self._end_span(event, output=getattr(event, "output", None)) # Kickoff is the outermost scope; drop any orphan entries left over # when an inner end-event was never delivered so state does not grow @@ -414,9 +399,6 @@ def on_llm_started(source: Any, event: LLMCallStartedEvent) -> None: } if getattr(event, "model", None): metadata["model"] = event.model - # Every ``llm`` span must carry ``metadata.provider`` per the - # instrumentation spec. CrewAI ``LLM`` objects expose ``.provider`` - # directly (``openai`` / ``anthropic`` / ``bedrock`` / ...). provider = _provider_from_source(source) if provider: metadata["provider"] = provider @@ -424,11 +406,8 @@ def on_llm_started(source: Any, event: LLMCallStartedEvent) -> None: metadata["call_id"] = event.call_id available_functions = getattr(event, "available_functions", None) if available_functions: - # Only the callable *names* — never the callables themselves. + # ``list(dict)`` yields only the function names, never the callables. metadata["available_functions"] = list(available_functions) - # Tool definitions belong in ``metadata.tools``, not ``input``, - # per the spec. ``bt_json`` at log time handles Pydantic models - # and dataclasses, so pass through without eager serialization. tools = getattr(event, "tools", None) if tools: metadata["tools"] = tools @@ -458,9 +437,6 @@ def on_llm_completed(_source: Any, event: LLMCallCompletedEvent) -> None: if getattr(event, "model", None): metadata["model"] = event.model - # Timing metrics only. Token metrics belong on the leaf provider - # span (openai / anthropic / bedrock / litellm / ...) so trace-tree - # rollup does not double-count. See the module docstring. self._end_span( event, output=getattr(event, "response", None), From 5bbdacc1e391ca39d641dee9103dc727807e070f Mon Sep 17 00:00:00 2001 From: Starfolk Date: Fri, 17 Jul 2026 16:50:07 +0000 Subject: [PATCH 4/4] chore(crewai): drop remaining explanatory comments in tracing Co-Authored-By: Claude Opus 4.7 --- py/src/braintrust/integrations/crewai/tracing.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/py/src/braintrust/integrations/crewai/tracing.py b/py/src/braintrust/integrations/crewai/tracing.py index 9757e5f2..48dff638 100644 --- a/py/src/braintrust/integrations/crewai/tracing.py +++ b/py/src/braintrust/integrations/crewai/tracing.py @@ -101,8 +101,6 @@ def _agent_metadata(agent: Any) -> dict[str, Any]: meta[f"agent_{attr}"] = str(value) if attr == "id" else value llm = getattr(agent, "llm", None) if llm is not None: - # ``str(llm)`` on CrewAI's ``LLM`` dumps the full pydantic repr, - # which includes ``api_key`` / ``api_base`` / ``client_params``. model_name = getattr(llm, "model", None) if model_name: meta["agent_llm"] = model_name @@ -406,7 +404,6 @@ def on_llm_started(source: Any, event: LLMCallStartedEvent) -> None: metadata["call_id"] = event.call_id available_functions = getattr(event, "available_functions", None) if available_functions: - # ``list(dict)`` yields only the function names, never the callables. metadata["available_functions"] = list(available_functions) tools = getattr(event, "tools", None) if tools: