-
Notifications
You must be signed in to change notification settings - Fork 48
feat: Upgrade JS SDK to 9.16.1
and add durable objects instrumentation
#165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
a353740
2ec9864
67fc350
7c3e2fa
d033bdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,29 @@ | ||
import * as Sentry from "@sentry/cloudflare"; | ||
import { McpAgent } from "agents/mcp"; | ||
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; | ||
import { configureServer } from "@sentry/mcp-server/server"; | ||
import type { Env, WorkerProps } from "../types"; | ||
import { flush } from "@sentry/cloudflare"; | ||
import { wrapMcpServerWithSentry } from "@sentry/core"; | ||
import { LIB_VERSION } from "@sentry/mcp-server/version"; | ||
|
||
// Context from the auth process, encrypted & stored in the auth token | ||
// and provided to the DurableMCP as this.props | ||
export default class SentryMCP extends McpAgent<Env, unknown, WorkerProps> { | ||
server = wrapMcpServerWithSentry( | ||
new McpServer({ | ||
name: "Sentry MCP", | ||
version: LIB_VERSION, | ||
}), | ||
); | ||
class SentryMCPBase extends McpAgent<Env, unknown, WorkerProps> { | ||
server = new McpServer({ | ||
name: "Sentry MCP", | ||
version: LIB_VERSION, | ||
}); | ||
// Note: This does not work locally with miniflare so we are not using it | ||
// server = wrapMcpServerWithSentry( | ||
// new McpServer({ | ||
// name: "Sentry MCP", | ||
// version: LIB_VERSION, | ||
// }), | ||
// ); | ||
|
||
// biome-ignore lint/complexity/noUselessConstructor: Need the constructor to match the durable object types. | ||
constructor(state: DurableObjectState, env: Env) { | ||
super(state, env); | ||
} | ||
|
||
async init() { | ||
await configureServer({ | ||
|
@@ -25,8 +34,27 @@ export default class SentryMCP extends McpAgent<Env, unknown, WorkerProps> { | |
userId: this.props.id, | ||
}, | ||
onToolComplete: () => { | ||
this.ctx.waitUntil(flush(2000)); | ||
this.ctx.waitUntil(Sentry.flush(2000)); | ||
}, | ||
}); | ||
} | ||
} | ||
|
||
export default Sentry.instrumentDurableObjectWithSentry( | ||
(env) => ({ | ||
dsn: env.SENTRY_DSN, | ||
tracesSampleRate: 1, | ||
sendDefaultPii: true, | ||
initialScope: { | ||
tags: { | ||
durable_object: true, | ||
mcp_server_version: LIB_VERSION, | ||
}, | ||
}, | ||
_experiments: { | ||
enableLogs: true, | ||
}, | ||
integrations: [Sentry.consoleLoggingIntegration()], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this make console.error go to sentry as a new issue? if so prob gonna remove it as i prefer the more explicit abstraction There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right now it only records it as a Once the SDK is not experimental, we'll merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah thats what i want (I dont ever want console to create issues, generally speaking) maybe sometimes id want things showing up as logs to be issues, but by default logger integrations are noisey There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yeah we're debating between just a boolean option to forward everything, or some callback that allows you to create issues on log message. We'll have this implemented by GA. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. id just have the server handle it - no reason we cant have a thing that says "turn logs that look like X into issues" |
||
}), | ||
SentryMCPBase, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { captureException, captureMessage, withScope } from "@sentry/core"; | ||
import { captureException, captureMessage, withScope } from "@sentry/node"; | ||
|
||
|
||
export function logError( | ||
error: Error | unknown, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we actually n eed this flush at this point? still no idea why it didnt do anything before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't need this flush now that we have durable objects instrumentation. We can keep it around for now, let me test quickly to make sure everything is okay - I'll open another PR in a sec.