feat(tracing): allow custom OpenTelemetry ID generators#1724
Conversation
|
@claude review |
There was a problem hiding this comment.
LGTM — straightforward additive parameter for the OpenTelemetry ID generator, properly threaded through and well-tested.
Extended reasoning...
Overview
This PR adds an optional id_generator parameter to the Langfuse client constructor, threading it through LangfuseResourceManager and into _init_tracer_provider so callers can supply a custom OpenTelemetry IdGenerator when Langfuse creates its own TracerProvider. The parameter is also stored on the resource manager instance so _create_client_from_instance in get_client.py preserves it when reconstructing clients. Unit tests cover both the default (RandomIdGenerator) and a custom predictable generator case.
Security risks
None. The new parameter only affects ID generation semantics inside the local OpenTelemetry SDK. There is no auth, crypto, or data exposure surface touched.
Level of scrutiny
Low-to-moderate. The change is purely additive (new optional kwarg defaulting to None), preserves existing behavior in all code paths, and emits a clear warning when the generator is ignored because an OTel provider is already registered globally. The diff is small, follows the established pattern used by sibling parameters like tracer_provider and span_exporter, and the new tests directly exercise both branches.
Other factors
No outstanding reviewer comments, no prior review from me on this PR, the bug hunting system found nothing, and the author lists ruff/mypy/pytest verification in the PR description. The change is small enough and follows the existing conventions closely enough that human review is not strictly required.
Summary
id_generatorparameter toLangfusefor the OpenTelemetryTracerProviderthat Langfuse creates.RandomIdGeneratorwhen omitted.Verification
uv run --frozen ruff check .uv run --frozen mypy langfuse --no-error-summaryuv run --frozen pytest tests/unit/test_otel.py::TestOtelIdGeneration -quv run --frozen pytest tests/unit/test_resource_manager.py tests/unit/test_initialization.py -qGreptile Summary
This PR adds an optional
id_generatorparameter toLangfuseso callers can plug in a custom OpenTelemetryIdGeneratorwhen Langfuse creates its ownTracerProvider; default behavior (random IDs) is preserved when the parameter is omitted.id_generatoris threaded throughLangfuse→LangfuseResourceManager→_init_tracer_provider→TracerProvider, with a clear ownership boundary: the parameter is only honoured when Langfuse creates a new provider; a warning is logged when a global provider is already registered.get_client.pyis updated to forwardid_generatorwhen reconstructing a client from an existing resource-manager singleton (safe because the singleton short-circuits re-initialization).PredictableIdGeneratorhelper and the existingautouseOTel-reset fixture.Confidence Score: 4/5
Safe to merge; the change is additive and the default path is unchanged.
The implementation is correct and well-tested. The one gap is that passing both an explicit
tracer_providerand anid_generatorsilently drops the generator with no diagnostic, while the analogous globally-registered-provider case does warn. This is a usability inconsistency rather than a data or functional defect, but a user who misconfigures this combination will see no feedback.The branch in
_initialize_instancewheretracer_provideris provided by the caller deserves a second look for the missing warning.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant Langfuse participant LangfuseResourceManager participant _init_tracer_provider participant TracerProvider User->>Langfuse: "Langfuse(id_generator=custom_gen)" Langfuse->>LangfuseResourceManager: "__new__(public_key, id_generator=custom_gen)" LangfuseResourceManager->>LangfuseResourceManager: "_initialize_instance(id_generator=custom_gen)" alt tracer_provider arg is None LangfuseResourceManager->>_init_tracer_provider: "(id_generator=custom_gen)" alt Global provider is ProxyTracerProvider _init_tracer_provider->>TracerProvider: "TracerProvider(id_generator=custom_gen)" TracerProvider-->>_init_tracer_provider: new provider _init_tracer_provider-->>LangfuseResourceManager: new provider (custom_gen applied) else Global provider already registered _init_tracer_provider->>_init_tracer_provider: log warning (id_generator ignored) _init_tracer_provider-->>LangfuseResourceManager: existing global provider end else tracer_provider arg is provided Note over LangfuseResourceManager: id_generator silently ignored (no warning emitted) LangfuseResourceManager->>LangfuseResourceManager: "self.tracer_provider = arg" end LangfuseResourceManager-->>Langfuse: resource manager Langfuse-->>User: client ready%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant Langfuse participant LangfuseResourceManager participant _init_tracer_provider participant TracerProvider User->>Langfuse: "Langfuse(id_generator=custom_gen)" Langfuse->>LangfuseResourceManager: "__new__(public_key, id_generator=custom_gen)" LangfuseResourceManager->>LangfuseResourceManager: "_initialize_instance(id_generator=custom_gen)" alt tracer_provider arg is None LangfuseResourceManager->>_init_tracer_provider: "(id_generator=custom_gen)" alt Global provider is ProxyTracerProvider _init_tracer_provider->>TracerProvider: "TracerProvider(id_generator=custom_gen)" TracerProvider-->>_init_tracer_provider: new provider _init_tracer_provider-->>LangfuseResourceManager: new provider (custom_gen applied) else Global provider already registered _init_tracer_provider->>_init_tracer_provider: log warning (id_generator ignored) _init_tracer_provider-->>LangfuseResourceManager: existing global provider end else tracer_provider arg is provided Note over LangfuseResourceManager: id_generator silently ignored (no warning emitted) LangfuseResourceManager->>LangfuseResourceManager: "self.tracer_provider = arg" end LangfuseResourceManager-->>Langfuse: resource manager Langfuse-->>User: client readyPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "Merge branch 'main' into hassiebbot/otel..." | Re-trigger Greptile