diff --git a/packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py b/packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py index 15ed817e15..d6068b8062 100644 --- a/packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py +++ b/packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py @@ -106,16 +106,38 @@ def _extract_class_name_from_serialized(serialized: Optional[dict[str, Any]]) -> return "" +_METADATA_PRIMITIVES = (bool, str, bytes, int, float) + + def _sanitize_metadata_value(value: Any) -> Any: - """Convert metadata values to OpenTelemetry-compatible types.""" + """Convert metadata values to OpenTelemetry-compatible types. + + Only plain data is forwarded. An arbitrary object is dropped rather than + stringified: ``str()`` on a model, client or config object renders its + constructor state, which routinely includes credentials the caller never + meant to export. Association properties are also copied onto every + descendant span, so a single such value spreads across the whole trace. + + Returns ``None`` for anything that is not plain data; callers drop those + keys rather than recording a placeholder. + """ if value is None: return None - if isinstance(value, (bool, str, bytes, int, float)): + if isinstance(value, _METADATA_PRIMITIVES): return value if isinstance(value, (list, tuple)): - return [str(_sanitize_metadata_value(v)) for v in value] - # Convert other types to strings - return str(value) + # Keep primitive elements, drop object elements, preserving the + # existing "sequence of strings" attribute shape. + return [str(v) for v in value if isinstance(v, _METADATA_PRIMITIVES)] + if isinstance(value, dict): + # A mapping of plain data is legitimate metadata, so keep it as JSON. + # json.dumps has no ``default``, so a mapping holding a non-serializable + # object raises and the key is dropped instead of being stringified. + try: + return json.dumps(value) + except (TypeError, ValueError): + return None + return None def valid_role(role: str) -> bool: @@ -165,6 +187,8 @@ def _extract_tool_call_data( class TraceloopCallbackHandler(BaseCallbackHandler): + """LangChain callback handler that records chain, tool and LLM runs as spans.""" + def __init__( self, tracer: Tracer, duration_histogram: Histogram, token_histogram: Histogram ) -> None: @@ -296,11 +320,14 @@ def _create_span( current_association_properties = ( context_api.get_value("association_properties") or {} ) - # Sanitize metadata values to ensure they're compatible with OpenTelemetry + # Sanitize metadata values to ensure they're compatible with + # OpenTelemetry. Values that are not plain data sanitize to None + # and are dropped. sanitized_metadata = { - k: _sanitize_metadata_value(v) + k: sanitized for k, v in metadata.items() if v is not None + and (sanitized := _sanitize_metadata_value(v)) is not None } try: association_properties_token = context_api.attach( diff --git a/packages/opentelemetry-instrumentation-langchain/tests/test_metadata_sanitization.py b/packages/opentelemetry-instrumentation-langchain/tests/test_metadata_sanitization.py new file mode 100644 index 0000000000..43220e5f8b --- /dev/null +++ b/packages/opentelemetry-instrumentation-langchain/tests/test_metadata_sanitization.py @@ -0,0 +1,118 @@ +"""Metadata values must be plain data, never a stringified object. + +Association properties are set from the caller's ``config={"metadata": ...}`` and +are copied onto every descendant span. Before this test, any non-primitive value +was passed through ``str()``, so an object's repr landed on the whole trace. A +model, client or config object renders its constructor state, which routinely +includes an API key, so one such value exported a credential to the trace +backend. Metadata is also not gated by TRACELOOP_TRACE_CONTENT, so turning +content capture off did not suppress it. + +These are unit tests over the sanitizer: they need no network and no cassette. +""" + +from opentelemetry.semconv_ai import SpanAttributes + +from opentelemetry.instrumentation.langchain.callback_handler import ( + _sanitize_metadata_value, +) + +MARKER = "metadata-object-marker-9f3a" + + +class _ClientLikeObject: + """Stands in for a model/client object whose repr renders its config.""" + + def __init__(self, api_key: str) -> None: + """Store the marker the way a real client stores its credential.""" + self.api_key = api_key + + def __repr__(self) -> str: + """Render the credential, as a real client's repr does.""" + return f"_ClientLikeObject(api_key='{self.api_key}')" + + +def test_primitives_are_preserved(): + """The documented use case - string and numeric labels - keeps working.""" + assert _sanitize_metadata_value("12345") == "12345" + assert _sanitize_metadata_value(42) == 42 + assert _sanitize_metadata_value(1.5) == 1.5 + assert _sanitize_metadata_value(True) is True + assert _sanitize_metadata_value(b"bytes") == b"bytes" + + +def test_falsy_primitives_are_preserved_not_dropped(): + """0, False and "" are values, not absences.""" + assert _sanitize_metadata_value(0) == 0 + assert _sanitize_metadata_value(False) is False + assert _sanitize_metadata_value("") == "" + + +def test_object_is_dropped_not_stringified(): + """The leak: an object's repr must never become the attribute value.""" + assert _sanitize_metadata_value(_ClientLikeObject(MARKER)) is None + + +def test_object_inside_a_list_is_dropped(): + """A sequence keeps its primitive elements and loses its object elements.""" + value = _sanitize_metadata_value(["ok", _ClientLikeObject(MARKER)]) + assert value == ["ok"] + assert MARKER not in str(value) + + +def test_plain_dict_is_kept_as_json(): + """A mapping of plain data stays, encoded as JSON rather than a Python repr.""" + value = _sanitize_metadata_value({"tenant": "acme", "retries": 2}) + assert value == '{"tenant": "acme", "retries": 2}' + + +def test_dict_holding_an_object_is_dropped(): + """A mapping is only kept when every value in it is plain data.""" + value = _sanitize_metadata_value({"client": _ClientLikeObject(MARKER)}) + assert value is None + + +def test_object_never_reaches_association_properties(instrument_legacy, span_exporter): + """End to end through the callback handler: the object key never appears. + + Only association properties are checked. The caller's metadata is also + dumped onto ``traceloop.entity.input``, marker and all, but that path is + gated by TRACELOOP_TRACE_CONTENT and dumps the whole input wholesale -- + content capture working as documented, not the ungated trace-wide leak + this fix is about. + """ + from langchain_core.prompts import ChatPromptTemplate + from langchain_core.runnables import RunnableLambda + + chain = ChatPromptTemplate.from_messages( + [("user", "{question}")] + ) | RunnableLambda(lambda prompt: "stubbed") + + chain.invoke( + {"question": "hi"}, + config={ + "metadata": { + "user_id": "12345", + "client": _ClientLikeObject(MARKER), + } + }, + ) + + spans = span_exporter.get_finished_spans() + assert spans, "expected the chain invocation to be traced" + + prefix = f"{SpanAttributes.TRACELOOP_ASSOCIATION_PROPERTIES}." + properties = { + key: value + for span in spans + for key, value in (span.attributes or {}).items() + if key.startswith(prefix) + } + + # The legitimate label still propagates to every span... + assert properties.get(f"{prefix}user_id") == "12345" + + # ...while the object is dropped rather than recorded as its repr. + assert f"{prefix}client" not in properties + for key, value in properties.items(): + assert MARKER not in str(value), f"marker leaked into {key}"