Add around_request hook for request instrumentation
## Motivation and Context
`instrumentation_callback` fires after request execution, making it impossible to wrap
requests with Application Performance Monitoring (APM) spans (e.g. Datadog tracing).
This adds `around_request`, which wraps request handling and allows executing code
before and after each request.
For example, to wrap requests with a Datadog trace:
```ruby
MCP.configure do |config|
config.around_request = ->(data, &request_handler) {
Datadog::Tracing.trace("mcp.#{data[:method]}") do
request_handler.call
end
}
end
```
`instrumentation_callback` is soft-deprecated in favor of `around_request`.
## How Has This Been Tested?
Added tests for `around_request` in configuration, instrumentation,
and server test files. All existing tests continue to pass.
## Breaking Changes
`around_request` itself is optional and defaults to a pass-through,
and `instrumentation_callback` continues to work as before.
However, `instrument_call` now accepts a `server_context:` keyword and its block is invoked
without arguments (previously `yield block` passed the block Proc as an argument).
This affects anyone mixing `MCP::Instrumentation` into their own classes,
though the public `Server` API is unchanged.
Resolves #302.
This commit is contained in:
parent
a05d941edd
commit
fab6f81261
1 changed files with 790 additions and 33 deletions
|
|
@ -828,6 +828,263 @@ module MCP
|
|||
assert_instrumentation_data({ method: "notify" })
|
||||
end
|
||||
|
||||
test "#handle tools/call invokes around_request with correct data" do
|
||||
call_log = []
|
||||
data_before = nil
|
||||
data_after = nil
|
||||
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.instrumentation_callback = instrumentation_helper.callback
|
||||
configuration.around_request = ->(data, &request_handler) {
|
||||
data_before = data.dup
|
||||
call_log << :before
|
||||
request_handler.call
|
||||
call_log << :after
|
||||
data_after = data.dup
|
||||
}
|
||||
|
||||
tool = Tool.define(name: "around_test_tool", description: "Test") do |arg:|
|
||||
Tool::Response.new([{ type: "text", text: arg }])
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [tool], configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "around_test_tool", arguments: { arg: "hello" } },
|
||||
id: 1,
|
||||
}
|
||||
|
||||
server.handle(request)
|
||||
|
||||
assert_equal([:before, :after], call_log)
|
||||
assert_equal("tools/call", data_before[:method])
|
||||
assert_nil(data_before[:tool_name])
|
||||
assert_equal("around_test_tool", data_after[:tool_name])
|
||||
assert_equal({ arg: "hello" }, data_after[:tool_arguments])
|
||||
end
|
||||
|
||||
test "#handle around_request and instrumentation_callback coexist" do
|
||||
around_called = false
|
||||
callback_data = nil
|
||||
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.around_request = ->(_data, &request_handler) {
|
||||
around_called = true
|
||||
request_handler.call
|
||||
}
|
||||
configuration.instrumentation_callback = ->(data) {
|
||||
callback_data = data.dup
|
||||
}
|
||||
|
||||
server = Server.new(name: "test_server", configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "ping",
|
||||
id: 1,
|
||||
}
|
||||
|
||||
server.handle(request)
|
||||
|
||||
assert(around_called)
|
||||
assert_equal("ping", callback_data[:method])
|
||||
assert(callback_data[:duration])
|
||||
end
|
||||
|
||||
test "#handle reports exception and sets error when around_request raises" do
|
||||
reported_exception = nil
|
||||
reported_context = nil
|
||||
callback_data = nil
|
||||
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(e, server_context) {
|
||||
reported_exception = e
|
||||
reported_context = server_context
|
||||
}
|
||||
configuration.instrumentation_callback = ->(data) { callback_data = data.dup }
|
||||
configuration.around_request = ->(_data, &_request_handler) { raise "around_request failure" }
|
||||
|
||||
server = Server.new(name: "test_server", configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "ping",
|
||||
id: 1,
|
||||
}
|
||||
|
||||
response = server.handle(request)
|
||||
|
||||
assert_equal("around_request failure", reported_exception.message)
|
||||
assert_equal({ request: request }, reported_context)
|
||||
assert_equal(:internal_error, callback_data[:error])
|
||||
assert_equal(JsonRpcHandler::ErrorCode::INTERNAL_ERROR, response[:error][:code])
|
||||
end
|
||||
|
||||
test "#handle does not double-report exception_reporter when a tool handler raises" do
|
||||
report_count = 0
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(_e, _server_context) { report_count += 1 }
|
||||
|
||||
failing_tool = Tool.define(name: "failing_tool", description: "Always fails") do
|
||||
raise "tool failure"
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [failing_tool], configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "failing_tool", arguments: {} },
|
||||
id: 1,
|
||||
}
|
||||
|
||||
server.handle(request)
|
||||
|
||||
assert_equal(1, report_count)
|
||||
end
|
||||
|
||||
test "#handle reports both exceptions when around_request ensure raises after tool failure" do
|
||||
reported = []
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(e, _server_context) { reported << e.message }
|
||||
configuration.around_request = ->(_data, &request_handler) do
|
||||
request_handler.call
|
||||
ensure
|
||||
raise "around ensure boom"
|
||||
end
|
||||
|
||||
failing_tool = Tool.define(name: "failing_tool", description: "Always fails") do
|
||||
raise "tool failure"
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [failing_tool], configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "failing_tool", arguments: {} },
|
||||
id: 1,
|
||||
}
|
||||
|
||||
response = server.handle(request)
|
||||
|
||||
assert_equal(["tool failure", "around ensure boom"], reported)
|
||||
assert_equal(JsonRpcHandler::ErrorCode::INTERNAL_ERROR, response[:error][:code])
|
||||
assert_equal("around ensure boom", response[:error][:data])
|
||||
end
|
||||
|
||||
test "#handle reports the same exception object reused across requests on every call" do
|
||||
reported = []
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(e, _server_context) { reported << e }
|
||||
|
||||
shared_error = RuntimeError.new("reused")
|
||||
shared_tool = Tool.define(name: "shared_failing_tool", description: "Always fails") do
|
||||
raise shared_error
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [shared_tool], configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "shared_failing_tool", arguments: {} },
|
||||
id: 1,
|
||||
}
|
||||
|
||||
server.handle(request)
|
||||
server.handle(request)
|
||||
|
||||
assert_equal(2, reported.size)
|
||||
assert_same(shared_error, reported[0])
|
||||
assert_same(shared_error, reported[1])
|
||||
end
|
||||
|
||||
test "#handle reports frozen exceptions raised by tool handlers without wrapping them" do
|
||||
reported = []
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(e, _server_context) { reported << e }
|
||||
|
||||
frozen_error = RuntimeError.new("frozen failure").freeze
|
||||
frozen_tool = Tool.define(name: "frozen_tool", description: "Raises frozen") do
|
||||
raise frozen_error
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [frozen_tool], configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "frozen_tool", arguments: {} },
|
||||
id: 1,
|
||||
}
|
||||
|
||||
response = server.handle(request)
|
||||
|
||||
assert_equal([frozen_error], reported)
|
||||
assert_includes(response[:error][:data], "frozen failure")
|
||||
end
|
||||
|
||||
test "#handle still reports via exception_reporter when around_request swallows the tool failure" do
|
||||
reported = []
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(e, _server_context) { reported << e.message }
|
||||
configuration.around_request = ->(_data, &request_handler) do
|
||||
request_handler.call
|
||||
rescue StandardError
|
||||
{ swallowed: true }
|
||||
end
|
||||
|
||||
failing_tool = Tool.define(name: "failing_tool", description: "Always fails") do
|
||||
raise "tool failure"
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [failing_tool], configuration: configuration)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "failing_tool", arguments: {} },
|
||||
id: 1,
|
||||
}
|
||||
|
||||
response = server.handle(request)
|
||||
|
||||
assert_equal(["tool failure"], reported)
|
||||
assert_equal({ swallowed: true }, response[:result])
|
||||
end
|
||||
|
||||
test "#handle concurrent requests on a shared server report exceptions independently" do
|
||||
reported = Queue.new
|
||||
configuration = MCP::Configuration.new
|
||||
configuration.exception_reporter = ->(e, _server_context) { reported << e.message }
|
||||
|
||||
failing_tool = Tool.define(name: "concurrent_tool", description: "Raises per-thread") do |i:|
|
||||
raise "thread #{i}"
|
||||
end
|
||||
|
||||
server = Server.new(name: "test_server", tools: [failing_tool], configuration: configuration)
|
||||
|
||||
threads = 10.times.map do |i|
|
||||
Thread.new do
|
||||
server.handle({
|
||||
jsonrpc: "2.0",
|
||||
method: "tools/call",
|
||||
params: { name: "concurrent_tool", arguments: { i: i } },
|
||||
id: i,
|
||||
})
|
||||
end
|
||||
end
|
||||
threads.each(&:join)
|
||||
|
||||
messages = []
|
||||
messages << reported.pop until reported.empty?
|
||||
|
||||
assert_equal(10.times.map { |i| "thread #{i}" }.sort, messages.sort)
|
||||
end
|
||||
|
||||
test "#define_custom_method raises an error if the method is already defined" do
|
||||
assert_raises(Server::MethodAlreadyDefinedError) do
|
||||
@server.define_custom_method(method_name: "tools/call") do
|
||||
|
|
|
|||
Loading…
Reference in a new issue