Review brief

Intermediate35 min

A service-observability team is adding structured request telemetry around an existing handler. The telemetry backend is broadly accessible to operators, so the same permitted-field policy must hold on normal and exceptional paths without leaking credentials or user-supplied identifiers.

Behavioral contract

Completed events may contain only event, method, route, and status. Failure events may contain only event, method, route, and errorName; failures are always logged rather than sampled.
For a matched request, route is the framework-provided routeTemplate. If it is absent, use the fixed value unmatched; never fall back to the concrete request path.
Concrete paths, raw query strings, all request headers, bodies, error messages or stacks, and other user-supplied identifiers are prohibited in telemetry on both success and failure paths.
sampleRate is prevalidated in the inclusive range 0 through 1. An omitted value defaults to 1, exactly 0 emits no completed events, and exactly 1 emits every completed event.
The injected random function returns a value from 0 inclusive to 1 exclusive; a completion is sampled when random() < sampleRate.
The logger is non-throwing, and instrumentation must return the original response or rethrow the original handler error unchanged.

Review instructions

1Trace every field written on both completed and failed request paths.
2Anchor each finding to the narrowest useful changed line and explain its privacy or operational impact.
3Review configuration boundaries as part of the telemetry contract.
4Recommend a direction rather than writing replacement code.
5Prioritize no more than five findings.

Learning objectives

• Apply one privacy policy across success and failure telemetry
• Distinguish route templates from user input
• Review boundary configuration values

src/request-telemetry-types.tsTypeScript

@@ -0,0 +1,23 @@

1+export interface TelemetryRequest {

2+ method: string;

3+ path: string;

4+ routeTemplate?: string;

5+ queryString: string;

6+ headers: Readonly<Record<string, string | undefined>>;

7+}

9+export interface TelemetryResponse {

10+ status: number;

11+}

12+

13+export type RequestHandler = () => Promise<TelemetryResponse>;

14+

15+export interface TelemetryLogger {

16+ info(fields: Readonly<Record<string, unknown>>): void;

17+ error(fields: Readonly<Record<string, unknown>>): void;

18+}

19+

20+export interface TelemetryOptions {

21+ sampleRate?: number;

22+ random?: () => number;

23+}

Reference solution

The findings below are ordered by priority.

Findings

1. Failure events copy credential-bearing request headers into logs

Location: src/request-telemetry.ts:32-39

The error event records the complete request.headers object. Ordinary failures can therefore persist authorization tokens, cookies, API keys, and other credentials in the telemetry backend, directly violating the policy on the path where debugging access is most likely.

Remove headers from the event entirely. Build failure telemetry from the explicit permitted fields rather than attempting to redact an open-ended header map.

2. Success events record the raw query string

Location: src/request-telemetry.ts:21-28

Every sampled completion includes request.queryString. User-entered searches, email addresses, reset tokens, and other sensitive values can therefore be retained as structured telemetry even though no query field is permitted.

Omit the query string rather than trying to classify individual parameter names; the route template and response status provide the allowed request context.

3. Both event paths log concrete URLs instead of route templates

Location: src/request-telemetry.ts:21-28 and src/request-telemetry.ts:32-39

The path field comes from the concrete request path while routeTemplate is ignored. A request such as /users/customer-42/orders/order-9 leaks user-supplied identifiers and creates a distinct high-cardinality log value for each resource on both success and failure.

Emit the matched route template as route, or the fixed unmatched label when no template exists. Never fall back to the concrete path.

4. A zero sampling rate is replaced with full sampling

Location: src/request-telemetry.ts:15-16

The || 1 default treats the valid value 0 as absent. An operator who deliberately disables completion telemetry therefore gets a 100% sampling rate, increasing both privacy exposure and telemetry volume.

Default only undefined, for example with nullish coalescing, while retaining the boundary comparison random() < sampleRate.

5. Tests check log existence but not privacy fields or sampling boundaries

Location: src/request-telemetry.test.ts:11-50

The tests use identifier-free paths, empty query strings, harmless headers, and only a sampling rate of 1. They assert that a logger method ran but never inspect its fields, so query, path, and header disclosure and the zero-rate regression all remain invisible.

Assert exact event objects for sensitive success and failure inputs, verify concrete values are absent, cover unmatched routing, and test deterministic sampling at both 0 and 1.

Reasonable non-findings

Logging the HTTP method, response status, fixed event name, and error class name is permitted.
Failure events are intentionally not sampled, so always calling logger.error is correct once its fields are safe.
Rethrowing the original handler error preserves request behavior.
Math.random is an acceptable production sampler, and injecting random makes boundary tests deterministic.
A default completion sampling rate of 1 is correct only when sampleRate is omitted.