Skip to content

Commit 53ac631

Browse files
nirinchevnirinchev
authored
chore: allow logging unredacted messages MCP-103 (#420)
1 parent a35d18d commit 53ac631

File tree

18 files changed

+499
-185
lines changed

18 files changed

+499
-185
lines changed

‎src/common/atlas/accessListUtils.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,25 @@ export async function ensureCurrentIpInAccessList(apiClient: ApiClient, projectI
3030
params: { path: { groupId: projectId } },
3131
body: [entry],
3232
});
33-
logger.debug(
34-
LogId.atlasIpAccessListAdded,
35-
"accessListUtils",
36-
`IP access list created: ${JSON.stringify(entry)}`
37-
);
33+
logger.debug({
34+
id: LogId.atlasIpAccessListAdded,
35+
context: "accessListUtils",
36+
message: `IP access list created: ${JSON.stringify(entry)}`,
37+
});
3838
} catch (err) {
3939
if (err instanceof ApiClientError && err.response?.status === 409) {
4040
// 409 Conflict: entry already exists, log info
41-
logger.debug(
42-
LogId.atlasIpAccessListAdded,
43-
"accessListUtils",
44-
`IP address ${entry.ipAddress} is already present in the access list for project ${projectId}.`
45-
);
41+
logger.debug({
42+
id: LogId.atlasIpAccessListAdded,
43+
context: "accessListUtils",
44+
message: `IP address ${entry.ipAddress} is already present in the access list for project ${projectId}.`,
45+
});
4646
return;
4747
}
48-
logger.warning(
49-
LogId.atlasIpAccessListAddFailure,
50-
"accessListUtils",
51-
`Error adding IP access list: ${err instanceof Error ? err.message : String(err)}`
52-
);
48+
logger.warning({
49+
id: LogId.atlasIpAccessListAddFailure,
50+
context: "accessListUtils",
51+
message: `Error adding IP access list: ${err instanceof Error ? err.message : String(err)}`,
52+
});
5353
}
5454
}

‎src/common/atlas/apiClient.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,11 @@ export class ApiClient {
180180
};
181181
} catch (error: unknown) {
182182
const err = error instanceof Error ? error : new Error(String(error));
183-
logger.error(LogId.atlasConnectFailure, "apiClient", `Failed to request access token: ${err.message}`);
183+
logger.error({
184+
id: LogId.atlasConnectFailure,
185+
context: "apiClient",
186+
message: `Failed to request access token: ${err.message}`,
187+
});
184188
}
185189
return this.accessToken;
186190
}
@@ -200,7 +204,11 @@ export class ApiClient {
200204
}
201205
} catch (error: unknown) {
202206
const err = error instanceof Error ? error : new Error(String(error));
203-
logger.error(LogId.atlasApiRevokeFailure, "apiClient", `Failed to revoke access token: ${err.message}`);
207+
logger.error({
208+
id: LogId.atlasApiRevokeFailure,
209+
context: "apiClient",
210+
message: `Failed to revoke access token: ${err.message}`,
211+
});
204212
}
205213
this.accessToken = undefined;
206214
}

‎src/common/atlas/cluster.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ export async function inspectCluster(apiClient: ApiClient, projectId: string, cl
8787
return formatFlexCluster(cluster);
8888
} catch (flexError) {
8989
const err = flexError instanceof Error ? flexError : new Error(String(flexError));
90-
logger.error(LogId.atlasInspectFailure, "inspect-cluster", `error inspecting cluster: ${err.message}`);
90+
logger.error({
91+
id: LogId.atlasInspectFailure,
92+
context: "inspect-cluster",
93+
message: `error inspecting cluster: ${err.message}`,
94+
});
9195
throw error;
9296
}
9397
}

‎src/common/logger.ts

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,44 +50,96 @@ export const LogId = {
5050
streamableHttpTransportCloseFailure: mongoLogId(1_006_006),
5151
} as const;
5252

53+
interface LogPayload {
54+
id: MongoLogId;
55+
context: string;
56+
message: string;
57+
noRedaction?: boolean | LoggerType | LoggerType[];
58+
}
59+
60+
export type LoggerType = "console" | "disk" | "mcp";
61+
5362
export abstract class LoggerBase {
54-
abstract log(level: LogLevel, id: MongoLogId, context: string, message: string): void;
63+
private defaultUnredactedLogger: LoggerType = "mcp";
5564

56-
info(id: MongoLogId, context: string, message: string): void {
57-
this.log("info", id, context, message);
65+
public log(level: LogLevel, payload: LogPayload): void {
66+
// If no explicit value is supplied for unredacted loggers, default to "mcp"
67+
const noRedaction = payload.noRedaction !== undefined ? payload.noRedaction : this.defaultUnredactedLogger;
68+
69+
this.logCore(level, {
70+
...payload,
71+
message: this.redactIfNecessary(payload.message, noRedaction),
72+
});
5873
}
5974

60-
error(id: MongoLogId, context: string, message: string): void {
61-
this.log("error", id, context, message);
75+
protected abstract type: LoggerType;
76+
77+
protected abstract logCore(level: LogLevel, payload: LogPayload): void;
78+
79+
private redactIfNecessary(message: string, noRedaction: LogPayload["noRedaction"]): string {
80+
if (typeof noRedaction === "boolean" && noRedaction) {
81+
// If the consumer has supplied noRedaction: true, we don't redact the log message
82+
// regardless of the logger type
83+
return message;
84+
}
85+
86+
if (typeof noRedaction === "string" && noRedaction === this.type) {
87+
// If the consumer has supplied noRedaction: logger-type, we skip redacting if
88+
// our logger type is the same as what the consumer requested
89+
return message;
90+
}
91+
92+
if (
93+
typeof noRedaction === "object" &&
94+
Array.isArray(noRedaction) &&
95+
this.type !== null &&
96+
noRedaction.indexOf(this.type) !== -1
97+
) {
98+
// If the consumer has supplied noRedaction: array, we skip redacting if our logger
99+
// type is included in that array
100+
return message;
101+
}
102+
103+
return redact(message);
104+
}
105+
106+
info(payload: LogPayload): void {
107+
this.log("info", payload);
62108
}
63-
debug(id: MongoLogId, context: string, message: string): void {
64-
this.log("debug", id, context, message);
109+
110+
error(payload: LogPayload): void {
111+
this.log("error", payload);
112+
}
113+
debug(payload: LogPayload): void {
114+
this.log("debug", payload);
65115
}
66116

67-
notice(id: MongoLogId, context: string, message: string): void {
68-
this.log("notice", id, context, message);
117+
notice(payload: LogPayload): void {
118+
this.log("notice", payload);
69119
}
70120

71-
warning(id: MongoLogId, context: string, message: string): void {
72-
this.log("warning", id, context, message);
121+
warning(payload: LogPayload): void {
122+
this.log("warning", payload);
73123
}
74124

75-
critical(id: MongoLogId, context: string, message: string): void {
76-
this.log("critical", id, context, message);
125+
critical(payload: LogPayload): void {
126+
this.log("critical", payload);
77127
}
78128

79-
alert(id: MongoLogId, context: string, message: string): void {
80-
this.log("alert", id, context, message);
129+
alert(payload: LogPayload): void {
130+
this.log("alert", payload);
81131
}
82132

83-
emergency(id: MongoLogId, context: string, message: string): void {
84-
this.log("emergency", id, context, message);
133+
emergency(payload: LogPayload): void {
134+
this.log("emergency", payload);
85135
}
86136
}
87137

88138
export class ConsoleLogger extends LoggerBase {
89-
log(level: LogLevel, id: MongoLogId, context: string, message: string): void {
90-
message = redact(message);
139+
protected type: LoggerType = "console";
140+
141+
protected logCore(level: LogLevel, payload: LogPayload): void {
142+
const { id, context, message } = payload;
91143
console.error(`[${level.toUpperCase()}] ${id.__value} - ${context}: ${message} (${process.pid})`);
92144
}
93145
}
@@ -97,6 +149,8 @@ export class DiskLogger extends LoggerBase {
97149
super();
98150
}
99151

152+
protected type: LoggerType = "disk";
153+
100154
static async fromPath(logPath: string): Promise<DiskLogger> {
101155
await fs.mkdir(logPath, { recursive: true });
102156

@@ -116,8 +170,8 @@ export class DiskLogger extends LoggerBase {
116170
return new DiskLogger(logWriter);
117171
}
118172

119-
log(level: LogLevel, id: MongoLogId, context: string, message: string): void {
120-
message = redact(message);
173+
protected logCore(level: LogLevel, payload: LogPayload): void {
174+
const { id, context, message } = payload;
121175
const mongoDBLevel = this.mapToMongoDBLogLevel(level);
122176

123177
this.logWriter[mongoDBLevel]("MONGODB-MCP", id, context, message);
@@ -149,20 +203,25 @@ export class McpLogger extends LoggerBase {
149203
super();
150204
}
151205

152-
log(level: LogLevel, _: MongoLogId, context: string, message: string): void {
206+
type: LoggerType = "mcp";
207+
208+
protected logCore(level: LogLevel, payload: LogPayload): void {
153209
// Only log if the server is connected
154210
if (!this.server?.isConnected()) {
155211
return;
156212
}
157213

158214
void this.server.server.sendLoggingMessage({
159215
level,
160-
data: `[${context}]: ${message}`,
216+
data: `[${payload.context}]: ${payload.message}`,
161217
});
162218
}
163219
}
164220

165-
class CompositeLogger extends LoggerBase {
221+
export class CompositeLogger extends LoggerBase {
222+
// This is not a real logger type - it should not be used anyway.
223+
protected type: LoggerType = "composite" as unknown as LoggerType;
224+
166225
private loggers: LoggerBase[] = [];
167226

168227
constructor(...loggers: LoggerBase[]) {
@@ -178,11 +237,16 @@ class CompositeLogger extends LoggerBase {
178237
this.loggers = [...loggers];
179238
}
180239

181-
log(level: LogLevel, id: MongoLogId, context: string, message: string): void {
240+
public log(level: LogLevel, payload: LogPayload): void {
241+
// Override the public method to avoid the base logger redacting the message payload
182242
for (const logger of this.loggers) {
183-
logger.log(level, id, context, message);
243+
logger.log(level, payload);
184244
}
185245
}
246+
247+
protected logCore(): void {
248+
throw new Error("logCore should never be invoked on CompositeLogger");
249+
}
186250
}
187251

188252
const logger = new CompositeLogger(new ConsoleLogger());

‎src/common/session.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,11 @@ export class Session extends EventEmitter<SessionEvents> {
7070
await this.connectionManager.disconnect();
7171
} catch (err: unknown) {
7272
const error = err instanceof Error ? err : new Error(String(err));
73-
logger.error(LogId.mongodbDisconnectFailure, "Error closing service provider:", error.message);
73+
logger.error({
74+
id: LogId.mongodbDisconnectFailure,
75+
context: "session",
76+
message: `Error closing service provider: ${error.message}`,
77+
});
7478
}
7579

7680
if (atlasCluster?.username && atlasCluster?.projectId) {
@@ -86,11 +90,11 @@ export class Session extends EventEmitter<SessionEvents> {
8690
})
8791
.catch((err: unknown) => {
8892
const error = err instanceof Error ? err : new Error(String(err));
89-
logger.error(
90-
LogId.atlasDeleteDatabaseUserFailure,
91-
"atlas-connect-cluster",
92-
`Error deleting previous database user: ${error.message}`
93-
);
93+
logger.error({
94+
id: LogId.atlasDeleteDatabaseUserFailure,
95+
context: "session",
96+
message: `Error deleting previous database user: ${error.message}`,
97+
});
9498
});
9599
}
96100
}

‎src/common/sessionStore.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,18 @@ export class SessionStore {
4747
private sendNotification(sessionId: string): void {
4848
const session = this.sessions[sessionId];
4949
if (!session) {
50-
logger.warning(
51-
LogId.streamableHttpTransportSessionCloseNotificationFailure,
52-
"sessionStore",
53-
`session ${sessionId} not found, no notification delivered`
54-
);
50+
logger.warning({
51+
id: LogId.streamableHttpTransportSessionCloseNotificationFailure,
52+
context: "sessionStore",
53+
message: `session ${sessionId} not found, no notification delivered`,
54+
});
5555
return;
5656
}
57-
session.logger.info(
58-
LogId.streamableHttpTransportSessionCloseNotification,
59-
"sessionStore",
60-
"Session is about to be closed due to inactivity"
61-
);
57+
session.logger.info({
58+
id: LogId.streamableHttpTransportSessionCloseNotification,
59+
context: "sessionStore",
60+
message: "Session is about to be closed due to inactivity",
61+
});
6262
}
6363

6464
setSession(sessionId: string, transport: StreamableHTTPServerTransport, mcpServer: McpServer): void {
@@ -68,11 +68,11 @@ export class SessionStore {
6868
}
6969
const abortTimeout = setManagedTimeout(async () => {
7070
if (this.sessions[sessionId]) {
71-
this.sessions[sessionId].logger.info(
72-
LogId.streamableHttpTransportSessionCloseNotification,
73-
"sessionStore",
74-
"Session closed due to inactivity"
75-
);
71+
this.sessions[sessionId].logger.info({
72+
id: LogId.streamableHttpTransportSessionCloseNotification,
73+
context: "sessionStore",
74+
message: "Session closed due to inactivity",
75+
});
7676

7777
await this.closeSession(sessionId);
7878
}
@@ -95,11 +95,11 @@ export class SessionStore {
9595
try {
9696
await session.transport.close();
9797
} catch (error) {
98-
logger.error(
99-
LogId.streamableHttpTransportSessionCloseFailure,
100-
"streamableHttpTransport",
101-
`Error closing transport ${sessionId}: ${error instanceof Error ? error.message : String(error)}`
102-
);
98+
logger.error({
99+
id: LogId.streamableHttpTransportSessionCloseFailure,
100+
context: "streamableHttpTransport",
101+
message: `Error closing transport ${sessionId}: ${error instanceof Error ? error.message : String(error)}`,
102+
});
103103
}
104104
}
105105
delete this.sessions[sessionId];

0 commit comments

Comments
 (0)