refactor(plugins): decouple plugin framework from gateway observability service#2827
refactor(plugins): decouple plugin framework from gateway observability service#2827
Conversation
086f205 to
bb1bb88
Compare
crivetimihai
left a comment
There was a problem hiding this comment.
Thanks for this clean decoupling work, @araujof. The Protocol + DI approach is the right pattern. Two critical issues:
1. Two independent current_trace_id ContextVars (silent tracing breakage)
mcpgateway/plugins/framework/observability.py:15 creates a new ContextVar("current_trace_id"), but the gateway's observability middleware sets the original one at mcpgateway/services/observability_service.py:53. These are two distinct Python objects despite sharing the same name string. After this PR, _execute_with_timeout imports from the plugin framework's copy, which the middleware never populates — trace_id will always be None at runtime and plugin spans silently stop recording.
2. No gateway-side adapter wiring
There is no class implementing ObservabilityProvider that bridges to ObservabilityService. And main.py:187 still calls PluginManager(_config_file) without passing observability=. So even after merge, self.observability will be None on the executor, making this a no-op. The PR needs a concrete adapter and it must be passed at the main.py call site.
Minor: The Borg singleton PluginManager.__init__ only sets _executor.observability inside the not self._initialized block, so a second call with a different provider silently drops it.
Thanks, working on it. |
3a9742b to
005d3ce
Compare
…ion for observability service Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
961137b to
2033885
Compare
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Description
Removes the plugin framework's direct dependency on
mcpgateway.db.SessionLocalandmcpgateway.services.observability_serviceby introducing a protocol-based observability abstraction with dependency injection.Closes: #2828
Problem
The plugin framework's
_execute_with_timeoutmethod importedSessionLocalandObservabilityServicedirectly from the gateway, creating a tight coupling that prevented the framework from operating standalone. This also required the plugin framework to manage database sessions for tracing — a concern that belongs to the host application.Solution
ObservabilityProviderprotocol andNullObservabilityno-op default in a newobservability.pymodulecurrent_trace_idContextVar owned by the plugin framework (replacing the one imported from the gateway's observability service)PluginExecutorandPluginManagernow accept an optionalobservabilityparameter via constructor injectionget_plugin_manager()forwards the observability provider to the managerDependency direction (before → after)
Changes
mcpgateway/plugins/framework/observability.pyObservabilityProviderprotocol,NullObservabilitydefault,current_trace_idContextVarmcpgateway/plugins/framework/manager.pyPluginExecutorandPluginManageracceptobservabilityparam;_execute_with_timeoutuses injected provider instead of direct gateway importsmcpgateway/plugins/framework/__init__.pyget_plugin_manager()acceptsobservabilityparam;ObservabilityProvideradded to__all__tests/unit/.../test_observability.pytests/unit/.../test_manager_coverage.pyTestExecuteWithTimeoutto test against injected provider instead of mockingmcpgateway.db/mcpgateway.servicesAdditional Changes
A few tests in
tests/unit/../test_http_auth_integration.pywere failing on dev environment when running the coverage target. Solution: Reset Starlette's cached middleware_stack after injecting the mock plugin manager so the middleware chain is rebuilt with the mock, and restore it afterward to avoid side effects on other tests.Test plan