Expose the user-defined server_context in instrumentation data
APM integrations need to tag a span with the subject of the request, but the
user-defined `server_context` hash passed to `Server.new` never reaches
`around_request` or `instrumentation_callback`. The exception reporter's
`server_context` argument is a different value — it describes where a failure
occurred (`{ request: ... }`, `{ notification: ... }`), not who made the call —
so there is currently no way to read the request subject from a hook.
Add `Configuration#instrument_server_context`, off by default. When enabled,
`instrument_call` puts the host's `server_context` into the instrumentation data
before invoking `around_request`, so it is readable both before and after
`request_handler.call`.
The flag is opt-in rather than unconditional because the hash is
application-supplied and may hold values a tracing backend should not receive.
Hosts that do not expose a `server_context` reader are unaffected.
`instrument_call` reads it through `self.server_context`: the existing
`server_context:` keyword shadows the reader, and the value wanted here is the
user-defined hash, not the reporter context.
Follows the same shape as the earlier additions of `tool_arguments` (#218) and
`client` (#221) to this hash.
This commit is contained in:
parent
9a62b9ae4f
commit
14e02da475
2 changed files with 158 additions and 3 deletions
|
|
@ -145,6 +145,35 @@ module MCP
|
|||
refute merged.validate_tool_call_arguments
|
||||
end
|
||||
|
||||
test "defaults instrument_server_context to false" do
|
||||
config = Configuration.new
|
||||
|
||||
refute_predicate config, :instrument_server_context?
|
||||
end
|
||||
|
||||
test "accepts instrument_server_context in the constructor" do
|
||||
config = Configuration.new(instrument_server_context: true)
|
||||
|
||||
assert_predicate config, :instrument_server_context?
|
||||
end
|
||||
|
||||
test "raises ArgumentError when instrument_server_context is not a boolean value" do
|
||||
config = Configuration.new
|
||||
|
||||
exception = assert_raises(ArgumentError) do
|
||||
config.instrument_server_context = "true"
|
||||
end
|
||||
assert_equal("instrument_server_context must be a boolean", exception.message)
|
||||
end
|
||||
|
||||
test "merge carries instrument_server_context from the other configuration" do
|
||||
base = Configuration.new(instrument_server_context: true)
|
||||
other = Configuration.new(instrument_server_context: false)
|
||||
|
||||
refute_predicate base.merge(other), :instrument_server_context?
|
||||
assert_predicate other.merge(base), :instrument_server_context?
|
||||
end
|
||||
|
||||
test "defaults validate_tool_call_results to false" do
|
||||
config = Configuration.new
|
||||
refute config.validate_tool_call_results
|
||||
|
|
|
|||
|
|
@ -40,6 +40,79 @@ module MCP
|
|||
end
|
||||
end
|
||||
|
||||
# Mirrors `MCP::Server`, which exposes the user-defined hash through
|
||||
# `attr_accessor :server_context`.
|
||||
class SubjectWithServerContext < Subject
|
||||
attr_accessor :server_context
|
||||
|
||||
def initialize(server_context)
|
||||
super()
|
||||
@server_context = server_context
|
||||
end
|
||||
end
|
||||
|
||||
test "#instrument_call omits the user-defined server_context by default" do
|
||||
subject = SubjectWithServerContext.new({ user_id: 42 })
|
||||
|
||||
subject.instrumented_method
|
||||
|
||||
refute_includes subject.instrumentation_data_received.keys, :server_context
|
||||
end
|
||||
|
||||
test "#instrument_call exposes the user-defined server_context when instrument_server_context is enabled" do
|
||||
subject = SubjectWithServerContext.new({ user_id: 42 })
|
||||
subject.configuration.instrument_server_context = true
|
||||
|
||||
subject.instrumented_method
|
||||
|
||||
assert_equal({ user_id: 42 }, subject.instrumentation_data_received[:server_context])
|
||||
end
|
||||
|
||||
test "#instrument_call exposes the user-defined server_context to around_request" do
|
||||
subject = SubjectWithServerContext.new({ user_id: 42 })
|
||||
subject.configuration.instrument_server_context = true
|
||||
seen = nil
|
||||
subject.configuration.around_request = ->(data, &handler) {
|
||||
seen = data[:server_context]
|
||||
handler.call
|
||||
}
|
||||
|
||||
subject.instrumented_method
|
||||
|
||||
assert_equal({ user_id: 42 }, seen)
|
||||
end
|
||||
|
||||
test "#instrument_call reports the user-defined server_context as nil when the host does not set one" do
|
||||
subject = SubjectWithServerContext.new(nil)
|
||||
subject.configuration.instrument_server_context = true
|
||||
|
||||
subject.instrumented_method
|
||||
|
||||
assert_includes subject.instrumentation_data_received.keys, :server_context
|
||||
assert_nil subject.instrumentation_data_received[:server_context]
|
||||
end
|
||||
|
||||
test "#instrument_call does not confuse the reporter context with the user-defined server_context" do
|
||||
# `instrument_call(server_context:)` is the exception-reporter context, a
|
||||
# different value from the user-defined hash. Enabling the flag must
|
||||
# surface the latter, not the former.
|
||||
subject = SubjectWithServerContext.new({ user_id: 42 })
|
||||
subject.configuration.instrument_server_context = true
|
||||
|
||||
subject.instrumented_method_with_server_context({ request: "reporter-context" })
|
||||
|
||||
assert_equal({ user_id: 42 }, subject.instrumentation_data_received[:server_context])
|
||||
end
|
||||
|
||||
test "#instrument_call skips server_context for hosts without the reader even when enabled" do
|
||||
subject = Subject.new
|
||||
subject.configuration.instrument_server_context = true
|
||||
|
||||
subject.instrumented_method
|
||||
|
||||
refute_includes subject.instrumentation_data_received.keys, :server_context
|
||||
end
|
||||
|
||||
test "#instrument_call adds the method name to the instrumentation data" do
|
||||
subject = Subject.new
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue