aminghadersohi opened a new pull request, #37859: URL: https://github.com/apache/superset/pull/37859
### SUMMARY Adds two layers of observability instrumentation to MCP tool calls, matching the patterns used across Superset's REST API endpoints: **Layer 1 — Middleware duration tracking for ALL tool calls:** - Restructured `LoggingMiddleware` to use `on_call_tool()` (wraps execution) instead of only `on_message()` (fires before execution) - Tool calls now record `duration_ms` (was `None`) and `success` status in the audit log - `on_message()` retained for non-tool traffic (resource reads, prompts) **Layer 2 — Sub-operation timing inside 7 complex tools:** - Added `event_logger.log_context(action="mcp.<tool>.<phase>")` blocks that automatically measure duration and write `Log` rows - **generate_chart**: validation, dataset_lookup, db_write, form_data_cache, preview (5 phases) - **get_chart_data**: chart_lookup, query_execution, format_conversion (3 phases) - **get_chart_preview**: chart_lookup, preview_generation, metadata (3 phases) - **execute_sql**: db_validation, query_execution, response_conversion (3 phases) - **generate_dashboard**: chart_validation, layout, db_write (3 phases) - **update_chart**: chart_lookup, db_write, preview (3 phases) - **add_chart_to_existing_dashboard**: validation, layout, db_write (3 phases) 12 simpler tools (list_*, get_*_info, get_schema, health_check, etc.) get automatic duration tracking from the middleware layer without per-tool changes. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — observability-only change, no UI impact. ### TESTING INSTRUCTIONS 1. Start the MCP service 2. Call any MCP tool (e.g., `list_charts`, `generate_chart`) 3. Check the Superset `logs` table for `mcp_tool_call` action entries — they should now have `duration_ms` populated and `success` in the curated payload 4. For complex tools, check for sub-operation log entries like `mcp.generate_chart.validation`, `mcp.execute_sql.query_execution`, etc. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [x] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
