Skip to content

Feat/improved logging#3833

Merged
TheodoreSpeaks merged 11 commits intostagingfrom
feat/improved-logging
Mar 30, 2026
Merged

Feat/improved logging#3833
TheodoreSpeaks merged 11 commits intostagingfrom
feat/improved-logging

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

Summary

We need additional observability into our system, even for successful calls.
Add additional logs for the /execute endpoint with metadata for user id, workflow id, workspace id, execution id. Reverted change to upgrade mothership chat logs to error.

I upgraded our logging system to INFO, so all logs should start showing up.

New formatting:

  [2026-03-28T14:32:01.500Z] [DEBUG] [ExecutionLogger] {workflowId=wf-9f3e workspaceId=ws-7720 executionId=exec-88f2} Starting workflow
   execution
  [2026-03-28T14:32:01.512Z] [DEBUG] [ExecutionLogger] {workflowId=wf-9f3e workspaceId=ws-7720 executionId=exec-88f2} Created workflow
  log {"logId":"log-ab12"}
  [2026-03-28T14:32:02.200Z] [DEBUG] [ExecutionLogger] {executionId=exec-88f2} Completing workflow execution {"isResume":false}
  [2026-03-28T14:32:02.350Z] [ERROR] [ExecutionLogger] {workflowId=wf-9f3e executionId=exec-88f2} Workflow not found for user stats
  update

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

Ran workflow execution and validated new metadata logs are shown

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 30, 2026 9:58pm

Request Review

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@BugBot review

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 28, 2026

PR Summary

Low Risk
Primarily refactors logging (levels and structured metadata) across request handlers and executor paths; functional behavior is largely unchanged aside from when/where some IDs are generated for logging context.

Overview
Improves observability by standardizing structured logging across Copilot chat APIs, Mothership endpoints, and workflow execution.

Replaces appendCopilotLogContext(...) string concatenation with logger.withMetadata(...) and generally shifts many success-path logs from error to info/warn, while preserving error logging for actual failures.

Adds consistent request-scoped metadata (requestId, messageId, and for workflow execution also workflowId, workspaceId, userId, executionId) throughout /workflows/[id]/execute, the executor (DAGExecutor, ExecutionEngine, BlockExecutor), and streaming/orchestrator code; updates affected tests to mock withMetadata.

Written by Cursor Bugbot for commit bb5702a. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 28, 2026

Greptile Summary

Adds withMetadata() to the Logger class so structured context is automatically included in log lines. Applied across the execute route, executor classes, ExecutionLogger, and copilot tooling routes — replacing the old appendCopilotLogContext helper and fixing many logger.error() calls used for routine informational events.

Notable changes:

  • Logger package: withMetadata(metadata) returns an immutable child logger with merged key-value pairs. Full unit tests added.
  • Executor classes gain an execLogger field initialised from execution context in each constructor.
  • Execute route builds up reqLogger progressively, replacing [requestId] string interpolation.
  • ExecutionLogger uses metadata-bound loggers throughout, addressing a prior review comment.
  • Minor: two logger.warn() calls in generate-visualization.ts and one logger.error() in user-table.ts still use the bare module logger instead of the local reqLogger, losing messageId context on those lines.

Confidence Score: 5/5

  • Safe to merge — no logic errors or data correctness issues; only minor style inconsistencies remain.
  • All findings are P2: two log calls in generate-visualization.ts and one in user-table.ts use the bare module logger instead of the enriched reqLogger, losing messageId context on those lines. These are observability nits with no impact on correctness or runtime behaviour. The core Logger.withMetadata implementation is clean, well-tested, and correctly applied across the vast majority of the codebase.
  • apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts (lines 98, 134) and apps/sim/lib/copilot/tools/server/table/user-table.ts (line 247) — bare logger used instead of local reqLogger.

Important Files Changed

Filename Overview
packages/logger/src/index.test.ts Comprehensive new test suite for withMetadata: covers immutability, chaining, key override, undefined-value filtering, and empty-metadata edge case.
apps/sim/lib/logs/execution/logger.ts Replaces bare string-interpolated debug/error messages with withMetadata-bound loggers carrying workflowId, workspaceId, and executionId; addresses the prior review comment about completeExecution lacking context.
apps/sim/app/api/workflows/[id]/execute/route.ts Introduces a reqLogger enriched progressively with requestId → workflowId → userId/executionId → workspaceId; replaces all [requestId] … string interpolation with structured metadata.
apps/sim/executor/execution/block-executor.ts Adds execLogger instance field initialised in constructor with workflowId, workspaceId, executionId, userId, requestId; all log calls in class methods migrated to it.
apps/sim/executor/execution/engine.ts Same pattern as block-executor: execLogger initialised in constructor from context fields; all module-level logger calls replaced.
apps/sim/executor/execution/executor.ts Same pattern as engine.ts; execLogger initialised from contextExtensions fields.
apps/sim/lib/copilot/tools/server/visualization/generate-visualization.ts Migrated to reqLogger but two logger.warn calls (lines 98, 134) in collectSandboxFiles were not updated, losing messageId context on those log lines.
apps/sim/lib/copilot/tools/server/table/user-table.ts Migrated to reqLogger except the unauthorized-access error on line 247 which still uses bare logger, losing messageId context.
packages/testing/src/mocks/logger.mock.ts Updated mock to expose withMetadata returning another mock logger, matching the real API.
apps/sim/app/api/workflows/[id]/execute/route.async.test.ts Updated @sim/logger mock to use the factory pattern, returning a recursive mock logger with withMetadata support.

Sequence Diagram

sequenceDiagram
    participant Route as execute/route.ts
    participant Logger as Logger (module)
    participant Child1 as reqLogger {requestId, workflowId}
    participant Child2 as reqLogger {+userId, executionId}
    participant Child3 as reqLogger {+workspaceId}
    participant Executor as DAGExecutor / Engine / BlockExecutor

    Route->>Logger: withMetadata({requestId, workflowId})
    Logger-->>Child1: child logger
    Route->>Child1: info("Starting server-side execution")
    Route->>Child1: withMetadata({userId, executionId})
    Child1-->>Child2: merged child logger
    Route->>Child2: error("Workflow has no workspaceId") [if missing]
    Route->>Child2: withMetadata({workspaceId})
    Child2-->>Child3: merged child logger
    Route->>Child3: info("Preprocessing passed")
    Route->>Executor: new DAGExecutor(contextExtensions)
    Executor->>Logger: withMetadata({workflowId, workspaceId, executionId, userId, requestId})
    Logger-->>Executor: execLogger stored as field
    Executor->>Executor: execLogger.info / warn / error (all log lines carry full context)
Loading

Reviews (4): Last reviewed commit: "Merge branch 'staging' into feat/improve..." | Re-trigger Greptile

waleedlatif1 and others added 5 commits March 28, 2026 15:54
#3830)

* improvement(sidebar): expand sidebar by hovering and clicking the edge

* improvement(sidebar): add keyboard shortcuts for new workflow/task, center search modal, fix edge ARIA

* improvement(sidebar): use Tooltip.Shortcut for inline shortcut display

* fix(sidebar): change new workflow shortcut from Mod+Shift+W to Mod+Shift+P to avoid browser close-window conflict

* fix(hotkeys): fall back to event.code for international keyboard layout compatibility

* fix(sidebar): guard add-workflow shortcut with canEdit and isCreatingWorkflow checks
* feat(ui): handle image paste

* Fix lint

* Fix type error

---------

Co-authored-by: Theodore Li <theo@sim.ai>
* feat(files): interactive markdown checkbox toggling in preview

* fix(files): handle ordered-list checkboxes and fix index drift

* lint

* fix(files): remove counter offset that prevented checkbox toggling

* fix(files): apply task-list styling to ordered lists too

* fix(files): render single pass when interactive to avoid index drift

* fix(files): move useMemo above conditional return to fix Rules of Hooks

* fix(files): pass content directly to preview when not streaming to avoid stale frame
…sistency (#3831)

* improvement(home): position @ mention popup at caret and fix icon consistency

* fix(home): pin mirror div to document origin and guard button anchor

* chore(auth): restore hybrid.ts to staging
@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@BugBot review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@TheodoreSpeaks TheodoreSpeaks marked this pull request as ready for review March 30, 2026 22:49
@TheodoreSpeaks TheodoreSpeaks merged commit d3d58a9 into staging Mar 30, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/improved-logging branch March 31, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants