Skip to content

fix(langchain): callback handler overwrites custom names for task and w… #3324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(langchain): callback handler overwrites custom names for task and w… #3324

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04a29a9
fix(langchain): callback handler overites custom names for task and w…
avivhalfon Aug 18, 2025
ae2fabd
fix full path
avivhalfon Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion ...try-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
LLMResult,
)
from opentelemetry import context as context_api
from opentelemetry.context import get_value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New import of get_value is added. Consider using a consistent approach (e.g., context_api.get_value) throughout the file for clarity.

from opentelemetry.instrumentation.langchain.event_emitter import emit_event
from opentelemetry.instrumentation.langchain.event_models import (
ChoiceEvent,
Expand Down Expand Up @@ -391,6 +392,10 @@ def on_chain_start(
if context_api.get_value(_SUPPRESS_INSTRUMENTATION_KEY):
return

# Check for existing Traceloop context first
existing_workflow_name = get_value("workflow_name")
existing_entity_path = get_value("entity_path")

workflow_name = ""
entity_path = ""

Expand All @@ -402,7 +407,9 @@ def on_chain_start(
)

if kind == TraceloopSpanKindValues.WORKFLOW:
workflow_name = name
# Respect existing Traceloop decorator context if present
workflow_name = existing_workflow_name or name
entity_path = existing_entity_path or ""
else:
workflow_name = self.get_workflow_name(parent_run_id)
entity_path = self.get_entity_path(parent_run_id)
Expand Down Expand Up @@ -724,6 +731,11 @@ def get_workflow_name(self, parent_run_id: str):
return parent_span.workflow_name

def get_entity_path(self, parent_run_id: str):
# Check for existing Traceloop context first
existing_entity_path = get_value("entity_path")
if existing_entity_path:
return existing_entity_path

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix: honor explicitly empty entity_path from context (truthiness bug).

Using a truthy check will ignore an explicitly empty string set by upstream context (a valid, intentional “root” path) and fall back to deriving a path. This reintroduces the overwrite you’re trying to avoid.

Use an is not None check so that "" is preserved.

Apply this diff:

-        # Check for existing Traceloop context first
-        existing_entity_path = get_value("entity_path")
-        if existing_entity_path:
-            return existing_entity_path
+        # Check for existing Traceloop context first
+        existing_entity_path = context_api.get_value("entity_path")
+        # Important: honor empty string ("") as a valid, explicit root path
+        if existing_entity_path is not None:
+            return existing_entity_path

Note: If you adopt this, you can remove the direct get_value import as suggested above.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Check for existing Traceloop context first
existing_entity_path = get_value("entity_path")
if existing_entity_path:
return existing_entity_path
# Check for existing Traceloop context first
existing_entity_path = context_api.get_value("entity_path")
# Important: honor empty string ("") as a valid, explicit root path
if existing_entity_path is not None:
return existing_entity_path
🤖 Prompt for AI Agents 
In
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
around lines 734 to 738, the code uses a truthy check on existing_entity_path
which treats an explicitly empty string ("") as false and incorrectly falls back
to deriving a path; change the condition to check `is not None` so that an
intentionally empty string is preserved (i.e., if get_value("entity_path") is
not None: return that value), and if you implement this change remove the
now-unnecessary direct get_value import noted earlier.

parent_span = self.get_parent_span(parent_run_id)

if parent_span is None:
Expand Down
Loading