fix: Skip trace url generation when tracing is disabled #1503
+16
−0
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We noticed that fetching a trace_url for recording within an existing span would throw an exception re the
apiattribute not existing if Langfuse tracing was disabled (all of our unit testing).I've added a guard here to return None early before attempting to get the project_id, trace_id, etc, ultimately avoiding that exception.
On our end, its a bit annoying to guard:
if span._langfuse_client._tracing_enabled:, so I figure its worth guarding on this end.I've added a test verifying that exact behavior.
Important
Add guard clause in
get_trace_url()to returnNonewhen tracing is disabled, preventing exceptions, and add a test for this behavior.get_trace_url()inclient.pyto returnNoneif_tracing_enabledisFalse, preventing exceptions when tracing is disabled.test_generate_trace_url_client_disabled()intest_core_sdk.pyto verifyget_trace_url()returnsNonewhen tracing is disabled.This description was created by
for 3bd86a5. You can customize this summary. It will automatically update as commits are pushed.
Disclaimer: Experimental PR review
Greptile Summary
This PR adds a defensive guard to the
get_trace_url()method to returnNoneearly when tracing is disabled, preventing potential downstream issues.The fix:
if not self._tracing_enabled: return Nonecheck at the beginning ofget_trace_url()inlangfuse/_client/client.py:2432-2433get_current_trace_id(),get_current_observation_id())tracing_enabled=FalseThe change is minimal, semantically correct, and improves the robustness of the API when tracing is disabled.
Confidence Score: 5/5
None, and the docstring accommodates this case.Important Files Changed
get_trace_url()when tracing is disabledSequence Diagram
sequenceDiagram participant User participant Langfuse participant get_trace_url participant _get_project_id participant get_current_trace_id participant API User->>Langfuse: get_trace_url(trace_id=None) Langfuse->>get_trace_url: call method alt tracing_enabled is False get_trace_url-->>Langfuse: return None (early exit) Langfuse-->>User: None else tracing_enabled is True get_trace_url->>_get_project_id: fetch project ID _get_project_id->>API: api.projects.get() API-->>_get_project_id: project data _get_project_id-->>get_trace_url: project_id get_trace_url->>get_current_trace_id: get current trace get_current_trace_id-->>get_trace_url: trace_id alt project_id and trace_id exist get_trace_url-->>Langfuse: formatted URL Langfuse-->>User: trace URL string else missing project_id or trace_id get_trace_url-->>Langfuse: None Langfuse-->>User: None end end