Skip to content

Commit 8f153e4

Browse files
committed
improvement(logs): move correlation ids from message tags into metadata and enforce it
Log messages no longer open with an interpolated [${id}] tag or interpolate a request id. The id moves into the line's metadata, so messages group in queries and lines filter by field. Inside withRouteHandler a generateRequestId() id is already stamped on every line by the request context, so those tags are dropped without a field. Routes, table runners, the Sim event emitter and the poll orchestrator that minted their own correlation ids now use generateRequestId(), so their lines carry the request's id. Each webhook poll logs through a logger bound to its webhook id, so concurrent polls in one tick stay distinguishable. check:source-conventions gains log-message-tag: it covers every app and package that imports @sim/logger, follows a message through concatenation, ternaries, local consts and helpers handed the caller's logger, and starts at zero.
1 parent 7625929 commit 8f153e4

325 files changed

Lines changed: 3891 additions & 3277 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.agents/skills/add-trigger/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ export const {service}PollingHandler: PollingProviderHandler = {
411411
await markWebhookSuccess(webhookId, logger)
412412
return 'success'
413413
} catch (error) {
414-
logger.error(`[${requestId}] Error processing {service} webhook ${webhookId}:`, error)
414+
logger.error(`Error processing {service} webhook ${webhookId}:`, error, { requestId })
415415
await markWebhookFailed(webhookId, logger)
416416
return 'failure'
417417
}

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ This file (also `AGENTS.md`) holds the repo-wide rules. Area detail lives in `.c
55
## Global Standards
66

77
- **Package manager**: `bun` and `bunx`, never `npm` and `npx`.
8-
- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Inside `withRouteHandler` the logger already carries the request ID — no manual `withMetadata({ requestId })`.
8+
- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Messages are static text; ids go in metadata (`logger.info('Sync started', { requestId, documentId })`), never a `[${requestId}]` tag or an interpolated request id, so lines group and filter by field (`check:source-conventions` rule `log-message-tag`). Inside `withRouteHandler` every line already carries the request ID, so a `generateRequestId()` id there needs no field and no manual `withMetadata({ requestId })`.
99
- **Comments**: name things so the code explains itself. TSDoc documents exported APIs and non-obvious modules. An inline `//` is only for a terse, non-obvious *why*, or for a script-enforced `// <tag>: <reason>` annotation (`boundary-raw-fetch`, `double-cast-allowed`, `boundary-raw-json`, `untyped-response`, `rq-lint-allow`, `client-boundary-allow`, `utils-lint-allow`, …). History belongs in the commit message. No `====` separators, commented-out code, or ESLint directives (`eslint-disable`/`eslint-enable`; nothing runs ESLint; `check:comment-hygiene` enforces all three). The `/you-might-not-need-a-comment` skill applies this to a diff.
1010
- **ID generation**: `generateId()` (UUID v4, the default) or `generateShortId(size?)` (URL-safe, 21 chars by default) from `@sim/utils/id` — never `crypto.randomUUID()`, `nanoid`, or `uuid`. Both use `crypto.getRandomValues()`, so they also work in non-secure (HTTP) browsers. For other randomness, `@sim/utils/random` (`randomInt`, `randomFloat`, `randomItem`, `generateRandomBytes`, `generateRandomHex`) — never `Math.random()` or `crypto.randomBytes()`.
1111
- **Common utilities**: use the shared helpers from the `@sim/utils/<module>` subpaths instead of inline implementations (`check:utils` bans most of the inline forms below):

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ This file (also `AGENTS.md`) holds the repo-wide rules. Area detail lives in `.c
55
## Global Standards
66

77
- **Package manager**: `bun` and `bunx`, never `npm` and `npx`.
8-
- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Inside `withRouteHandler` the logger already carries the request ID — no manual `withMetadata({ requestId })`.
8+
- **Logging**: `createLogger` from `@sim/logger`; `logger.info` / `logger.warn` / `logger.error`, never `console.log`. Messages are static text; ids go in metadata (`logger.info('Sync started', { requestId, documentId })`), never a `[${requestId}]` tag or an interpolated request id, so lines group and filter by field (`check:source-conventions` rule `log-message-tag`). Inside `withRouteHandler` every line already carries the request ID, so a `generateRequestId()` id there needs no field and no manual `withMetadata({ requestId })`.
99
- **Comments**: name things so the code explains itself. TSDoc documents exported APIs and non-obvious modules. An inline `//` is only for a terse, non-obvious *why*, or for a script-enforced `// <tag>: <reason>` annotation (`boundary-raw-fetch`, `double-cast-allowed`, `boundary-raw-json`, `untyped-response`, `rq-lint-allow`, `client-boundary-allow`, `utils-lint-allow`, …). History belongs in the commit message. No `====` separators, commented-out code, or ESLint directives (`eslint-disable`/`eslint-enable`; nothing runs ESLint; `check:comment-hygiene` enforces all three). The `/you-might-not-need-a-comment` skill applies this to a diff.
1010
- **ID generation**: `generateId()` (UUID v4, the default) or `generateShortId(size?)` (URL-safe, 21 chars by default) from `@sim/utils/id` — never `crypto.randomUUID()`, `nanoid`, or `uuid`. Both use `crypto.getRandomValues()`, so they also work in non-secure (HTTP) browsers. For other randomness, `@sim/utils/random` (`randomInt`, `randomFloat`, `randomItem`, `generateRandomBytes`, `generateRandomHex`) — never `Math.random()` or `crypto.randomBytes()`.
1111
- **Common utilities**: use the shared helpers from the `@sim/utils/<module>` subpaths instead of inline implementations (`check:utils` bans most of the inline forms below):

‎apps/sim/app/api/auth/oauth/credentials/route.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { type NextRequest, NextResponse } from 'next/server'
77
import { oauthCredentialsQuerySchema } from '@/lib/api/contracts/credentials'
88
import { getValidationErrorMessage } from '@/lib/api/server'
99
import { AuthType, type AuthTypeValue, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
10-
import { generateRequestId } from '@/lib/core/utils/request'
1110
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1211
import { canUseCredential, getCredentialActorContext } from '@/lib/credentials/access'
1312
import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
@@ -84,8 +83,6 @@ async function integrationsWithheldFromSession(
8483
* Get credentials for a specific provider
8584
*/
8685
export const GET = withRouteHandler(async (request: NextRequest) => {
87-
const requestId = generateRequestId()
88-
8986
try {
9087
const { searchParams } = new URL(request.url)
9188
const rawQuery = {
@@ -100,11 +97,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
10097
if (!parseResult.success) {
10198
const refinementError = parseResult.error.issues.find((err) => err.code === 'custom')
10299
if (refinementError) {
103-
logger.warn(`[${requestId}] Invalid query parameters: ${refinementError.message}`)
100+
logger.warn(`Invalid query parameters: ${refinementError.message}`)
104101
return NextResponse.json({ error: refinementError.message }, { status: 400 })
105102
}
106103

107-
logger.warn(`[${requestId}] Invalid query parameters`, {
104+
logger.warn('Invalid query parameters', {
108105
errors: parseResult.error.issues,
109106
})
110107

@@ -119,7 +116,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
119116
// Authenticate requester (supports session and internal JWT)
120117
const authResult = await checkSessionOrInternalAuth(request)
121118
if (!authResult.success || !authResult.userId) {
122-
logger.warn(`[${requestId}] Unauthenticated credentials request rejected`)
119+
logger.warn('Unauthenticated credentials request rejected')
123120
return NextResponse.json({ error: 'User not authenticated' }, { status: 401 })
124121
}
125122
const requesterUserId = authResult.userId
@@ -132,7 +129,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
132129
action: 'read',
133130
})
134131
if (!workflowAuthorization.allowed) {
135-
logger.warn(`[${requestId}] Forbidden credentials request for workflow`, {
132+
logger.warn('Forbidden credentials request for workflow', {
136133
requesterUserId,
137134
workflowId,
138135
status: workflowAuthorization.status,
@@ -378,7 +375,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
378375

379376
return NextResponse.json({ credentials: [] }, { status: 200 })
380377
} catch (error) {
381-
logger.error(`[${requestId}] Error fetching OAuth credentials`, error)
378+
logger.error('Error fetching OAuth credentials', error)
382379
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
383380
}
384381
})

‎apps/sim/app/api/auth/oauth/token/route.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const logger = createLogger('OAuthTokenAPI')
3333
export const POST = withRouteHandler(async (request: NextRequest) => {
3434
const requestId = generateRequestId()
3535

36-
logger.info(`[${requestId}] OAuth token API POST request received`)
36+
logger.info('OAuth token API POST request received')
3737

3838
try {
3939
const parsed = await parseRequest(
@@ -42,7 +42,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4242
{},
4343
{
4444
validationErrorResponse: (error) => {
45-
logger.warn(`[${requestId}] Invalid token request`, { errors: error.issues })
45+
logger.warn('Invalid token request', { errors: error.issues })
4646
return NextResponse.json(
4747
{ error: getValidationErrorMessage(error, 'Validation failed') },
4848
{ status: 400 }
@@ -64,14 +64,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6464
const callerUserId = parsed.data.query.userId
6565

6666
if (credentialAccountUserId && providerId) {
67-
logger.info(`[${requestId}] Fetching token by credentialAccountUserId + providerId`, {
67+
logger.info('Fetching token by credentialAccountUserId + providerId', {
6868
credentialAccountUserId,
6969
providerId,
7070
})
7171

7272
const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
7373
if (!auth.success || auth.authType !== AuthType.SESSION || !auth.userId) {
74-
logger.warn(`[${requestId}] Unauthorized request for credentialAccountUserId path`, {
74+
logger.warn('Unauthorized request for credentialAccountUserId path', {
7575
success: auth.success,
7676
authType: auth.authType,
7777
})
@@ -80,7 +80,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8080

8181
if (auth.userId !== credentialAccountUserId) {
8282
logger.warn(
83-
`[${requestId}] User ${auth.userId} attempted to access credentials for ${credentialAccountUserId}`
83+
`User ${auth.userId} attempted to access credentials for ${credentialAccountUserId}`
8484
)
8585
return NextResponse.json({ error: 'Unauthorized' }, { status: 403 })
8686
}
@@ -117,7 +117,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
117117
return NextResponse.json({ accessToken }, { status: 200 })
118118
} catch (error) {
119119
const message = getErrorMessage(error, 'Failed to get OAuth token')
120-
logger.warn(`[${requestId}] OAuth token error: ${message}`)
120+
logger.warn(`OAuth token error: ${message}`)
121121
return NextResponse.json({ error: message }, { status: 403 })
122122
}
123123
}
@@ -148,7 +148,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
148148

149149
return NextResponse.json(result.token, { status: 200 })
150150
} catch (error) {
151-
logger.error(`[${requestId}] Error getting access token`, error)
151+
logger.error('Error getting access token', error)
152152
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
153153
}
154154
})
@@ -166,7 +166,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
166166
{},
167167
{
168168
validationErrorResponse: (error) => {
169-
logger.warn(`[${requestId}] Invalid query parameters`, { errors: error.issues })
169+
logger.warn('Invalid query parameters', { errors: error.issues })
170170
return NextResponse.json(
171171
{ error: getValidationErrorMessage(error, 'Validation failed') },
172172
{ status: 400 }
@@ -203,7 +203,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
203203
}
204204

205205
if (!credential.accessToken) {
206-
logger.warn(`[${requestId}] No access token available for credential`)
206+
logger.warn('No access token available for credential')
207207
return NextResponse.json({ error: 'No access token available' }, { status: 400 })
208208
}
209209

@@ -225,7 +225,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
225225

226226
return NextResponse.json(result.token, { status: 200 })
227227
} catch (error) {
228-
logger.error(`[${requestId}] Error fetching access token`, error)
228+
logger.error('Error fetching access token', error)
229229
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
230230
}
231231
})

‎apps/sim/app/api/billing/update-cost/route.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,12 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
196196
const startTime = Date.now()
197197

198198
try {
199-
logger.info(`[${requestId}] Update cost request started`)
199+
logger.info('Update cost request started')
200200

201201
// Check authentication (internal API key)
202202
const authResult = checkInternalApiKey(req)
203203
if (!authResult.success) {
204-
logger.warn(`[${requestId}] Authentication failed: ${authResult.error}`)
204+
logger.warn(`Authentication failed: ${authResult.error}`)
205205
span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.AuthFailed)
206206
span.setAttribute(TraceAttr.HttpStatusCode, 401)
207207
return NextResponse.json(
@@ -234,7 +234,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
234234
{},
235235
{
236236
validationErrorResponse: (error) => {
237-
logger.warn(`[${requestId}] Invalid request body`, {
237+
logger.warn('Invalid request body', {
238238
errors: error.issues,
239239
})
240240
span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.InvalidBody)
@@ -324,7 +324,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
324324
...(idempotencyKey ? { [TraceAttr.BillingIdempotencyKey]: idempotencyKey } : {}),
325325
})
326326

327-
logger.info(`[${requestId}] Processing cost update`, {
327+
logger.info('Processing cost update', {
328328
userId,
329329
cost,
330330
model,
@@ -408,7 +408,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
408408
}
409409
: undefined
410410

411-
logger.info(`[${requestId}] Billing payer resolved`, {
411+
logger.info('Billing payer resolved', {
412412
userId,
413413
billingProtocol: protocol,
414414
billingResolution: getBillingResolution(isMarkerlessLegacy, billingAttribution),
@@ -441,7 +441,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
441441
...(rolloverSubscriptionId ? { payerSubscriptionId: rolloverSubscriptionId } : {}),
442442
})
443443
const billed = result.billed
444-
logger.info(`[${requestId}] Cumulative cost top-up`, {
444+
logger.info('Cumulative cost top-up', {
445445
userId,
446446
source,
447447
cumulativeCost: cost,
@@ -473,7 +473,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
473473
) {
474474
throw error
475475
}
476-
logger.info(`[${requestId}] Charge landed in a period that has since rolled over`, {
476+
logger.info('Charge landed in a period that has since rolled over', {
477477
billingPeriodEnd: result.billingPeriod.end.toISOString(),
478478
})
479479
}
@@ -491,7 +491,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
491491
// Same-or-lower cumulative than already recorded: nothing new to bill.
492492
// Reconciliation has completed, so preserve Go's established 409 outcome.
493493
if (!billed) {
494-
logger.info(`[${requestId}] Duplicate/non-increasing cumulative cost; no new charge`, {
494+
logger.info('Duplicate/non-increasing cumulative cost; no new charge', {
495495
idempotencyKey,
496496
userId,
497497
cost,
@@ -511,7 +511,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
511511
)
512512
}
513513

514-
logger.info(`[${requestId}] Cost update completed successfully`, {
514+
logger.info('Cost update completed successfully', {
515515
userId,
516516
duration,
517517
cost,
@@ -536,7 +536,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
536536

537537
const isMarkerlessLegacy = !req.headers.get(COPILOT_BILLING_PROTOCOL_HEADER)
538538
if (error instanceof CumulativeUsageContextMismatchError && !isMarkerlessLegacy) {
539-
logger.error(`[${requestId}] Billing context mismatch`, {
539+
logger.error('Billing context mismatch', {
540540
eventKey: error.eventKey,
541541
mismatchedFields: error.mismatchedFields,
542542
duration,
@@ -560,7 +560,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
560560
* the caller resends later, and the monotonic top-up applies the higher cumulative once.
561561
*/
562562
if (error instanceof CumulativeUsageInFlightError) {
563-
logger.info(`[${requestId}] Cost update already in flight for this request`, {
563+
logger.info('Cost update already in flight for this request', {
564564
eventKey: error.eventKey,
565565
duration,
566566
})
@@ -593,7 +593,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
593593

594594
/** Old markerless clients treat every 409 as a successful duplicate. */
595595
if (reconciliationOutcome && !isMarkerlessLegacy) {
596-
logger.warn(`[${requestId}] Billing callback requires reconciliation`, {
596+
logger.warn('Billing callback requires reconciliation', {
597597
code: reconciliationOutcome.code,
598598
duration,
599599
billingProtocol:
@@ -615,7 +615,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
615615
}
616616

617617
if (error instanceof ThresholdSettlementError) {
618-
logger.error(`[${requestId}] Retryable threshold settlement failure`, {
618+
logger.error('Retryable threshold settlement failure', {
619619
settlementErrorCode: error.code,
620620
retryable: error.retryable,
621621
duration,
@@ -643,7 +643,7 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
643643
// lock timeout) — Drizzle's "Failed query" wrapper alone cannot
644644
// distinguish them, which made the dead-workspace incident undiagnosable
645645
// from logs.
646-
logger.error(`[${requestId}] Cost update failed`, {
646+
logger.error('Cost update failed', {
647647
error: toError(error).message,
648648
...(pgCode && { pgCode }),
649649
...(pgConstraint && { pgConstraint }),

0 commit comments

Comments
 (0)