Add messageId and requestId context to all mothership log messages#3770
Add messageId and requestId context to all mothership log messages#3770TheodoreSpeaks merged 1 commit intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Plumbs Written by Cursor Bugbot for commit 55c1a05. This will update automatically on new commits. Configure here. |
Greptile SummaryThis PR adds Key changes:
The implementation is mechanically consistent and correct. One style observation: the IDs are embedded into the message string rather than passed as structured log metadata (see inline comment), which may affect log aggregation/alerting depending on the team's observability tooling. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant ChatRoute as copilot/chat route
participant Orchestrator as orchestrateCopilotStream
participant SSEHandlers as SSE Handlers
participant ToolExec as executeToolAndReport
participant GoBackend as Go Agent Backend
Client->>ChatRoute: POST /api/copilot/chat (messageId?)
Note over ChatRoute: userMessageIdToUse = messageId || uuid()
ChatRoute->>ChatRoute: log("Received chat POST" [requestId messageId])
ChatRoute->>Orchestrator: orchestrateCopilotStream(payload{messageId})
Note over Orchestrator: messageId = payload.messageId || uuid()<br/>execContext.messageId = messageId
Orchestrator->>Orchestrator: log("Starting copilot orchestration" [requestId messageId])
Orchestrator->>GoBackend: SSE stream request
GoBackend-->>SSEHandlers: tool_call event
Note over SSEHandlers: context.messageId propagated
SSEHandlers->>ToolExec: executeToolAndReport(toolCallId, context)
ToolExec->>ToolExec: log("Tool execution started" [messageId])
ToolExec->>GoBackend: markToolComplete(toolCallId, ..., messageId)
GoBackend-->>SSEHandlers: stream_end
Orchestrator->>ChatRoute: OrchestratorResult
ChatRoute->>Client: JSON response
Reviews (1): Last reviewed commit: "Add messageId and requestId context to a..." | Re-trigger Greptile |
| return message | ||
| } | ||
|
|
||
| return `${message} [${suffixParts.join(' ')}]` |
There was a problem hiding this comment.
Consider structured metadata over message-string embedding
appendCopilotLogContext appends requestId and messageId directly into the log message string (e.g. "Tool execution started [requestId:abc messageId:xyz]"). Many structured log systems (Datadog, CloudWatch Insights, etc.) index on the raw message text, meaning each unique ID combination creates a new distinct log "fingerprint." This can make template-based alerting harder and increase log cardinality in the index.
A common alternative is to pass the IDs as structured metadata alongside the message:
// Instead of:
logger.info(appendCopilotLogContext('Tool execution started', { requestId, messageId }), { toolCallId })
// Consider:
logger.info('Tool execution started', { requestId, messageId, toolCallId })This keeps the message template stable while still making both IDs searchable. If the current log aggregation pipeline works better with IDs in the message text (e.g. for plain-text grep), this is fine to keep as-is — just worth flagging for awareness.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
We have some race condition happening between copilot and sim. Added logging to help debug, appending messageId and requestId as context
Type of Change
Testing
Validated logs exist and show correct message id.
Checklist
Screenshots/Videos