Skip to content

Commit 22a88f1

Browse files
dcramerclaude
andauthored
Overhaul logging and adopt LogTape (#562)
Replaces the previous logging implementation with LogTape for structured logging across the MCP server and Cloudflare Workers. This provides consistent JSON-formatted logs with Sentry integration for monitoring. - **Adopted LogTape**: Structured logging library with dual sinks - Console sink outputs JSON Lines format for log aggregation - Sentry sink sends logs directly to Sentry for monitoring - **Four clear logging functions**: - `logInfo()` - Routine telemetry (connection lifecycle, tool invocations) - `logWarn()` - Operational warnings (rate limits, deprecated usage) - `logError()` - Operational errors that are handled gracefully - `logIssue()` - System errors that create Sentry Issues - **Environment-based configuration**: `LOG_LEVEL` env var or `NODE_ENV` fallback (debug in development, info in production) - **HTTP request logging**: Cloudflare middleware automatically logs all requests with status and duration Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent 5bfe6eb commit 22a88f1

30 files changed

Lines changed: 910 additions & 236 deletions

‎docs/error-handling.mdc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ return agentTool({
195195
- Don't wrap API calls in try/catch unless adding value
196196
- Don't use `withApiErrorHandling` anymore (deprecated)
197197
- Don't use the old `wrapAgentToolExecute` function (use `agentTool` instead)
198-
- Don't use `logError()` for expected API errors (4xx)
198+
- Don't use `logIssue()` for expected API errors (4xx)
199199
- Don't use `captureException()` for UserInputError or ApiClientError
200200
- Don't create Sentry issues for user-facing errors
201201
- **SECURITY: NEVER pass untrusted error messages to AI agents - risk of prompt injection**
@@ -324,4 +324,4 @@ When testing tools, verify:
324324
3. 5xx errors are captured by Sentry with Event IDs
325325
4. Network errors bubble up appropriately
326326
5. UserInputErrors have clear, actionable messages
327-
6. ApiClientError in agent tools returns formatted markdown with "**Input Error**" header
327+
6. ApiClientError in agent tools returns formatted markdown with "**Input Error**" header

‎docs/logging.mdc‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
---
2+
description: Logging reference using LogTape and Sentry
3+
---
4+
# Logging Reference
5+
6+
How logging works in the Sentry MCP server using LogTape and Sentry. For tracing, spans, or metrics see @docs/monitoring.mdc.
7+
8+
## Overview
9+
10+
We use [LogTape](https://logtape.org/) for structured logging with two sinks:
11+
- **Console sink**: JSON Lines format for log aggregation
12+
- **Sentry sink**: Sends logs to Sentry for monitoring
13+
14+
Log levels are controlled by:
15+
1. `LOG_LEVEL` environment variable (e.g., `"debug"`, `"info"`, `"warning"`, `"error"`)
16+
2. Falls back to `NODE_ENV`: `"debug"` in development, `"info"` in production
17+
18+
Implementation: @packages/mcp-server/src/logging.ts
19+
20+
## Using the Log Helpers
21+
22+
### logInfo() - Routine telemetry
23+
```typescript
24+
import { logInfo } from "@sentry/mcp-server/logging";
25+
26+
logInfo("MCP server started", {
27+
loggerScope: ["server", "lifecycle"],
28+
extra: { port: 3000 }
29+
});
30+
```
31+
32+
### logWarn() - Operational warnings
33+
```typescript
34+
import { logWarn } from "@sentry/mcp-server/logging";
35+
36+
logWarn("Rate limit approaching", {
37+
loggerScope: ["api", "rate-limit"],
38+
extra: { remaining: 10, limit: 100 }
39+
});
40+
```
41+
42+
### logError() - Operational errors (no Sentry issue)
43+
```typescript
44+
import { logError } from "@sentry/mcp-server/logging";
45+
46+
logError(error, {
47+
loggerScope: ["tools", "fetch-trace"],
48+
extra: { traceId: "abc123" }
49+
});
50+
```
51+
52+
### logIssue() - System errors (creates Sentry issue)
53+
```typescript
54+
import { logIssue } from "@sentry/mcp-server/logging";
55+
56+
const eventId = logIssue(error, {
57+
loggerScope: ["oauth", "token-refresh"],
58+
contexts: {
59+
oauth: { client_id: "..." }
60+
}
61+
});
62+
```
63+
64+
## When to Use Each Helper
65+
66+
**Use `logIssue()` for:**
67+
- System errors (5xx, network failures, unexpected exceptions)
68+
- Critical failures that need investigation
69+
- Creates a Sentry Issue + emits a log entry with Event ID
70+
71+
**Use `logError()` for:**
72+
- Expected operational errors (trace fetch failed, encoding error)
73+
- Errors that are handled gracefully
74+
- Sends to Sentry Logs only (no Issue created)
75+
76+
**Use `logWarn()` for:**
77+
- Rate limit approaching
78+
- Deprecated API usage
79+
- Configuration warnings
80+
81+
**Use `logInfo()` for:**
82+
- Request lifecycle (connection established, tool invoked)
83+
- Configuration output
84+
- Routine telemetry
85+
86+
**Skip logging:**
87+
- `UserInputError` - Expected validation failures
88+
- 4xx API responses - Client errors (except 429 rate limits)
89+
- See @docs/error-handling.mdc for complete rules
90+
91+
## Log Options
92+
93+
All helpers accept:
94+
95+
```typescript
96+
interface LogOptions {
97+
loggerScope?: string | readonly string[]; // e.g., ["cloudflare", "oauth"]
98+
extra?: Record<string, unknown>; // Additional context
99+
contexts?: Record<string, Record<string, unknown>>; // Sentry contexts
100+
}
101+
```
102+
103+
`logIssue()` also accepts `attachments` for files to attach to the Sentry event.
104+
105+
## Configuration
106+
107+
**Environment Variables:**
108+
- `LOG_LEVEL` - Override log level (`"debug"`, `"info"`, `"warning"`, `"error"`)
109+
- `NODE_ENV` - Determines default level (`"development"` = debug, else info)
110+
111+
**Sinks:**
112+
- Console: JSON Lines format for log aggregation
113+
- Sentry: Sends logs to Sentry for monitoring (uses `Sentry.init` config)
114+
115+
## HTTP Request Logging
116+
117+
Cloudflare middleware logs all HTTP requests automatically:
118+
119+
```typescript
120+
// Middleware in packages/mcp-cloudflare/src/server/logging.ts
121+
app.use(createRequestLogger(["cloudflare", "http"]));
122+
123+
// Logs: {"level":"INFO","message":"GET /api/search","properties":{"status":200,"duration_ms":42}}
124+
```
125+
126+
## References
127+
128+
- Implementation: @packages/mcp-server/src/logging.ts
129+
- Error handling patterns: @docs/error-handling.mdc
130+
- Monitoring and tracing: @docs/monitoring.mdc
131+
- LogTape docs: https://logtape.org/

‎docs/monitoring.mdc‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,7 @@ Different Sentry SDKs for different environments:
1919

2020
### Error Logging
2121

22-
```typescript
23-
import { captureException, withScope } from "@sentry/core";
24-
25-
export function logError(
26-
error: Error | unknown,
27-
contexts?: Record<string, any>,
28-
attachments?: Record<string, any>
29-
): string | undefined {
30-
// Skip UserInputErrors - these are expected
31-
if (error instanceof UserInputError) {
32-
return;
33-
}
34-
35-
return withScope((scope) => {
36-
if (contexts) scope.setContext("mcp", contexts);
37-
if (attachments) {
38-
for (const [key, data] of Object.entries(attachments)) {
39-
scope.addAttachment({ data, filename: key });
40-
}
41-
}
42-
return captureException(error);
43-
});
44-
}
45-
```
22+
See `logIssue` in @packages/mcp-server/src/logging.ts (documented in @docs/logging.mdc) for the canonical way to create an Issue and structured log entry.
4623

4724
### Tracing Pattern
4825

‎packages/mcp-cloudflare/src/server/app.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,22 @@ import chatOauth from "./routes/chat-oauth";
88
import chat from "./routes/chat";
99
import search from "./routes/search";
1010
import metadata from "./routes/metadata";
11-
import { logError } from "@sentry/mcp-server/logging";
11+
import { logIssue } from "@sentry/mcp-server/logging";
12+
import { createRequestLogger } from "./logging";
1213
import mcpRoutes from "./routes/mcp";
1314

1415
const app = new Hono<{
1516
Bindings: Env;
1617
}>()
18+
.use("*", createRequestLogger())
1719
// Set user IP address from X-Real-IP header for Sentry
1820
.use("*", async (c, next) => {
19-
// Extract client IP from headers in order of preference
2021
const clientIP =
2122
c.req.header("X-Real-IP") ||
2223
c.req.header("CF-Connecting-IP") ||
2324
c.req.header("X-Forwarded-For")?.split(",")[0]?.trim();
2425

2526
if (clientIP) {
26-
// Set the user context with the correct IP address
2727
Sentry.setUser({ ip_address: clientIP });
2828
}
2929

@@ -43,12 +43,9 @@ const app = new Hono<{
4343
"*",
4444
csrf({
4545
origin: (origin, c) => {
46-
// If no Origin header is present, this cannot be a CSRF attack
47-
// (CSRF requires a cross-origin request which always has an Origin header)
4846
if (!origin) {
4947
return true;
5048
}
51-
// If Origin is present, verify it matches the request URL's origin
5249
const requestUrl = new URL(c.req.url);
5350
return origin === requestUrl.origin;
5451
},
@@ -78,7 +75,7 @@ const app = new Hono<{
7875

7976
// TODO: propagate the error as sentry isnt injecting into hono
8077
app.onError((err, c) => {
81-
logError(err);
78+
logIssue(err);
8279
return c.text("Internal Server Error", 500);
8380
});
8481

‎packages/mcp-cloudflare/src/server/lib/approval-dialog.ts‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type {
22
AuthRequest,
33
ClientInfo,
44
} from "@cloudflare/workers-oauth-provider";
5-
import { logError } from "@sentry/mcp-server/logging";
5+
import { logError, logIssue, logWarn } from "@sentry/mcp-server/logging";
66
import { sanitizeHtml } from "./html-utils";
77

88
const COOKIE_NAME = "mcp-approved-clients";
@@ -71,9 +71,13 @@ async function verifySignature(
7171
signatureBytes.buffer,
7272
enc.encode(data),
7373
);
74-
} catch (e) {
75-
// Handle errors during hex parsing or verification
76-
console.error("Error verifying signature:", e);
74+
} catch (error) {
75+
logError(error, {
76+
loggerScope: ["cloudflare", "approval-dialog"],
77+
extra: {
78+
message: "Error verifying signature",
79+
},
80+
});
7781
return false;
7882
}
7983
}
@@ -99,7 +103,9 @@ async function getApprovedClientsFromCookie(
99103
const parts = cookieValue.split(".");
100104

101105
if (parts.length !== 2) {
102-
console.warn("Invalid cookie format received.");
106+
logWarn("Invalid approval cookie format", {
107+
loggerScope: ["cloudflare", "approval-dialog"],
108+
});
103109
return null; // Invalid format
104110
}
105111

@@ -110,24 +116,30 @@ async function getApprovedClientsFromCookie(
110116
const isValid = await verifySignature(key, signatureHex, payload);
111117

112118
if (!isValid) {
113-
console.warn("Cookie signature verification failed.");
119+
logWarn("Approval cookie signature verification failed", {
120+
loggerScope: ["cloudflare", "approval-dialog"],
121+
});
114122
return null; // Signature invalid
115123
}
116124

117125
try {
118126
const approvedClients = JSON.parse(payload);
119127
if (!Array.isArray(approvedClients)) {
120-
console.warn("Cookie payload is not an array.");
128+
logWarn("Approval cookie payload is not an array", {
129+
loggerScope: ["cloudflare", "approval-dialog"],
130+
});
121131
return null; // Payload isn't an array
122132
}
123133
// Ensure all elements are strings
124134
if (!approvedClients.every((item) => typeof item === "string")) {
125-
console.warn("Cookie payload contains non-string elements.");
135+
logWarn("Approval cookie payload contains non-string elements", {
136+
loggerScope: ["cloudflare", "approval-dialog"],
137+
});
126138
return null;
127139
}
128140
return approvedClients as string[];
129141
} catch (e) {
130-
logError(new Error(`Error parsing cookie payload: ${e}`, { cause: e }));
142+
logIssue(new Error(`Error parsing cookie payload: ${e}`, { cause: e }));
131143
return null; // JSON parsing failed
132144
}
133145
}
@@ -190,8 +202,13 @@ function encodeState(data: any): string {
190202
// Use btoa for simplicity, assuming Worker environment supports it well enough
191203
// For complex binary data, a Buffer/Uint8Array approach might be better
192204
return btoa(jsonString);
193-
} catch (e) {
194-
console.error("Error encoding state:", e);
205+
} catch (error) {
206+
logError(error, {
207+
loggerScope: ["cloudflare", "approval-dialog"],
208+
extra: {
209+
message: "Error encoding approval dialog state",
210+
},
211+
});
195212
throw new Error("Could not encode state");
196213
}
197214
}
@@ -205,8 +222,13 @@ function decodeState<T = any>(encoded: string): T {
205222
try {
206223
const jsonString = atob(encoded);
207224
return JSON.parse(jsonString);
208-
} catch (e) {
209-
console.error("Error decoding state:", e);
225+
} catch (error) {
226+
logError(error, {
227+
loggerScope: ["cloudflare", "approval-dialog"],
228+
extra: {
229+
message: "Error decoding approval dialog state",
230+
},
231+
});
210232
throw new Error("Could not decode state");
211233
}
212234
}
@@ -773,11 +795,15 @@ export async function parseRedirectApproval(
773795
permissions = formData
774796
.getAll("permission")
775797
.filter((p): p is string => typeof p === "string");
776-
} catch (e) {
777-
console.error("Error processing form submission:", e);
778-
// Rethrow or handle as appropriate, maybe return a specific error response
798+
} catch (error) {
799+
logError(error, {
800+
loggerScope: ["cloudflare", "approval-dialog"],
801+
extra: {
802+
message: "Error processing approval form submission",
803+
},
804+
});
779805
throw new Error(
780-
`Failed to parse approval form: ${e instanceof Error ? e.message : String(e)}`,
806+
`Failed to parse approval form: ${error instanceof Error ? error.message : String(error)}`,
781807
);
782808
}
783809

‎packages/mcp-cloudflare/src/server/lib/constraint-utils.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Constraints } from "@sentry/mcp-server/types";
22
import { SentryApiService, ApiError } from "@sentry/mcp-server/api-client";
3-
import { logError } from "@sentry/mcp-server/logging";
3+
import { logIssue } from "@sentry/mcp-server/logging";
44

55
/**
66
* Verify that provided org/project constraints exist and the user has access
@@ -58,7 +58,7 @@ export async function verifyConstraintsAccess(
5858
: error.message;
5959
return { ok: false, status: error.status, message };
6060
}
61-
const eventId = logError(error);
61+
const eventId = logIssue(error);
6262
return {
6363
ok: false,
6464
status: 502,
@@ -85,7 +85,7 @@ export async function verifyConstraintsAccess(
8585
: error.message;
8686
return { ok: false, status: error.status, message };
8787
}
88-
const eventId = logError(error);
88+
const eventId = logIssue(error);
8989
return {
9090
ok: false,
9191
status: 502,

0 commit comments

Comments
 (0)