diff --git a/litellm/integrations/otel/plumbing/providers.py b/litellm/integrations/otel/plumbing/providers.py index d81ef89f0fb..aec4b780324 100644 --- a/litellm/integrations/otel/plumbing/providers.py +++ b/litellm/integrations/otel/plumbing/providers.py @@ -500,13 +500,14 @@ def _is_litellm_span(span: ReadableSpan) -> bool: def with_request_user_id(span: ReadableSpan) -> ReadableSpan: """``span`` carrying the id the request's caller named, for a backend that drops a span without one. - Called as the span ends, on the request's own task. A held span must already have - been through here, since a tree can be released on another request's task. + That id replaces a ``user.id`` already on the span, such as what a mapper wrote for another backend, + so the whole request names one caller. Called as the span ends, on the request's own task. A held span + must already have been through here, since a tree can be released on another request's task. """ user_id: Final = request_user_id() - attributes: Final = span.attributes or _NO_ATTRIBUTES - if user_id is None or User.ID in attributes: + if user_id is None: return span + attributes: Final = span.attributes or _NO_ATTRIBUTES return _CountedSpanView(span, MappingProxyType({**attributes, User.ID: user_id})) diff --git a/litellm/proxy/common_utils/callback_config_validation.py b/litellm/proxy/common_utils/callback_config_validation.py index f4197433a0b..9cc449a6cb4 100644 --- a/litellm/proxy/common_utils/callback_config_validation.py +++ b/litellm/proxy/common_utils/callback_config_validation.py @@ -504,8 +504,8 @@ def _mixpanel_config_error(callback_vars: Mapping[str, str]) -> str | None: f"Supported regions: {', '.join(sorted(MIXPANEL_OTLP_ENDPOINT_BY_REGION))}." ) - token: Final = callback_vars.get("mixpanel_project_token") - if not token or not token.strip() or token in _UNSENDABLE_MIXPANEL_TOKENS: + token: Final = (callback_vars.get("mixpanel_project_token") or "").strip() + if not token or token in _UNSENDABLE_MIXPANEL_TOKENS: return "mixpanel_project_token must be the project's own token, sent in full on every create or update." unknown: Final = tuple( key for key in callback_vars if key.startswith(_MIXPANEL_VAR_PREFIX) and key not in _MIXPANEL_VARS diff --git a/tests/integration/observability/test_mixpanel_user_id.py b/tests/integration/observability/test_mixpanel_user_id.py index eb256f3e4a7..e04cb57f663 100644 --- a/tests/integration/observability/test_mixpanel_user_id.py +++ b/tests/integration/observability/test_mixpanel_user_id.py @@ -525,7 +525,9 @@ def test_every_request_on_a_long_lived_key_names_its_own_caller( ) settled: Final = tuple((caller, _settled_trace(mixpanel.control_url, trace_id)) for caller, trace_id in traces) - assert [(caller, _without_the_id(_reached(mixpanel, spans, host), caller)) for caller, spans in settled] == [ + at_project: Final = tuple((caller, _reached(mixpanel, spans, host)) for caller, spans in settled) + assert [caller for caller, spans in at_project if not spans] == [] + assert [(caller, _without_the_id(spans, caller)) for caller, spans in at_project] == [ (caller, []) for caller in callers ] @@ -685,8 +687,8 @@ def test_a_collector_never_gets_the_id_only_a_team_project_asked_for( @pytest.mark.parametrize( "token", - ["", " ", None, "None", "***REDACTED***"], - ids=["empty", "whitespace", "null", "none-string", "masked"], + ["", " ", None, "None", " None ", "***REDACTED***", " ***REDACTED*** "], + ids=["empty", "whitespace", "null", "none-string", "padded-none-string", "masked", "padded-masked"], ) def test_a_team_or_key_callback_refuses_a_token_it_could_never_send( token: JsonValue, operator_setup: OwnedProxy, mixpanel: ConnectSink @@ -766,6 +768,55 @@ def test_mixpanel_listed_with_another_callback_gets_one_labeled_copy_of_the_mode assert _sent_to(_exports_of(mixpanel, at_mixpanel)) == {(OPERATOR_HOST, _authorization(OPERATOR_TOKEN))} +def test_the_operators_project_names_one_caller_when_a_request_also_sends_a_trace_user_id( + combined_setup: tuple[OwnedProxy, tuple[str, ...]], provider: Wire, mixpanel: ConnectSink, audit_sinks: SpanSinks +) -> None: + owned, callbacks = combined_setup + proxy: Final = owned.gateway + caller: Final = f"caller-{uuid.uuid4().hex}" + traced_as: Final = f"traced-as-{uuid.uuid4().hex}" + with proxy.scenario() as scenario: + model: Final = scenario.model(api_base=provider.url + "/v1") + key: Final = scenario.key(key_alias=f"mixpanel-two-ids-{uuid.uuid4().hex}") + trace_id: Final = _traced_call( + proxy, + key, + {"model": model, **_chat_call("hi"), "metadata": {"user.id": caller, "trace_user_id": traced_as}}, + ) + at_mixpanel: Final = _settled_trace(mixpanel.control_url, trace_id, _has_model_call) + at_other: Final = _settled_trace(audit_sinks.operator, trace_id, _has_model_call) + + assert _without_the_id(_outside_auth(at_mixpanel), caller) == [] + assert {span["attributes"].get(USER_ID) for span in at_other if OPERATION in span["attributes"]} == ( + {traced_as} if "langfuse_otel" in callbacks else {None} + ) + + +def test_a_team_project_names_one_caller_when_a_request_also_sends_a_trace_user_id( + combined_setup: tuple[OwnedProxy, tuple[str, ...]], provider: Wire, mixpanel: ConnectSink +) -> None: + owned, _ = combined_setup + proxy: Final = owned.gateway + caller: Final = f"caller-{uuid.uuid4().hex}" + with proxy.scenario() as scenario: + model: Final = scenario.model(api_base=provider.url + "/v1") + key, host, _token = _team_or_key(scenario, "team") + trace_id: Final = _traced_call( + proxy, + key, + { + "model": model, + **_chat_call("hi"), + "metadata": {"user.id": caller, "trace_user_id": f"traced-as-{uuid.uuid4().hex}"}, + }, + ) + spans: Final = _settled_trace(mixpanel.control_url, trace_id, _has_model_call) + + at_project: Final = _reached(mixpanel, spans, host) + assert [span for span in at_project if OPERATION in span["attributes"]], at_project + assert _without_the_id(at_project, caller) == [] + + @pytest.mark.parametrize("entry", ["mcp", "rest"]) def test_a_team_project_gets_the_callers_id_on_an_mcp_tool_call( entry: EntryPoint, tenant_setup: OwnedProxy, mixpanel: ConnectSink diff --git a/tests/unit/integrations/otel/test_otel_v2_destinations.py b/tests/unit/integrations/otel/test_otel_v2_destinations.py index c14fff5afbd..202e359abe5 100644 --- a/tests/unit/integrations/otel/test_otel_v2_destinations.py +++ b/tests/unit/integrations/otel/test_otel_v2_destinations.py @@ -5977,7 +5977,7 @@ class TestMixpanelGetsTheCallersId: assert "session.id" not in by_name(mixpanel)["chat gpt-4o"].attributes assert "user.id" not in by_name(other)["chat gpt-4o"].attributes - def test_the_id_the_mapper_wrote_wins_over_the_id_read_at_auth(self) -> None: + def test_the_id_read_at_auth_replaces_the_id_a_mapper_wrote(self) -> None: provider, mixpanel, _ = self._fan_out() self._request( @@ -5986,9 +5986,22 @@ class TestMixpanelGetsTheCallersId: model_call_attributes={"gen_ai.operation.name": "chat", "user.id": "from-mapper"}, ) - assert by_name(mixpanel)["chat gpt-4o"].attributes.get("user.id") == "from-mapper" + assert by_name(mixpanel)["chat gpt-4o"].attributes.get("user.id") == "caller-1" assert by_name(mixpanel)["POST /v1/chat/completions"].attributes.get("user.id") == "caller-1" + def test_a_span_from_other_code_names_the_caller_only_for_mixpanel(self) -> None: + provider, mixpanel, other = self._fan_out() + + def run() -> None: + set_request_destinations((self.MIXPANEL, self.ARIZE)) + set_request_user_id("caller-1") + get_tracer(provider, "application").start_span("checkout", attributes={"user.id": "its-own"}).end() + + in_fresh_context(run) + + assert by_name(mixpanel)["checkout"].attributes.get("user.id") == "caller-1" + assert by_name(other)["checkout"].attributes.get("user.id") == "its-own" + def test_a_request_without_an_id_adds_nothing(self) -> None: provider, mixpanel, _ = self._fan_out() diff --git a/tests/unit/proxy/management_endpoints/test_callback_management_endpoints.py b/tests/unit/proxy/management_endpoints/test_callback_management_endpoints.py index 84712f1b81c..34347519eee 100644 --- a/tests/unit/proxy/management_endpoints/test_callback_management_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_callback_management_endpoints.py @@ -630,7 +630,7 @@ class TestMixpanelCallbackValidation: with pytest.raises(HTTPException) as exc: raise_on_invalid_key_logging_config(self._key_metadata({"mixpanel_region": "eu"})) assert "mixpanel_project_token must be" in str(exc.value.detail) - for unsendable in ("", " ", "None", "***REDACTED***"): + for unsendable in ("", " ", "None", " None ", "***REDACTED***", " ***REDACTED*** "): with pytest.raises(HTTPException) as exc: _validate_team_callback(self._team_data({"mixpanel_project_token": unsendable})) assert "mixpanel_project_token must be" in str(exc.value.detail)