fix(telemetry): address review feedback from wenshao

- Add debugLogger.warn in catch blocks of endLLMRequestSpan/endToolSpan/
  endToolExecutionSpan instead of silent swallowing
- Add JSDoc on endToolSpan documenting intentional no-metadata-no-status
  contract with setToolSpanFailure/setToolSpanCancelled
- Add warning in startToolExecutionSpan when called outside
  runInToolSpanContext (no active toolContext)
- Sanitize error message in endToolExecutionSpan: use constant
  TOOL_SPAN_STATUS_TOOL_EXCEPTION instead of raw error message

Author: doudouOUC

packages/core/src/core/coreToolScheduler.ts

@@ -1898,9 +1898,7 @@ export class CoreToolScheduler {
     const toolUseId = generateToolUseId();
 
     // Get MessageBus for hook execution
-    const messageBus = this.config.getMessageBus() as
-      | MessageBus
-      | undefined;
+    const messageBus = this.config.getMessageBus() as MessageBus | undefined;
     const hooksEnabled = !this.config.getDisableAllHooks();
 
     // PreToolUse Hook
@@ -2092,10 +2090,7 @@ export class CoreToolScheduler {
             ? toolResult.resultFilePaths
             : [];
         const candidatePaths = Array.from(
-          new Set([
-            ...inputPaths.map((p) => unescapePath(p)),
-            ...resultPaths,
-          ]),
+          new Set([...inputPaths.map((p) => unescapePath(p)), ...resultPaths]),
         );
 
         if (candidatePaths.length > 0) {
@@ -2137,9 +2132,7 @@ export class CoreToolScheduler {
             // and would waste a turn trying. Gate the reminder on
             // whether the active tool registry actually exposes
             // SkillTool to the model.
-            const hasSkillTool = !!this.toolRegistry.getTool(
-              ToolNames.SKILL,
-            );
+            const hasSkillTool = !!this.toolRegistry.getTool(ToolNames.SKILL);
             if (hasSkillTool) {
               // Escape skill names defensively: validateSkillName already
               // excludes `<>&` for parsed file-based skills, but
@@ -2171,11 +2164,7 @@ export class CoreToolScheduler {
           }
         }
 
-        const response = convertToFunctionResponse(
-          toolName,
-          callId,
-          content,
-        );
+        const response = convertToFunctionResponse(toolName, callId, content);
         const successResponse: ToolCallResponseInfo = {
           callId,
           responseParts: response,
@@ -2232,7 +2221,7 @@ export class CoreToolScheduler {
           : String(executionError);
       endToolExecutionSpan(execSpan, {
         success: false,
-        error: errorMessage,
+        error: TOOL_SPAN_STATUS_TOOL_EXCEPTION,
       });
 
       if (signal.aborted) {

packages/core/src/telemetry/session-tracing.ts

@@ -22,6 +22,9 @@ import {
   SPAN_TOOL_EXECUTION,
 } from './constants.js';
 import { isTelemetrySdkInitialized } from './sdk.js';
+import { createDebugLogger } from '../utils/debugLogger.js';
+
+const debugLogger = createDebugLogger('SESSION_TRACING');
 
 type InteractionStatus = 'ok' | 'error' | 'cancelled';
 
@@ -254,8 +257,10 @@ export function endLLMRequestSpan(
     }
 
     span.end();
-  } catch {
-    // OTel errors must not affect API behavior.
+  } catch (error) {
+    debugLogger.warn(
+      `Failed to end LLM request span: ${error instanceof Error ? error.message : String(error)}`,
+    );
   }
   activeSpans.delete(spanId);
   strongSpans.delete(spanId);
@@ -312,6 +317,12 @@ export function runInToolSpanContext<T>(span: Span, fn: () => T): T {
   return toolContext.run(spanCtx, fn);
 }
 
+/**
+ * When metadata is omitted, span status is NOT set — callers on failure paths
+ * must pre-set status via setToolSpanFailure/setToolSpanCancelled before calling
+ * this. This asymmetry with endLLMRequestSpan (which defaults to OK) is intentional:
+ * tool spans have multiple failure modes that set status before endToolSpan runs.
+ */
 export function endToolSpan(span: Span, metadata?: ToolSpanMetadata): void {
   const spanId = getSpanId(span);
   const spanCtx = activeSpans.get(spanId)?.deref();
@@ -343,8 +354,10 @@ export function endToolSpan(span: Span, metadata?: ToolSpanMetadata): void {
     }
 
     spanCtx.span.end();
-  } catch {
-    // OTel errors must not affect tool scheduling.
+  } catch (error) {
+    debugLogger.warn(
+      `Failed to end tool span: ${error instanceof Error ? error.message : String(error)}`,
+    );
   }
   activeSpans.delete(spanId);
   strongSpans.delete(spanId);
@@ -358,6 +371,11 @@ export function startToolExecutionSpan(): Span {
   }
 
   const parentCtx = toolContext.getStore();
+  if (!parentCtx) {
+    debugLogger.warn(
+      'startToolExecutionSpan called outside runInToolSpanContext — span will not be parented to tool span',
+    );
+  }
   const ctx = parentCtx
     ? trace.setSpan(otelContext.active(), parentCtx.span)
     : otelContext.active();
@@ -416,8 +434,10 @@ export function endToolExecutionSpan(
     }
 
     spanCtx.span.end();
-  } catch {
-    // OTel errors must not affect tool scheduling.
+  } catch (error) {
+    debugLogger.warn(
+      `Failed to end tool execution span: ${error instanceof Error ? error.message : String(error)}`,
+    );
   }
   activeSpans.delete(spanId);
   strongSpans.delete(spanId);