-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(telemetry): updated telemetry, added nested sanitization, added granular trace spans for logs and updated UI #1627
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.
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR significantly enhances the telemetry infrastructure by migrating from a basic implementation to a comprehensive OpenTelemetry-based system with proper semantic conventions for GenAI operations.
Key changes:
- Refactored telemetry from
lib/telemetry.tsinto a dedicatedlib/telemetry/tracer.tswith GenAI semantic conventions - Implemented client-side event batching (10s intervals or 50 events) with nested sanitization to reduce overhead
- Added OpenTelemetry trace span generation from workflow execution logs with proper parent-child relationships
- Enhanced trace spans UI with new modular components (
trace-spans.tsx,trace-span-item.tsx) for better visualization - Improved token normalization to handle multiple formats (object with input/output/prompt/completion fields)
- Added platform event tracking for workflow deployment, undeployment, and execution outcomes
- Updated batch processor settings for better performance (2048 queue size, 512 batch size)
Technical improvements:
- Streaming duration calculation now correctly extends to block end time
- Trace span building handles nested workflows and iteration blocks properly
- Client-side telemetry uses
navigator.sendBeaconfor reliable delivery before page unload
Confidence Score: 4/5
- Safe to merge with minor style improvements recommended
- The PR implements a comprehensive telemetry refactoring with proper error handling and privacy considerations. The code is well-structured with good separation of concerns. However, there are several type assertions to
anythat violate the project's custom instruction (c63aedff-69e6-48d8-81cf-9763416ee01c), and the nested sanitization could be more robust by scanning string values for sensitive patterns. These are non-critical style issues that don't affect functionality. - Pay attention to
apps/sim/lib/telemetry/tracer.tsandapps/sim/instrumentation-client.tsfor type safety and sanitization improvements
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/lib/telemetry/tracer.ts | 5/5 | New comprehensive OpenTelemetry tracer implementation with semantic conventions for GenAI operations - well-structured with proper error handling |
| apps/sim/instrumentation-client.ts | 4/5 | Client-side telemetry refactored to use event batching and nested sanitization - improved performance but sanitization could be more robust |
| apps/sim/lib/logs/execution/trace-spans/trace-spans.ts | 5/5 | Enhanced trace span building with improved token normalization and streaming duration fixes |
| apps/sim/app/workspace/[workspaceId]/logs/components/trace-spans/trace-spans.tsx | 5/5 | New comprehensive trace spans UI component with timeline visualization, filtering, and responsive design |
Sequence Diagram
sequenceDiagram
participant Client as Browser Client
participant API as Telemetry API
participant Collector as OTel Collector
participant Executor as Workflow Executor
participant Logger as Logging Session
participant Tracer as OTel Tracer
Note over Client: Client-side Telemetry
Client->>Client: Batch events (10s or 50 events)
Client->>Client: Sanitize events (remove sensitive data)
Client->>API: POST /api/telemetry (batch)
API->>API: Validate & sanitize data
API->>Collector: Forward to OTel endpoint
Note over Executor,Tracer: Server-side Workflow Execution
Executor->>Logger: Start workflow execution
Logger->>Logger: Create execution log entry
Executor->>Executor: Execute workflow blocks
Executor->>Logger: Log block executions
Logger->>Logger: Build trace spans from logs
Executor->>Logger: Complete workflow execution
Logger->>Tracer: createOTelSpansForWorkflowExecution()
Tracer->>Tracer: Convert TraceSpans to OTel spans
Tracer->>Collector: Export spans via BatchSpanProcessor
Logger->>Tracer: trackPlatformEvent('workflow.executed')
Tracer->>Collector: Send platform event span
Additional Comments (1)
-
apps/sim/lib/logs/execution/trace-spans/trace-spans.ts, line 118-150 (link)style: type assertion to
any- consider defining a proper TokenData type interface instead of usinganyContext Used: Context from
dashboard- Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used... (source)
28 files reviewed, 3 comments
…ranular trace spans for logs and updated UI (#1627) * feat(logs): updated telemetry, added nested sanitization, added granular trace spans for logs and updated UI * refactor trace spans into separate components * remove any's from tool defs * updated UI and overlayed spans * cleanup * ack PR comments * stricter type safety * clean
Summary
updated telemetry, added nested sanitization, added granular trace spans for logs and updated UI
Type of Change
Testing
Tested manually
Checklist