Added tracing metadata for LiteLLM provider by Daedaelius · Pull Request #86 · BrowserOperator/browser-operator-core

Daedaelius · 2025-12-26T05:28:52Z

Summary by CodeRabbit

Release Notes

New Features
- Added comprehensive tracing and observability support for LLM calls and agent evaluations, enabling enhanced session grouping and request tracking.
- Implemented automatic tracing metadata propagation throughout the API request and evaluation pipeline.
Improvements
- Enhanced configuration resilience with automatic fallbacks for model endpoints and credentials.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

chatgpt-codex-connector · 2025-12-26T05:28:55Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

claude · 2025-12-26T05:29:27Z

Claude finished @Daedaelius's task —— View job

PR Code Review

Reviewing PR #86: "Added tracing metadata for LiteLLM provider"

Review Checklist:

Read repository guidelines (CLAUDE.md)
Analyze the 14 modified files to understand changes
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Review test coverage implications
Provide comprehensive feedback

Summary

This PR implements tracing metadata support for the LiteLLM provider to enable Langfuse integration. The changes establish a metadata flow from the evaluation framework through the API server, browser agent server, and frontend LLM client to the LiteLLM provider.

✅ Positive Aspects

1. Clean Architecture

Good separation of concerns with tracing metadata flowing through proper channels
Consistent interface design across the stack
Proper fallback mechanisms in LLMClient.ts:216-232

2. Robust Implementation

Graceful handling of missing tracing metadata with fallbacks
Good logging for debugging tracing flow
Proper TypeScript interfaces in LLMTypes.ts

3. Environment Variable Support

Smart endpoint resolution in api-server.js:596-601 with proper fallback chain
Automatic API key fallback for LiteLLM provider

🔍 Issues Found

1. Code Quality Issues

agent-server/nodejs/src/api-server.js:596-601 - Complex nested logic:

const getEndpoint = (tierConfig) => {
  const explicitEndpoint = tierConfig?.endpoint || requestBody.model.endpoint;
  if (explicitEndpoint) return explicitEndpoint;
  // Use env var default for litellm provider
  if (tierConfig?.provider === 'litellm') return defaultLiteLLMEndpoint;
  return undefined;
};

Recommendation: Extract this logic into a separate method resolveEndpointForTier() for better readability and testability.

2. Potential Bugs

front_end/panels/ai_chat/LLM/LLMClient.ts:214 - Async context loss risk:
The fallback to global tracing context could fail in complex async scenarios. Consider passing tracing metadata explicitly through all call chains rather than relying on global state.

3. Security Concerns

front_end/panels/ai_chat/LLM/LiteLLMProvider.ts:214-216 - Metadata exposure:

if (options?.tracingMetadata) {
  payloadBody.metadata = options.tracingMetadata;
}

Concern: Tracing metadata is passed directly to external LiteLLM service without validation or sanitization. Could potentially expose sensitive information.
Recommendation: Implement metadata filtering to only pass whitelisted fields.

4. Missing Error Handling

agent-server/nodejs/src/lib/BrowserAgentServer.js:739-744 - Debug logging without error handling:
The tracing metadata is accessed without checking for potential errors in the object structure.

🏗️ Architecture Considerations

1. Performance

✅ Good: Tracing metadata is only processed when present
✅ Good: Minimal overhead added to existing request flow
⚠️ Consider: Additional object cloning could impact performance in high-throughput scenarios

2. Maintainability

⚠️ The tracing metadata flow spans many files, making it harder to track
⚠️ Mixed patterns: some places use explicit metadata passing, others rely on global context

🧪 Test Coverage

Missing Test Cases:

Tracing metadata validation and sanitization
Fallback behavior when global tracing context is unavailable
Error scenarios when tracing metadata is malformed
Integration tests for end-to-end metadata flow

📋 Recommendations

High Priority:

Add metadata sanitization in LiteLLMProvider.ts before external API calls
Add error handling for malformed tracing objects
Extract complex endpoint resolution logic into separate methods

Medium Priority:

Add comprehensive tests for tracing metadata flow
Consider making tracing metadata passing more consistent (either all explicit or all context-based)
Add validation for required tracing fields

Low Priority:

Add JSDoc documentation for new tracing-related interfaces
Consider adding metrics for tracing metadata usage

🎯 Overall Assessment

This is a well-architected addition that properly implements tracing metadata support. The code follows existing patterns and handles edge cases reasonably well. The main concerns are around security (metadata sanitization) and consistency (mixed explicit/context-based patterns).

Recommendation: Approve with minor changes - Address the metadata sanitization security concern before merging.

coderabbitai · 2025-12-26T05:29:39Z

📝 Walkthrough

Walkthrough

The changes introduce end-to-end tracing metadata propagation from server-side API request handling through client-side LLM calls and agent evaluation flows. Server-side enhancements add environment-driven fallback mechanisms for LiteLLM configuration (endpoint and API key). Tracing metadata is extracted, forwarded, and preserved across protocol layers, evaluation requests, and tool invocations.

Changes

Cohort / File(s)	Summary
Server-side request handling `agent-server/nodejs/src/api-server.js`, `agent-server/nodejs/src/lib/BrowserAgentServer.js`	Added tracing metadata extraction and forwarding through dynamic request creation. Introduced environment-based fallbacks for LiteLLM endpoint and API key (LITELLM_ENDPOINT, LITELLM_API_KEY). Extended `createDynamicRequestNested` signature to accept tracingMetadata parameter and include it in returned EvaluationRequest.
LLM client and provider integration `front_end/panels/ai_chat/LLM/LLMClient.ts`, `front_end/panels/ai_chat/LLM/LLMTypes.ts`, `front_end/panels/ai_chat/LLM/LiteLLMProvider.ts`	Added `tracingMetadata` field to LLMCallRequest and LLMCallOptions interfaces. Updated LLMClient.call to resolve tracingMetadata from request or global context, then pass to provider. Modified LiteLLMProvider to include metadata in callWithMessages payload when provided.
Agent execution framework `front_end/panels/ai_chat/agent_framework/AgentRunner.ts`, `front_end/panels/ai_chat/core/AgentNodes.ts`, `front_end/panels/ai_chat/core/AgentService.ts`	Propagated tracing metadata through agent LLM calls. Modified AgentService to allow missing API key in AUTOMATED_MODE and normalize it to empty string. Updated AgentService to forward existing evaluation metadata within tracingContext.
Evaluation framework `front_end/panels/ai_chat/evaluation/EvaluationAgent.ts`, `front_end/panels/ai_chat/evaluation/EvaluationProtocol.ts`, `front_end/panels/ai_chat/evaluation/remote/EvaluationAgent.ts`, `front_end/panels/ai_chat/evaluation/remote/EvaluationProtocol.ts`	Added optional `tracing` field to EvaluationParams. Updated both local and remote EvaluationAgent to read tracing metadata from requests, use provided trace/session IDs with fallbacks, and propagate metadata through evaluation lifecycle and tool execution. Enhanced logging for tracing metadata receipt.
Tracing infrastructure `front_end/panels/ai_chat/tools/LLMTracingWrapper.ts`, `front_end/panels/ai_chat/tracing/TracingProvider.ts`	Added `metadata` field to TracingContext interface. Updated LLMTracingWrapper to forward tracingContext metadata as tracingMetadata in LLM client calls.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant APIServer
    participant LLMClient
    participant LLMProvider
    participant TracingService

    Client->>APIServer: POST with tracing metadata<br/>(requestBody.metadata)
    Note over APIServer: Extract tracingMetadata<br/>from requestBody
    
    APIServer->>APIServer: createDynamicRequestNested<br/>(input, config, tracingMetadata)
    Note over APIServer: Read fallback endpoints<br/>from LITELLM_ENDPOINT<br/>Read fallback API key<br/>from LITELLM_API_KEY
    
    APIServer->>APIServer: Create EvaluationRequest<br/>with tracing field
    
    APIServer->>LLMClient: call(request with<br/>tracingMetadata)
    
    alt Explicit tracingMetadata
        LLMClient->>LLMClient: Use request.tracingMetadata
    else No explicit metadata
        LLMClient->>TracingService: getCurrentTracingContext()
        TracingService-->>LLMClient: Return global context
    end
    
    Note over LLMClient: Merge metadata into<br/>options.tracingMetadata
    
    LLMClient->>LLMProvider: callWithMessages<br/>(payload with metadata)
    
    Note over LLMProvider: Include metadata in<br/>provider payload
    
    LLMProvider-->>LLMClient: Response
    LLMClient-->>APIServer: Result with tracing
    APIServer-->>Client: Response

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Added OpenAI compatible "input" field support in /v1/responses API #69: Introduced the api-server.js model-config and nested endpoint handling logic that this PR extends with tracingMetadata propagation and LiteLLM fallback configuration.
Fix Config Propagation in Automated Mode #61: Modifies AgentService's AUTOMATED_MODE API key initialization, aligning with this PR's API key normalization changes.
Update agent framework structure #43: Updates agent runtime (AgentRunner, agent nodes, tool calls) to propagate per-run tracing context, directly complementing this PR's tracing infrastructure.

Suggested reviewers

olesho
tysonthomas9

Poem

🐰 Hops through the traces
Metadata dances from queue to LLM,
Each hop leaves a trail,
Sessions grouped in the moon-light glow,
Langfuse smiles as whispers flow. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The title mentions adding tracing metadata for LiteLLM provider, but the changeset encompasses much broader tracing integration across the entire codebase (frontend, backend, agent framework, evaluation systems) with metadata propagation through multiple layers.	Consider updating the title to reflect the broader scope, such as 'Add comprehensive tracing metadata integration across LLM calls' or similar, to accurately represent the full extent of changes beyond just the LiteLLM provider.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/litellm-tracing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

agent-server/nodejs/src/api-server.js (2)

551-556: Consider guarding requestBody.metadata shape before treating it as tracing metadata

Deriving const tracingMetadata = requestBody.metadata || {} works when metadata is an object, but if callers accidentally send a non-object (string/number), that value will be forwarded into tracing and down into evaluation/tracing code. A lightweight guard such as typeof requestBody.metadata === 'object' && requestBody.metadata !== null would keep the protocol robust against malformed inputs.

585-617: LiteLLM endpoint/API key fallbacks mostly correct; a couple of edge cases worth tightening

The new defaultLiteLLMEndpoint and getEndpoint helper correctly prefer tier- or model-level endpoints and then fall back to LITELLM_ENDPOINT when the tier config explicitly has provider: 'litellm'. Likewise, extractModelTierConfig now sensibly falls back to LITELLM_API_KEY for litellm tier objects, and createDefaultModelConfig adds an endpoint for litellm based on defaults or LITELLM_ENDPOINT.

Two small follow‑ups to consider:

For nested configs where provider is only specified at the top level (model.provider === 'litellm') and tier entries are plain strings, getEndpoint(tierConfig) sees only the string and will never look at the provider, so litellm tiers won’t get the LITELLM_ENDPOINT fallback unless an endpoint is explicitly provided. You could extend getEndpoint to also consider requestBody.model.provider when tierConfig lacks a provider field.

Similarly, the string-tier path in extractModelTierConfig always uses OPENAI_API_KEY, even if the effective provider for that tier is litellm via defaults. If you expect string tiers under litellm, it might be worth branching on the resolved provider and using LITELLM_API_KEY in that case.

These are edge cases; the main happy paths for object-based litellm configs are handled well.

Also applies to: 637-654, 689-692

front_end/panels/ai_chat/tracing/TracingProvider.ts (1)

21-30: LGTM! Metadata structure aligns with evaluation protocol.

The metadata field properly extends TracingContext for Langfuse integration. The structure mirrors EvaluationParams.tracing, enabling seamless propagation through the evaluation pipeline.

Consider extracting this type to a shared location to avoid duplication with EvaluationProtocol.ts, but this is a minor cleanup for future work.
front_end/panels/ai_chat/LLM/LLMClient.ts (1)
212-242: Consider using DEBUG level for verbose tracing logs.

The tracing metadata propagation logic is correct. However, the logging at lines 219-225, 230-234, and 239 outputs detailed tracing information at INFO level. In production, this may generate excessive log volume.
🔎 Suggested change to use DEBUG level
-      logger.info('LLMClient.call() - Checking tracing context (fallback):', {
+      logger.debug('LLMClient.call() - Checking tracing context (fallback):', {
         hasContext: !!tracingContext,
         hasMetadata: !!tracingContext?.metadata,
         metadataKeys: tracingContext?.metadata ? Object.keys(tracingContext.metadata) : [],
         sessionId: tracingContext?.metadata?.session_id,
         traceId: tracingContext?.metadata?.trace_id
       });
Apply similar changes to lines 230-234 and 239.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 197f88f and d6bf633.

📒 Files selected for processing (14)

agent-server/nodejs/src/api-server.js
agent-server/nodejs/src/lib/BrowserAgentServer.js
front_end/panels/ai_chat/LLM/LLMClient.ts
front_end/panels/ai_chat/LLM/LLMTypes.ts
front_end/panels/ai_chat/LLM/LiteLLMProvider.ts
front_end/panels/ai_chat/agent_framework/AgentRunner.ts
front_end/panels/ai_chat/core/AgentNodes.ts
front_end/panels/ai_chat/core/AgentService.ts
front_end/panels/ai_chat/evaluation/EvaluationAgent.ts
front_end/panels/ai_chat/evaluation/EvaluationProtocol.ts
front_end/panels/ai_chat/evaluation/remote/EvaluationAgent.ts
front_end/panels/ai_chat/evaluation/remote/EvaluationProtocol.ts
front_end/panels/ai_chat/tools/LLMTracingWrapper.ts
front_end/panels/ai_chat/tracing/TracingProvider.ts

🧰 Additional context used

📓 Path-based instructions (1)

agent-server/nodejs/src/api-server.js

📄 CodeRabbit inference engine (agent-server/nodejs/CLAUDE.md)

agent-server/nodejs/src/api-server.js: Expose REST endpoint POST /v1/responses that accepts task input, URL, timeout, and model configuration, and returns OpenAI-compatible response with metadata
Use formatResponse() method to convert agent responses to OpenAI-compatible format and include metadata with clientId and tabId for screenshot capture
Model configuration must use canonical nested format with main_model, mini_model, and nano_model tiers, each containing provider, model, and api_key fields
POST /page/screenshot endpoint must accept clientId and tabId, use CDP Page.captureScreenshot, and return base64-encoded PNG with metadata and timestamp
POST /page/content endpoint must accept clientId, tabId, format (html or text), and includeIframes parameters; recursively capture iframe content when includeIframes is true
POST /page/execute endpoint must accept clientId, tabId, expression, returnByValue, and awaitPromise; use CDP Runtime.evaluate and return result with type and value
Accept POST /v1/responses input as either string format (simple message) or conversation array format with role and content fields; enforce at least one user message and max 100 messages/10,000 characters per message

Files:

agent-server/nodejs/src/api-server.js

🧠 Learnings (9)