Skip to content

feat(mcp): add observability for MCP Server with log search and metrics - #2580

Merged
Han-Ya-Jun merged 15 commits into
TencentBlueKing:masterfrom
Han-Ya-Jun:feat-add-mcpserver-observable
Apr 16, 2026
Merged

feat(mcp): add observability for MCP Server with log search and metrics#2580
Han-Ya-Jun merged 15 commits into
TencentBlueKing:masterfrom
Han-Ya-Jun:feat-add-mcpserver-observable

Conversation

@Han-Ya-Jun

Copy link
Copy Markdown
Member

Description

feat(mcp): Add observability for MCP Server with log search and metrics

Add log search and Prometheus metrics monitoring capabilities for MCP Server to help users understand the running status of MCP Server.

Dashboard Backend

  • Log Search (apis/web/mcp_server_log):

    • Support multi-dimensional log search by request_id, client_ip, tool_name, mcp_server_name, etc.
    • Support chain search (ChainSearch) to trace complete request flow
    • Support time range queries and ES log storage integration
  • Metrics Monitoring (apis/web/mcp_server_metrics):

    • Provide Prometheus queries for core metrics: QPS, latency (P50/P95/P99), error rate
    • Support aggregation by mcp_server_name, tool_name, status_code dimensions
    • Provide metrics overview API
  • Config: Add MCP metrics related Prometheus config in conf/default.py

MCP Proxy (Go)

  • Add Prometheus push related config in config.yaml.tpl
  • Adjust metric registration and push logic in metric/init.go
  • Add integration tests

Frontend

  • Add MCP Server log search API in access-log.ts

chore(lint): Remove stale ESB ignore rules from import-linter config

Remove 8 obsolete ESB import ignore rules from pyproject.toml that no longer match any actual imports, fixing No matches for ignored import errors.

Impact

  • MCP Server management module (logs, metrics)
  • MCP Proxy metrics push config
  • import-linter config cleanup (no functional impact)

@wklken

This comment was marked as duplicate.

@Han-Ya-Jun
Han-Ya-Jun marked this pull request as draft March 24, 2026 12:47
wklken

This comment was marked as duplicate.

@Han-Ya-Jun
Han-Ya-Jun marked this pull request as ready for review March 25, 2026 06:49
Comment thread src/mcp-proxy/pkg/config/config.go
Comment thread src/mcp-proxy/tests/integration/config.yaml Outdated
Comment thread src/dashboard/apigateway/apigateway/urls.py Outdated
Comment thread src/dashboard/apigateway/apigateway/urls.py
Comment thread src/dashboard/apigateway/apigateway/conf/default.py
Comment thread src/dashboard/apigateway/apigateway/biz/plugin/synchronizers.py Outdated
@wklken

wklken commented Apr 1, 2026

Copy link
Copy Markdown
Collaborator

review again

wklken

This comment was marked as outdated.

@Han-Ya-Jun
Han-Ya-Jun force-pushed the feat-add-mcpserver-observable branch 2 times, most recently from 80a51aa to 6088a68 Compare April 14, 2026 12:08
Features:
- Add MCP server log query APIs (chain search, waterfall view, time chart)
- Add MCP server metrics APIs (prometheus integration)
- Enhance mcp-proxy with audit logging and tracing support
- Add request chain tracking and latency distribution
- Support upstream gateway log correlation
- Add tool_name, latency, and other fields to audit logs
- Fix total_latency_ms calculation using max end time of all spans
- Add upstream field to span for upstream service tracking
- Fix sort field validation and timestamp field addition
- Improve MCP server log query with default time range
- Add upstream_request_id support for traceability

Fixes:
- Fix lint issues (cyclomatic complexity, line length)
- Fix mypy type errors in MCP Server metrics
- Fix CI failures and code review issues
- Address PR review comments
@Han-Ya-Jun
Han-Ya-Jun force-pushed the feat-add-mcpserver-observable branch from 6088a68 to 967d985 Compare April 14, 2026 12:15
- Fix line length issues in proxy.go and metric/init.go
- Reduce cyclomatic complexity of genToolHandler by extracting helper functions:
  - setupToolCallSpan: setup trace span for tool calls
  - prepareToolCallAuditLog: prepare audit logger with context
  - buildToolCallClient: build HTTP client with shared transport
  - handleToolCallError: handle tool call errors
@wklken

wklken commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

review again

- Add missing app_code parameter to MCPSessionTotal metric calls in test
- MCPSessionTotal expects 3 labels: gateway_name, mcp_server_name, app_code
@wklken

This comment was marked as duplicate.

Comment thread src/dashboard/apigateway/apigateway/apis/web/mcp_server/views.py
Comment thread src/dashboard/apigateway/apigateway/apis/web/mcp_server_log/views.py Outdated
Comment thread src/dashboard/apigateway/apigateway/apis/web/mcp_server_log/views.py Outdated
Comment thread src/dashboard/apigateway/apigateway/biz/mcp_server_log/chain_search.py Outdated
Comment thread src/mcp-proxy/pkg/infra/logging/init.go
Comment thread src/dashboard/apigateway/apigateway/biz/mcp_server_log/chain_search.py Outdated
Comment thread src/dashboard/apigateway/apigateway/biz/mcp_server_log/chain_search.py Outdated
Comment thread src/dashboard/apigateway/apigateway/biz/mcp_server_log/chain_search.py Outdated
Why this change was needed:
MCP Server log query APIs with gateway_id in path were not returning data
due to missing fallback query logic and ES field mapping issues. Also,
tools/call requests were missing from the MCPRequestTotal metric.

What changed:
- Added fallback query logic for log detail/chain APIs to try request_id,
  x_request_id, and upstream_request_id sequentially
- Changed ES filter from 'term' to 'match' query for better field mapping
  compatibility (text vs keyword types)
- Added MCPRequestTotal metric recording for tools/call in proxy.go
- Updated metrics middleware to record tools/call metrics only when errors
  occur (successful calls are recorded in tool handler)
- Updated integration tests to verify tools/call metrics

Problem solved:
- Log query APIs with gateway_id now return data correctly
- tools/call requests are now properly counted in MCPRequestTotal metric

Refs: story=132566647

Co-authored-by: claude <noreply@anthropic.com>
@wklken

This comment was marked as duplicate.

…dule

Why this change was needed:
PR review (2580) identified several code quality issues:
- Helper functions defined inline in views.py instead of separate modules
- Inner function definitions within chain_search.py methods
- Utility functions and constants not extracted to reusable modules
- Debug logging at INFO level should be DEBUG
- MCPServerAppPermissionAppCodeListOutputSLZ not used for serialization
- _build_mcp_server_log_client should be in biz layer with gateway_name param
- Accidentally removed enable_gateway_operation_status/enable_run_data_metrics from DEFAULT_FEATURE_FLAG
- Unnecessary change to releaser.py

What changed:
- Extract helper functions to chain_helpers.py (span_to_log, flatten_spans_to_logs, build_chain_summary, build_latency_distribution, enrich_chain_data)
- Extract utility functions to utils.py (parse_latency_ms, calc_max_end_time, build_mcp_server_log_client)
- Move constants to constants.py
- Replace inline function definitions in chain_search.py with imports
- Move _build_mcp_server_log_client from views.py to biz/utils.py with gateway_name parameter
- Use MCPServerAppPermissionAppCodeListOutputSLZ for serialization in mcp_server/views.py
- Downgrade debug logging from logger.info to logger.debug in chain_search.py and log_search.py
- Restore enable_gateway_operation_status and enable_run_data_metrics in DEFAULT_FEATURE_FLAG
- Add these parameters to get_default_feature_flags function signature
- Revert unnecessary change to releaser.py (split maxsplit)

Co-authored-by: claude <noreply@anthropic.com>
@wklken

This comment was marked as duplicate.

…odule

- Extract chain query helpers from views.py to biz/chain_query.py
- Move nested functions to module level in chain_helpers.py
- Split chain_search.py into:
  - es_query.py: ES search operations
  - span_builder.py: span tree construction and log merging
  - gateway_log.py: gateway log search
  - chain_search.py: MCPServerLogChainSearchClient orchestration

Addresses wklken's CR comments on PR TencentBlueKing#2580
@wklken

This comment was marked as duplicate.

…s param

Change latency_ms from float to Optional[float] to match parse_latency_ms return type.
@wklken

wklken commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

PR #2580 Code Review 汇总报告

由 codex-internal (gpt-5.4) + claude-internal 双模型 review,主 agent 汇总整理。
PR: feat(mcp): add observability for MCP Server with log search and metrics
最新 Commit: fad80b3(fix(mcp_log): fix type annotation for _calc_start_offset_ms latency_ms param)
触发原因:检测到新提交 fad80b3(旧:8c7912b3),且 wklken 评论了 "review again"

变更概述

本次 PR 为 MCP Server 补全了可观测性能力,覆盖三个主要方向:

  1. Dashboard Backend(Python):新增 biz/mcp_server_log/ 业务逻辑层,包含日志搜索(log_search)、调用链路追踪(chain_search)、Span 构建(span_builder)等模块,以及对应的 apis/web/mcp_server_log/ Web 入口(列表、汇总、链路查询、导出)和 service/prometheus/mcp_server_dimension.py 指标查询(QPS、P50/P95/P99 延迟、错误率、请求体/响应体大小)。
  2. MCP Proxy(Go):增强 LoggingMiddleware(添加 panic 恢复、结构化日志),在 proxy.go 中补充 tools/call 的日志、审计日志和 Prometheus 指标推送,新增可配置的 metricNamePrefix 和集成测试。
  3. Frontend:在 access-log.ts 中新增 MCP Server 日志搜索 API 入口。

整体设计方向清晰:把 request_idx_request_idupstream_request_id 在 proxy 和 dashboard 两侧串联,形成完整的链路排障和指标监控体系。

问题列表

🔴 Blocking

1. 日志查询接口权限漏洞(跨网关/跨租户风险) [codex]

MCPServerLogQueryApiMCPServerLogQuerySummaryApiMCPServerLogQueryChainApi 均设置了 gateway_permission_exempt = True,将日志查询开放为"仅需登录认证"即可访问。

这些接口返回请求参数、响应内容、客户端 IP、上下游关联网关日志等敏感运维数据。一旦 request_id 泄露(工单、截图等),任意登录用户均可读取不属于自己的网关日志,形成跨网关、跨租户的数据泄露风险。

修复建议:至少补充网关归属校验(确认请求者有权访问该网关的日志),或者调整为需要更高权限(管理员/网关管理员),不能直接跳过网关权限控制。


🟠 Major

2. tools/call 成功/失败判定不一致,导致指标/日志统计失真 [codex]

proxy.gohandleToolCallError()recordToolCallMetrics()logToolCall() 统一以 err != nil 作为失败条件。但 MCP 框架层的很多 tool 调用失败会被包装成 CallToolResult{IsError: true} 并以 err == nil 返回——这类失败不会计入 MCPErrorTotalstatus 会被错误打成 success

这直接削弱本次 PR 的核心价值:最需要排查的失败场景(上游 API 错误)反而被统计成成功,dashboard 的 QPS/错误率聚合会出现误差。

修复建议:在失败判定时同时检查 err != nilresult.IsError == true,确保两种失败路径均被正确统计。

3. LoggingMiddleware 对 tools/call 无条件跳过,形成观测盲区 [codex]

middleware.goLoggingMiddleware()tools/call 做了无条件跳过(return next(...));替代日志仅在 proxy.go 的 tool handler 真正执行时才写出。这会漏掉在 handler 分发前就失败的 tools/call(工具不存在、框架层校验失败、路由阶段报错等)。

相比之下,MetricMiddleware() 已经保留了错误场景埋点;日志这里的处理不一致,使此类失败在日志检索和链路查询中不可见。

修复建议:在 LoggingMiddleware 跳过正常日志前,至少对 err != nil 的情况补充日志写出,与 MetricMiddleware 保持一致。

4. 新增 Python 业务逻辑模块缺少单元测试 [claude]

biz/mcp_server_log/chain_search.pyspan_builder.pylog_search.py 等核心业务逻辑模块均缺少对应的单元测试。根据项目 AGENTS.md 规范,所有新增代码必须包含测试覆盖。链路搜索、日志组装等逻辑较为复杂,缺少测试覆盖会导致边界情况难以验证。

修复建议:为所有新增 biz 层模块补充单元测试,重点覆盖 ES 查询异常、空结果、跨链路 ID 关联等边界情况。


🟡 Minor

5. PromQL 分母为 0 未做保护,低流量场景会出现 NaN [codex]

mcp_server_dimension.pyMCPServerRequestBodySizeMetricsMCPServerResponseBodySizeMetrics 使用 sum(increase(..._sum)) / sum(increase(..._count)),没有处理分母为 0 的时间窗。对低流量或无流量的网关,前端图表会出现断点、空白或异常值。

修复建议:对分母加保护,例如 clamp_min(..., 1) 或显式的 0 回退。

6. chain_search 空结果分支返回结构与正常分支不一致 [codex]

chain_search.pysearch_chain()search_chain_by_x_request_id() 在查不到日志时返回 gateway_log,但正常分支返回的是 upstream_gateway_log / downstream_gateway_log,且空结果分支缺少 timestamp 字段。

修复建议:空结果分支应与正常分支保持相同的 schema,避免前端出现字段不稳定的兼容问题。

7. 日志导出静默截断,审计场景存在误导风险 [codex]

MCPServerLogExportApi 把导出上限硬编码为 10000,但 total_count > limit 时没有任何截断提示。日志导出常用于排障和审计,静默截断会让使用方误以为 CSV 是全量结果,遗漏关键日志。

修复建议:在响应中显式告知"已截断",或在超过上限时直接拒绝导出并提示用户缩小查询范围。

8. 配置管理分散 [claude]

MCP Server 相关配置分散在 default.pyconfig.yaml.tpl 等多处,建议统一配置管理策略,便于后续维护。


💬 Nit

9. 部分日志处理逻辑重复 [claude]

latency 解析、字段截断等逻辑在多个文件中重复出现,可考虑提取为公共工具函数。

10. 部分注释可优化 [claude]

部分中文注释可以优化为更清晰的英文注释,部分函数缺少详细的参数和返回值说明。


优点

  • 链路追踪串联设计清晰request_idx_request_idupstream_request_id 三条关联线在 proxy 和 dashboard 两侧统一,后续链路排障会比现在容易很多。 [codex + claude]
  • MCP 协议语义拆分合理tools/call 的日志、审计、指标分层思路清晰,体现了对 MCP 协议层和上游调用层的语义区分。 [codex]
  • 接口职责明确:dashboard 侧把日志列表、链路详情、汇总、Prometheus 维度查询拆成独立入口,扩展性较好。 [codex + claude]
  • Go 侧稳定性改进LoggingMiddleware 新增 panic 恢复机制,提升系统健壮性。 [claude]
  • 功能完整性:从日志搜索到指标监控覆盖了完整的可观测性方案,支持多维度查询和聚合分析。 [claude]

综合建议

建议暂不合并,优先修复以下两类问题:

第一批(必须修复):

  1. 日志查询接口的权限模型 —— 直接涉及数据安全,跨网关/跨租户读接口风险不可接受
  2. tools/call 成功/失败判定不一致 —— 会导致错误率统计失真,削弱本次 PR 的核心价值
  3. LoggingMiddleware 观测盲区 —— 关键失败场景不可见
  4. 新增 biz 层模块补充单元测试 —— 符合项目规范要求

第二批(上线前建议补齐):
5. PromQL 分母为 0 保护
6. chain_search 空结果 schema 一致性
7. 日志导出截断提示
8. 配置管理统一

整体架构方向正确,代码质量良好,修完以上问题后可以合并。


由 codex-internal (gpt-5.4) + claude-internal 双模型 review,主 agent 汇总 | [from openclaw-internal]

- Check result.IsError in recordToolCallMetrics and logToolCall to catch
  MCP framework-level failures (CallToolResult{IsError: true}) as errors
- Log tools/call failures in LoggingMiddleware when call fails before
  reaching the tool handler (tool not found, validation error, etc.)
- Add clamp_min protection for PromQL body size queries to prevent NaN
  on low-traffic gateways
- Fix chain_search empty result schema to match normal branch structure
  (upstream_gateway_log/downstream_gateway_log + timestamp)
- Add truncation warning log when MCP log export exceeds 10000 limit
Extract helper functions:
- serializeToolCallRequest: serialize tool call request params
- serializeToolCallResponse: serialize tool call response result
- extractToolCallContextInfo: extract context info into struct
- extractToolCallSessionInfo: extract session ID and client_id

Fixes gocyclo lint error and line length violations.
- 移除 exists mcp_method 过滤,同时展示 HTTP 层和 MCP 协议层日志
- 移除 gateway_name match 过滤,使用 gateway_id 精确过滤
- 将 __ext_json 嵌套字段合并到日志顶层展示
- 移除 OutputSLZ 中 bk_app_codes 冗余的 read_only 属性
- 新增 MCP_SERVER_LOG_SEARCH_FIELDS 常量定义搜索字段
…play

Why this change was needed:
The HTTP layer logger (logger.go) only outputted basic HTTP request fields,
while MCP protocol-specific fields (mcp_method, params, response, tool_name,
etc.) were only present in MCP layer logs (middleware.go). This caused the
frontend to display many null values when viewing HTTP layer logs, making
the log display inconsistent and confusing.

What changed:
- Go: Added MCP protocol fields (mcp_method, tool_name, prompt_name, params,
  response, request_body_size, response_body_size, session_id) to HTTP layer
  logger with zero values to maintain structural consistency
- Go: Added bk_username and trace_id fields to HTTP layer logger
- Go: Added GetBkUsername() and GetTraceID() gin context getter functions
- Python: Relaxed __ext_json merge logic to allow empty strings and zero
  values to override null in top-level fields, since "" is more meaningful
  than null in the frontend display

Problem solved:
HTTP layer and MCP layer logs now have a unified field structure. The
frontend no longer shows a large number of null values for MCP-specific
fields when displaying HTTP layer logs.

Co-authored-by: claude <noreply@anthropic.com>
Comment thread src/dashboard/apigateway/apigateway/conf/utils.py Outdated
Han-Ya-Jun and others added 2 commits April 16, 2026 16:25
…h/method in MCP logs

Why this change was needed:
- ES queries using `match` on keyword fields like `mcp_server_name` could
  cause tokenization issues (e.g., "bk-apigateway-prod-context" split into
  multiple tokens), making filters unreliable. `term` is semantically correct
  for keyword fields and avoids this problem.
- MCP protocol-layer logs were outputting HTTP path/method fields that are
  always the same value (POST /mcp/{name}/sse) with no diagnostic value,
  while Filebeat's file path could leak into the `path` field for old logs.
- The `metricNamePrefix` config was under `mcpServer` in YAML but the Go
  struct had moved it to a separate `Metric` section, causing a mismatch
  between config files and code.

What changed:
- Python: Changed `match` to `term` for `mcp_server_name` and other keyword
  field filters in log_search.py to ensure exact matching
- Python: Added `_sanitize_filebeat_path()` to clear Filebeat-injected file
  paths (ending in .log) from the `path` field in ES results
- Go: Set `method` and `path` fields to empty strings in MCP protocol-layer
  logs (middleware.go, proxy.go) instead of propagating HTTP values
- Config: Moved `metricNamePrefix` from `mcpServer` to top-level `metric`
  section in config.yaml.tpl and tests/integration/config.yaml, aligning
  with the `Config.Metric.NamePrefix` Go struct

Problem solved:
- Keyword field filters now work reliably without tokenization side effects
- MCP protocol-layer logs no longer show misleading HTTP path/method values
- Config YAML structure matches the Go struct, preventing silent config
  loading failures

Co-authored-by: claude <noreply@anthropic.com>

@wklken wklken left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2580 Code Review 汇总报告

由 codex-internal (gpt-5.4) + claude-internal 双模型 review,主 agent 汇总整理。

:本次 review 触发自 "review again" 请求(新增提交 a46830a,内容为 fix(conf): set FEATURE_FLAG_ENABLE_RUN_DATA default to False)。该提交为 1 行配置修复,PR 主体为 MCP Server 可观测性功能(日志检索、链路追踪、Prometheus 指标),已在前次 review 中分析。本次综合双模型对全量变更的审查意见汇总如下。

变更概述

本 PR 为 MCP Server 新增完整的可观测性能力,涵盖两条主线:

  1. mcp-proxy 侧:新增 tools/call 日志记录、审计日志、Prometheus 指标埋点,以及调用链路字段(request_id / x_request_id / upstream_request_id)的完整追踪。
  2. dashboard 侧:新增 MCP 日志检索、链路查询(Span 树构建)、Prometheus 维度查询 API,以及工具箱导出能力。

最新提交(a46830a0):修复 FEATURE_FLAG_ENABLE_RUN_DATA 默认值为 False,与 master 分支对齐。

规模:44 个文件,+4415 / -360 行。


问题列表

🔴 Blocking

1. 日志查询接口缺少对象级权限控制 [codex]

src/dashboard/apigateway/apigateway/apis/web/mcp_server_log/views.pyMCPServerLogQueryApiMCPServerLogQuerySummaryApiMCPServerLogQueryChainApi 设置了 gateway_permission_exempt = True,使其仅需登录认证即可访问。

这些接口返回敏感运维数据(请求参数、响应内容、客户端 IP、上下游网关日志)。只要 request_id 被泄露或枚举,任意登录用户即可读取不属于自己的网关日志,形成跨网关、跨租户的数据越权风险。

建议:补充网关归属校验(验证 request_id 归属当前用户的网关)、对象级权限控制,或将接口限制为管理员权限。

2. tools/call 成功/失败判定不一致,导致可观测性数据失真 [codex]

src/mcp-proxy/pkg/infra/proxy/proxy.gohandleToolCallError() / recordToolCallMetrics() / logToolCall() 的失败判定逻辑不一致。

许多 tool 调用失败会被包装为 CallToolResult{IsError: true} 并以 err == nil 返回,但日志和指标统计统一用 err != nil 作为失败条件,导致这类上游 API 错误不会计入 MCPErrorTotalstatus 也会被记为 success。这直接削弱了可观测性功能的核心价值——最需要排查的失败场景反而统计错误。

建议:统一成功/失败判定逻辑,同时检查 CallToolResult.IsErrorerr != nil

🟠 Major

1. 日志中间件无条件跳过 tools/call,形成观测盲区 [codex]

src/mcp-proxy/pkg/mcp/middleware.goLoggingMiddleware()tools/call 做了无条件跳过,替代日志只在 proxy.go 的 handler 实际执行时才写出。这会漏掉在 handler 分发前即失败的请求(如工具不存在、框架层校验失败)。MetricMiddleware() 已专门保留了错误场景埋点,但日志这里直接 return next(...),形成不对称的观测盲区。

2. chain_search.py 空数据场景返回结构不一致 [codex]

search_chain() / search_chain_by_x_request_id() 查不到日志时返回 gateway_log,但正常路径返回 upstream_gateway_log / downstream_gateway_log,且空结果分支缺少 timestamp 字段。这会导致前端在空数据场景下出现字段缺失或渲染异常。

3. ES 查询缺少优化和缓存 [claude]

调用链路查询涉及多次 ES 查询,对高频查询场景可能对 ES 造成压力,缺少查询优化和缓存机制。

4. 输入参数缺少优先级定义 [claude]

MCPServerLogChainSearchClient 构造函数接受多个 ID 参数(request_id、x_request_id、upstream_request_id),当同时传入多个 ID 时缺少明确的查询优先级定义和参数验证。

🟡 Minor

1. PromQL 除零风险 [codex]

src/dashboard/apigateway/apigateway/service/prometheus/mcp_server_dimension.pyMCPServerRequestBodySizeMetrics / MCPServerResponseBodySizeMetrics 直接做 sum(increase(..._sum)) / sum(increase(..._count)),没有处理分母为 0 的情况。低流量场景会返回 NaN / 空样本,前端图表会出现断点或异常值。建议加 clamp_min(..., 1) 保护或提供显式 0 回退。

2. 日志导出无截断提示 [codex]

MCPServerLogExportApi 导出上限硬编码 10000,但超限时无任何截断提示,用户会误以为 CSV 是全量结果,影响审计可靠性。

3. 代码重复 [claude]

chain_search.pychain_query.py 有大量相似的 ES 查询逻辑,span_builder.py 中时间计算逻辑在多处重复,建议提取公共组件。

4. 测试覆盖不足 [claude]

新增的复杂业务逻辑(链路构建、多次 ES 查询)缺少单元测试,边界和异常场景覆盖不够。

💬 Nit

  • 日志字段常量分散在多个文件,建议统一管理 [claude]
  • 部分函数命名风格不一致(下划线 vs 驼峰),建议统一 [claude]
  • 新增 API 接口的 Swagger 文档说明不够详细 [claude]

优点

  • 链路串联设计合理:request_id / x_request_id / upstream_request_id 在 proxy 和 dashboard 两侧统一,为链路排障奠定了良好基础。[codex+claude]
  • 分层架构清晰:MCP 协议层与真实上游调用层的日志、指标分层思路明确,接口职责划分合理。[codex+claude]
  • panic 恢复机制:新增 panic recover,提升了系统稳定性。[claude]
  • 代码模块化程度高:新增模块职责分离清晰,类型注解覆盖良好。[claude]

综合建议

合并前必须修复(Blocking):

  1. 补充日志查询接口的网关归属/对象级权限校验,避免跨租户数据越权。
  2. 统一 tools/call 成功/失败判定(同时检查 err != nilCallToolResult.IsError),确保指标和日志的准确性。

建议尽快修复(Major):
3. LoggingMiddleware 对 tools/call 的处理与 MetricMiddleware 对齐,补全 handler 前失败的日志记录。
4. chain_search.py 空数据返回结构对齐正常路径。
5. 明确多 ID 参数的优先级和验证规则。

后续优化:

  • PromQL 除零保护、导出截断提示、代码重构、测试补全。

由 codex-internal (gpt-5.4) + claude-internal 双模型 review,主 agent 汇总 | [from openclaw-internal]

…n and use backend_name for downstream gateway

Why this change was needed:
MCP proxy logs in ES have a flattened __ext_json field where key values like
request_id, x_request_id, and status are stored as strings. The original query
only searched top-level fields, missing data stored exclusively in __ext_json.
Also, the downstream gateway log displayed a hardcoded "biz-gateway" service name
instead of the actual backend service name from the gateway access log.

What changed:
- es_query.py: search request_id/x_request_id in both top-level and __ext_json
  fields using bool/should queries with raw dict Q() to preserve __ext_json prefix
- log_search.py: add status filter supporting both integer and string HTTP codes
  (for integer/keyword/flattened mapping compatibility), normalize status display
  to "success"/"failed" string format, skip HTTP path/method for MCP layer logs
- constants.py: add STATUS_HTTP_CODES mapping for status filter value conversion
- gateway_log.py: use backend_name field from gateway log as service name for
  downstream gateway instead of hardcoded "biz-gateway"

Problem solved:
- MCP log queries no longer miss records where fields only exist in __ext_json
- Status filtering works correctly regardless of ES mapping type
- Downstream gateway logs now show the actual backend service name

Co-authored-by: claude <noreply@anthropic.com>
@Han-Ya-Jun
Han-Ya-Jun merged commit 405b0a5 into TencentBlueKing:master Apr 16, 2026
5 checks passed
Han-Ya-Jun added a commit to Han-Ya-Jun/blueking-apigateway that referenced this pull request May 18, 2026
…cs (TencentBlueKing#2580)

* feat(mcp): add MCP Server observability with log query and metrics

Features:
- Add MCP server log query APIs (chain search, waterfall view, time chart)
- Add MCP server metrics APIs (prometheus integration)
- Enhance mcp-proxy with audit logging and tracing support
- Add request chain tracking and latency distribution
- Support upstream gateway log correlation
- Add tool_name, latency, and other fields to audit logs
- Fix total_latency_ms calculation using max end time of all spans
- Add upstream field to span for upstream service tracking
- Fix sort field validation and timestamp field addition
- Improve MCP server log query with default time range
- Add upstream_request_id support for traceability

Fixes:
- Fix lint issues (cyclomatic complexity, line length)
- Fix mypy type errors in MCP Server metrics
- Fix CI failures and code review issues
- Address PR review comments

* fix(mcp-proxy): resolve lint issues

- Fix line length issues in proxy.go and metric/init.go
- Reduce cyclomatic complexity of genToolHandler by extracting helper functions:
  - setupToolCallSpan: setup trace span for tool calls
  - prepareToolCallAuditLog: prepare audit logger with context
  - buildToolCallClient: build HTTP client with shared transport
  - handleToolCallError: handle tool call errors

* fix(mcp-proxy): fix test for MCPSessionTotal metric label count

- Add missing app_code parameter to MCPSessionTotal metric calls in test
- MCPSessionTotal expects 3 labels: gateway_name, mcp_server_name, app_code

* fix(mcp): fix MCP Server log query and metrics issues

Why this change was needed:
MCP Server log query APIs with gateway_id in path were not returning data
due to missing fallback query logic and ES field mapping issues. Also,
tools/call requests were missing from the MCPRequestTotal metric.

What changed:
- Added fallback query logic for log detail/chain APIs to try request_id,
  x_request_id, and upstream_request_id sequentially
- Changed ES filter from 'term' to 'match' query for better field mapping
  compatibility (text vs keyword types)
- Added MCPRequestTotal metric recording for tools/call in proxy.go
- Updated metrics middleware to record tools/call metrics only when errors
  occur (successful calls are recorded in tool handler)
- Updated integration tests to verify tools/call metrics

Problem solved:
- Log query APIs with gateway_id now return data correctly
- tools/call requests are now properly counted in MCPRequestTotal metric

Refs: story=132566647

Co-authored-by: claude <noreply@anthropic.com>

* refactor(mcp_log): address code review feedback for MCP Server log module

Why this change was needed:
PR review (2580) identified several code quality issues:
- Helper functions defined inline in views.py instead of separate modules
- Inner function definitions within chain_search.py methods
- Utility functions and constants not extracted to reusable modules
- Debug logging at INFO level should be DEBUG
- MCPServerAppPermissionAppCodeListOutputSLZ not used for serialization
- _build_mcp_server_log_client should be in biz layer with gateway_name param
- Accidentally removed enable_gateway_operation_status/enable_run_data_metrics from DEFAULT_FEATURE_FLAG
- Unnecessary change to releaser.py

What changed:
- Extract helper functions to chain_helpers.py (span_to_log, flatten_spans_to_logs, build_chain_summary, build_latency_distribution, enrich_chain_data)
- Extract utility functions to utils.py (parse_latency_ms, calc_max_end_time, build_mcp_server_log_client)
- Move constants to constants.py
- Replace inline function definitions in chain_search.py with imports
- Move _build_mcp_server_log_client from views.py to biz/utils.py with gateway_name parameter
- Use MCPServerAppPermissionAppCodeListOutputSLZ for serialization in mcp_server/views.py
- Downgrade debug logging from logger.info to logger.debug in chain_search.py and log_search.py
- Restore enable_gateway_operation_status and enable_run_data_metrics in DEFAULT_FEATURE_FLAG
- Add these parameters to get_default_feature_flags function signature
- Revert unnecessary change to releaser.py (split maxsplit)

Co-authored-by: claude <noreply@anthropic.com>

* refactor(mcp_log): extract chain query logic and split chain_search module

- Extract chain query helpers from views.py to biz/chain_query.py
- Move nested functions to module level in chain_helpers.py
- Split chain_search.py into:
  - es_query.py: ES search operations
  - span_builder.py: span tree construction and log merging
  - gateway_log.py: gateway log search
  - chain_search.py: MCPServerLogChainSearchClient orchestration

Addresses wklken's CR comments on PR TencentBlueKing#2580

* fix(mcp_log): fix type annotation for _calc_start_offset_ms latency_ms param

Change latency_ms from float to Optional[float] to match parse_latency_ms return type.

* fix: address CR review issues for MCP observability

- Check result.IsError in recordToolCallMetrics and logToolCall to catch
  MCP framework-level failures (CallToolResult{IsError: true}) as errors
- Log tools/call failures in LoggingMiddleware when call fails before
  reaching the tool handler (tool not found, validation error, etc.)
- Add clamp_min protection for PromQL body size queries to prevent NaN
  on low-traffic gateways
- Fix chain_search empty result schema to match normal branch structure
  (upstream_gateway_log/downstream_gateway_log + timestamp)
- Add truncation warning log when MCP log export exceeds 10000 limit

* fix: move logger definition after all imports to fix E402

* refactor(mcp-proxy): reduce logToolCall cyclomatic complexity below 30

Extract helper functions:
- serializeToolCallRequest: serialize tool call request params
- serializeToolCallResponse: serialize tool call response result
- extractToolCallContextInfo: extract context info into struct
- extractToolCallSessionInfo: extract session ID and client_id

Fixes gocyclo lint error and line length violations.

* fix(mcp-server-log): 修复 MCP Server 日志查询无数据问题

- 移除 exists mcp_method 过滤,同时展示 HTTP 层和 MCP 协议层日志
- 移除 gateway_name match 过滤,使用 gateway_id 精确过滤
- 将 __ext_json 嵌套字段合并到日志顶层展示
- 移除 OutputSLZ 中 bk_app_codes 冗余的 read_only 属性
- 新增 MCP_SERVER_LOG_SEARCH_FIELDS 常量定义搜索字段

* feat(mcp-log): unify HTTP and MCP layer log fields for consistent display

Why this change was needed:
The HTTP layer logger (logger.go) only outputted basic HTTP request fields,
while MCP protocol-specific fields (mcp_method, params, response, tool_name,
etc.) were only present in MCP layer logs (middleware.go). This caused the
frontend to display many null values when viewing HTTP layer logs, making
the log display inconsistent and confusing.

What changed:
- Go: Added MCP protocol fields (mcp_method, tool_name, prompt_name, params,
  response, request_body_size, response_body_size, session_id) to HTTP layer
  logger with zero values to maintain structural consistency
- Go: Added bk_username and trace_id fields to HTTP layer logger
- Go: Added GetBkUsername() and GetTraceID() gin context getter functions
- Python: Relaxed __ext_json merge logic to allow empty strings and zero
  values to override null in top-level fields, since "" is more meaningful
  than null in the frontend display

Problem solved:
HTTP layer and MCP layer logs now have a unified field structure. The
frontend no longer shows a large number of null values for MCP-specific
fields when displaying HTTP layer logs.

Co-authored-by: claude <noreply@anthropic.com>

* fix(mcp-log): use term query for keyword fields and clean up HTTP path/method in MCP logs

Why this change was needed:
- ES queries using `match` on keyword fields like `mcp_server_name` could
  cause tokenization issues (e.g., "bk-apigateway-prod-context" split into
  multiple tokens), making filters unreliable. `term` is semantically correct
  for keyword fields and avoids this problem.
- MCP protocol-layer logs were outputting HTTP path/method fields that are
  always the same value (POST /mcp/{name}/sse) with no diagnostic value,
  while Filebeat's file path could leak into the `path` field for old logs.
- The `metricNamePrefix` config was under `mcpServer` in YAML but the Go
  struct had moved it to a separate `Metric` section, causing a mismatch
  between config files and code.

What changed:
- Python: Changed `match` to `term` for `mcp_server_name` and other keyword
  field filters in log_search.py to ensure exact matching
- Python: Added `_sanitize_filebeat_path()` to clear Filebeat-injected file
  paths (ending in .log) from the `path` field in ES results
- Go: Set `method` and `path` fields to empty strings in MCP protocol-layer
  logs (middleware.go, proxy.go) instead of propagating HTTP values
- Config: Moved `metricNamePrefix` from `mcpServer` to top-level `metric`
  section in config.yaml.tpl and tests/integration/config.yaml, aligning
  with the `Config.Metric.NamePrefix` Go struct

Problem solved:
- Keyword field filters now work reliably without tokenization side effects
- MCP protocol-layer logs no longer show misleading HTTP path/method values
- Config YAML structure matches the Go struct, preventing silent config
  loading failures

Co-authored-by: claude <noreply@anthropic.com>

* fix(conf): set FEATURE_FLAG_ENABLE_RUN_DATA default to False to align with master

* fix(mcp-log): improve ES query compatibility with flattened __ext_json and use backend_name for downstream gateway

Why this change was needed:
MCP proxy logs in ES have a flattened __ext_json field where key values like
request_id, x_request_id, and status are stored as strings. The original query
only searched top-level fields, missing data stored exclusively in __ext_json.
Also, the downstream gateway log displayed a hardcoded "biz-gateway" service name
instead of the actual backend service name from the gateway access log.

What changed:
- es_query.py: search request_id/x_request_id in both top-level and __ext_json
  fields using bool/should queries with raw dict Q() to preserve __ext_json prefix
- log_search.py: add status filter supporting both integer and string HTTP codes
  (for integer/keyword/flattened mapping compatibility), normalize status display
  to "success"/"failed" string format, skip HTTP path/method for MCP layer logs
- constants.py: add STATUS_HTTP_CODES mapping for status filter value conversion
- gateway_log.py: use backend_name field from gateway log as service name for
  downstream gateway instead of hardcoded "biz-gateway"

Problem solved:
- MCP log queries no longer miss records where fields only exist in __ext_json
- Status filtering works correctly regardless of ES mapping type
- Downstream gateway logs now show the actual backend service name

Co-authored-by: claude <noreply@anthropic.com>

---------

Co-authored-by: claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants