Skip to content

Commit 802e613

Browse files
committed
fix(webapp,redis-worker): stop logging raw metadata, alert payloads, and job items
Debug logging around run metadata writes previously included the full, unfiltered metadata object on every flush. It now logs key/operation counts and byte sizes instead, and skips the log entirely when there is nothing buffered. The webapp logger's redaction list also now covers `metadata` and `seedMetadata` as a backstop. Alert webhook delivery failures no longer log the outgoing request body or the (useless, non-serializable) HMAC signature; they log the response status, the webhook URL host, and the relevant ids instead. The redis-worker's failure, retry, and dead-letter logs no longer include the raw job item. The item is still retrievable by id when needed, and the default worker logger now filters the `item` key as a backstop.
1 parent ec562c0 commit 802e613

4 files changed

Lines changed: 41 additions & 22 deletions

File tree

apps/webapp/app/services/logger.server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ function flattenArgs(args: Array<Record<string, unknown> | undefined>) {
5050
export const logger = new Logger(
5151
"webapp",
5252
(process.env.APP_LOG_LEVEL ?? "info") as LogLevel,
53-
["examples", "output", "connectionString", "payload"],
53+
["examples", "output", "connectionString", "payload", "metadata", "seedMetadata"],
5454
sensitiveDataReplacer,
5555
() => {
5656
const fields = currentFieldsStore.getStore();

apps/webapp/app/services/metadata/updateMetadata.server.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,14 @@ export class UpdateMetadataService {
9191
this._bufferedOperations.clear();
9292

9393
yield* Effect.sync(() => {
94-
if (this.flushLoggingEnabled) {
94+
if (this.flushLoggingEnabled && currentOperations.size > 0) {
95+
const operationCount = Array.from(currentOperations.values()).reduce(
96+
(sum, ops) => sum + ops.length,
97+
0
98+
);
9599
this.logger.debug(`[UpdateMetadataService] Flushing operations`, {
96-
operations: Object.fromEntries(currentOperations),
100+
runCount: currentOperations.size,
101+
operationCount,
97102
});
98103
}
99104
});
@@ -520,9 +525,9 @@ export class UpdateMetadataService {
520525

521526
if (this.flushLoggingEnabled) {
522527
this.logger.debug(`[updateRunMetadataWithOperations] Updated metadata for run`, {
523-
metadata: applyResults.newMetadata,
524-
operations: operations,
525528
runId,
529+
metadataKeyCount: Object.keys(applyResults.newMetadata).length,
530+
operationCount: operations.length,
526531
});
527532
}
528533

@@ -567,8 +572,8 @@ export class UpdateMetadataService {
567572
) {
568573
if (this.flushLoggingEnabled) {
569574
this.logger.debug(`[updateRunMetadataDirectly] Updating metadata directly for run`, {
570-
metadata: metadataPacket.data,
571575
runId,
576+
metadataSizeBytes: metadataPacket.data?.length ?? 0,
572577
});
573578
}
574579

@@ -607,7 +612,7 @@ export class UpdateMetadataService {
607612
if (this.flushLoggingEnabled) {
608613
this.logger.debug(`[ingestRunOperations] Ingesting operations for run`, {
609614
runId,
610-
bufferedOperations,
615+
operationCount: bufferedOperations.length,
611616
});
612617
}
613618

apps/webapp/app/v3/services/alerts/deliverAlert.server.ts

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ export class DeliverAlertService extends BaseService {
455455
error,
456456
};
457457

458-
await this.#deliverWebhook(payload, webhookProperties.data);
458+
await this.#deliverWebhook(payload, webhookProperties.data, { webhookId: alert.channel.id, runId: alert.taskRun.friendlyId });
459459
break;
460460
}
461461
case "v2": {
@@ -516,7 +516,7 @@ export class DeliverAlertService extends BaseService {
516516
},
517517
};
518518

519-
await this.#deliverWebhook(payload, webhookProperties.data);
519+
await this.#deliverWebhook(payload, webhookProperties.data, { webhookId: alert.channel.id, runId: alert.taskRun.friendlyId });
520520

521521
break;
522522
}
@@ -577,7 +577,7 @@ export class DeliverAlertService extends BaseService {
577577
vercel: this.#buildWebhookVercelObject(deploymentMeta.vercelDeploymentUrl),
578578
};
579579

580-
await this.#deliverWebhook(payload, webhookProperties.data);
580+
await this.#deliverWebhook(payload, webhookProperties.data, { webhookId: alert.channel.id });
581581
break;
582582
}
583583
case "v2": {
@@ -616,7 +616,7 @@ export class DeliverAlertService extends BaseService {
616616
},
617617
};
618618

619-
await this.#deliverWebhook(payload, webhookProperties.data);
619+
await this.#deliverWebhook(payload, webhookProperties.data, { webhookId: alert.channel.id });
620620

621621
break;
622622
}
@@ -671,7 +671,7 @@ export class DeliverAlertService extends BaseService {
671671
vercel: this.#buildWebhookVercelObject(deploymentMeta.vercelDeploymentUrl),
672672
};
673673

674-
await this.#deliverWebhook(payload, webhookProperties.data);
674+
await this.#deliverWebhook(payload, webhookProperties.data, { webhookId: alert.channel.id });
675675
break;
676676
}
677677
case "v2": {
@@ -716,7 +716,7 @@ export class DeliverAlertService extends BaseService {
716716
},
717717
};
718718

719-
await this.#deliverWebhook(payload, webhookProperties.data);
719+
await this.#deliverWebhook(payload, webhookProperties.data, { webhookId: alert.channel.id });
720720

721721
break;
722722
}
@@ -1017,7 +1017,11 @@ export class DeliverAlertService extends BaseService {
10171017
}
10181018
}
10191019

1020-
async #deliverWebhook<T>(payload: T, webhook: ProjectAlertWebhookProperties) {
1020+
async #deliverWebhook<T>(
1021+
payload: T,
1022+
webhook: ProjectAlertWebhookProperties,
1023+
context: { webhookId: string; runId?: string }
1024+
) {
10211025
const rawPayload = JSON.stringify(payload);
10221026
const hashPayload = Buffer.from(rawPayload, "utf-8");
10231027

@@ -1046,12 +1050,14 @@ export class DeliverAlertService extends BaseService {
10461050
});
10471051

10481052
if (!response.ok) {
1053+
// Never log the request/response body here: it is customer-controlled alert
1054+
// content and may include stack traces or other application data.
10491055
logger.info("[DeliverAlert] Failed to send alert webhook", {
10501056
status: response.status,
10511057
statusText: response.statusText,
1052-
url: webhook.url,
1053-
body: payload,
1054-
signature,
1058+
urlHost: safeUrlHost(webhook.url),
1059+
webhookId: context.webhookId,
1060+
runId: context.runId,
10551061
});
10561062

10571063
throw new Error(`Failed to send alert webhook to ${webhook.url}`);
@@ -1435,3 +1441,11 @@ function isWebAPIHTTPError(error: unknown): error is WebAPIHTTPError {
14351441
function isWebAPIRateLimitedError(error: unknown): error is WebAPIRateLimitedError {
14361442
return (error as WebAPIRateLimitedError).code === ErrorCode.RateLimitedError;
14371443
}
1444+
1445+
function safeUrlHost(url: string): string {
1446+
try {
1447+
return new URL(url).host;
1448+
} catch {
1449+
return "unknown";
1450+
}
1451+
}

packages/redis-worker/src/worker.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class Worker<TCatalog extends WorkerCatalog> {
140140
> = new Map();
141141

142142
constructor(private options: WorkerOptions<TCatalog>) {
143-
this.logger = options.logger ?? new Logger("Worker", "debug");
143+
this.logger = options.logger ?? new Logger("Worker", "debug", ["item"]);
144144
this.tracer = options.tracer ?? trace.getTracer(options.name);
145145
this.meter = options.meter ?? metrics.getMeter(options.name);
146146

@@ -608,7 +608,8 @@ class Worker<TCatalog extends WorkerCatalog> {
608608
this.logger.error("Unhandled error in processItem:", {
609609
error: err,
610610
workerId,
611-
item,
611+
id: queueItem.id,
612+
job: queueItem.job,
612613
});
613614
}
614615
);
@@ -933,11 +934,12 @@ class Worker<TCatalog extends WorkerCatalog> {
933934
const errorLogLevel =
934935
error && typeof error === "object" && "logLevel" in error ? error.logLevel : undefined;
935936

937+
// Never include the raw item/payload here: it is job data that may be
938+
// customer-controlled. It is retrievable via `getJob(id)` if needed for triage.
936939
const logAttributes = {
937940
name: this.options.name,
938941
id,
939942
job,
940-
item,
941943
visibilityTimeoutMs,
942944
error,
943945
errorMessage,
@@ -994,7 +996,6 @@ class Worker<TCatalog extends WorkerCatalog> {
994996
name: this.options.name,
995997
id,
996998
job,
997-
item,
998999
retryDate,
9991000
retryDelay,
10001001
visibilityTimeoutMs,
@@ -1015,7 +1016,6 @@ class Worker<TCatalog extends WorkerCatalog> {
10151016
name: this.options.name,
10161017
id,
10171018
job,
1018-
item,
10191019
visibilityTimeoutMs,
10201020
error: requeueError,
10211021
}

0 commit comments

Comments
 (0)