address pr comments

Author: yiyuan-he

instrumentation-genai/opentelemetry-instrumentation-langchain/src/opentelemetry/instrumentation/langchain/callback_handler.py

@@ -22,7 +22,10 @@
 from langchain_core.messages import BaseMessage  # type: ignore
 from langchain_core.outputs import LLMResult  # type: ignore
 
-from opentelemetry.instrumentation.langchain.span_manager import _SpanManager
+from opentelemetry.instrumentation.langchain.span_manager import (
+    _OPERATION_INVOKE_AGENT,
+    _SpanManager,
+)
 from opentelemetry.semconv._incubating.attributes import (
     gen_ai_attributes as GenAI,
 )
@@ -260,7 +263,7 @@ def on_chain_start(
         # If this is an agent chain, set agent-specific attributes
         if metadata and "agent_name" in metadata:
             span.set_attribute(GenAI.GEN_AI_AGENT_NAME, metadata["agent_name"])
-            span.set_attribute(GenAI.GEN_AI_OPERATION_NAME, "invoke_agent")
+            span.set_attribute(GenAI.GEN_AI_OPERATION_NAME, _OPERATION_INVOKE_AGENT)
 
     def on_chain_end(
         self,

instrumentation-genai/opentelemetry-instrumentation-langchain/src/opentelemetry/instrumentation/langchain/span_manager.py

@@ -25,7 +25,10 @@
 from opentelemetry.trace import Span, SpanKind, Tracer, set_span_in_context
 from opentelemetry.trace.status import Status, StatusCode
 
-__all__ = ["_SpanManager"]
+__all__ = ["_SpanManager", "_OPERATION_INVOKE_AGENT"]
+
+# Operation name constants
+_OPERATION_INVOKE_AGENT = "invoke_agent"
 
 
 @dataclass
@@ -98,9 +101,9 @@ def create_agent_span(
         agent_name: Optional[str] = None,
     ) -> Span:
         """Create a span for agent invocation."""
-        span_name = (
-            f"invoke_agent {agent_name}" if agent_name else "invoke_agent"
-        )
+        # Use "unknown" as default if agent_name is not provided
+        effective_agent_name = agent_name or "unknown"
+        span_name = f"{_OPERATION_INVOKE_AGENT} {effective_agent_name}"
         span = self._create_span(
             run_id=run_id,
             parent_run_id=parent_run_id,
@@ -109,10 +112,9 @@ def create_agent_span(
         )
         span.set_attribute(
             GenAI.GEN_AI_OPERATION_NAME,
-            "invoke_agent",
+            _OPERATION_INVOKE_AGENT,
         )
-        if agent_name:
-            span.set_attribute(GenAI.GEN_AI_AGENT_NAME, agent_name)
+        span.set_attribute(GenAI.GEN_AI_AGENT_NAME, effective_agent_name)
 
         return span
 
@@ -122,15 +124,19 @@ def create_chain_span(
         parent_run_id: Optional[UUID],
         chain_name: str,
     ) -> Span:
-        """Create a span for chain execution."""
+        """Create a span for chain execution.
+
+        Chains are internal operations by default and don't have gen_ai.operation.name.
+        However, if the chain represents an agent (determined by metadata in the callback),
+        the operation name and agent name attributes will be set separately by the
+        callback handler to make it an agent span.
+        """
         span = self._create_span(
             run_id=run_id,
             parent_run_id=parent_run_id,
             span_name=f"chain {chain_name}",
             kind=SpanKind.INTERNAL,
         )
-        # Chains are internal operations, not direct GenAI operations
-        # We can track them but they don't have a gen_ai.operation.name
         return span
 
     def end_span(self, run_id: UUID) -> None:

instrumentation-genai/opentelemetry-instrumentation-langchain/tests/test_agent_spans.py

@@ -22,11 +22,17 @@ def callback_handler(tracer_provider):
 
 
 def test_agent_chain_span(callback_handler, span_exporter):
-    """Test that agent chains create proper invoke_agent spans."""
+    """Test that agent chains create proper invoke_agent spans.
+
+    Note: Agent chains are created via create_chain_span() but are enhanced
+    with agent-specific attributes (gen_ai.agent.name and gen_ai.operation.name)
+    when metadata contains 'agent_name'. This is different from regular chains
+    which remain as internal operations without gen_ai.operation.name.
+    """
     run_id = uuid4()
     parent_run_id = uuid4()
 
-    # Start a chain that represents an agent
+    # Start a chain that represents an agent (note: metadata includes agent_name)
     callback_handler.on_chain_start(
         serialized={
             "name": "TestAgent",
@@ -52,6 +58,7 @@ def test_agent_chain_span(callback_handler, span_exporter):
     span = spans[0]
     assert span.name == "chain TestAgent"
     assert span.kind == SpanKind.INTERNAL
+    # Agent chains have these attributes set via callback_handler when metadata contains agent_name
     assert span.attributes.get(GenAI.GEN_AI_AGENT_NAME) == "TestAgent"
     assert span.attributes.get(GenAI.GEN_AI_OPERATION_NAME) == "invoke_agent"