Skip to content

feat(sre): integrate traces, logs, metrics into one sdk#6580

Merged
c121914yu merged 8 commits intolabring:v4.14.9-devfrom
xqvvu:feat/metrics-monitor
Mar 20, 2026
Merged

feat(sre): integrate traces, logs, metrics into one sdk#6580
c121914yu merged 8 commits intolabring:v4.14.9-devfrom
xqvvu:feat/metrics-monitor

Conversation

@xqvvu
Copy link
Copy Markdown
Collaborator

@xqvvu xqvvu commented Mar 18, 2026

No description provided.

@c121914yu c121914yu force-pushed the v4.14.9-dev branch 3 times, most recently from c955339 to b29e10c Compare March 19, 2026 06:09
@xqvvu xqvvu force-pushed the feat/metrics-monitor branch 2 times, most recently from df3f271 to 625ba84 Compare March 20, 2026 02:38
Copy link
Copy Markdown
Collaborator

@c121914yu c121914yu left a comment

Choose a reason for hiding this comment

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

PR Review: feat(sre): integrate traces, logs, metrics into one sdk

变更概览

  • PR 编号: #6580
  • 作者: @xqvvu
  • 分支: v4.14.9-dev <- feat/metrics-monitor
  • 变更统计: +2821 -475 行, 42 个文件
  • CI 状态: 全部通过

优点

  1. 架构方向正确: 将 logger、metrics、tracing 统一到 @fastgpt-sdk/otel 是合理的可观测性演进方向, 渐进式迁移策略降低了风险
  2. Workflow 可观测性: step 级别的 tracing + metrics 埋点设计合理, span 属性覆盖了关键维度
  3. HTTP 中间件集成: NextEntry 中的 span 集成干净, 正确处理了错误场景
  4. 环境变量设计: 每个信号独立开关, 同时兼容标准 OTEL 环境变量

问题汇总

严重问题 (3 个)

  1. Span 状态码魔法数字重复 - 3 个文件中硬编码 SPAN_STATUS_CODE_ERROR = 2, 应统一导入
  2. 模块级 getMeter() 调用时序问题 - metrics.ts 在模块加载时调用 getMeter(), 可能在 configureMetrics() 之前执行
  3. z.url() 类型变更 - Zod v4 中 z.url() 返回 URL 对象而非 string, 可能导致下游运行时错误

建议改进 (4 个)

  1. normalizeAttributes 在 2 个文件中重复实现
  2. process.memoryUsage() per-step 调用存在性能隐患
  3. Workflow dispatch 嵌套层级过深, 建议分离 observability 和业务逻辑
  4. 缺少 disposeMetrics() / disposeTracing() 的 shutdown 调用

可选优化 (3 个)

  1. HTTP span 命名建议遵循 OTel 语义约定
  2. 内存增长指标是进程级别的, 并发 step 时无法归因
  3. @fastgpt-sdk/logger 依赖可在后续清理

总体评价

  • 代码质量: 4/5
  • 安全性: 5/5
  • 性能: 3/5
  • 可维护性: 4/5

审查结论

需修改 - 建议修复严重问题后合并。整体方向正确, 是一个有价值的可观测性基础设施改进。

详细代码评论见下方行级注释。

Comment thread packages/service/common/middle/entry.ts
Comment thread packages/service/core/workflow/metrics.ts
Comment thread packages/service/core/workflow/metrics.ts
Comment thread packages/service/env.ts
Comment thread packages/service/common/tracing/client.ts
Comment thread packages/service/core/workflow/dispatch/index.ts
Comment thread projects/app/src/instrumentation.ts
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026

Build Successful - Preview sandbox Image for this PR:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:sandbox_70adfc096bfacf402ac7b010cb1e4a73c53603e5

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026

Build Successful - Preview fastgpt Image for this PR:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fastgpt_70adfc096bfacf402ac7b010cb1e4a73c53603e5

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 20, 2026

Build Successful - Preview mcp_server Image for this PR:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:mcp_server_70adfc096bfacf402ac7b010cb1e4a73c53603e5

@c121914yu c121914yu force-pushed the v4.14.9-dev branch 2 times, most recently from 96e4511 to 9c6c5b1 Compare March 20, 2026 10:11
@c121914yu c121914yu force-pushed the feat/metrics-monitor branch from d1f4a7c to 31bb56a Compare March 20, 2026 10:20
@cla-assistant
Copy link
Copy Markdown

cla-assistant bot commented Mar 20, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ xqvvu
✅ c121914yu
❌ YYH211
You have signed the CLA already but the status is still pending? Let us recheck it.

@cla-assistant
Copy link
Copy Markdown

cla-assistant bot commented Mar 20, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ c121914yu
✅ xqvvu
❌ YYH211
You have signed the CLA already but the status is still pending? Let us recheck it.

@c121914yu c121914yu merged commit c188e97 into labring:v4.14.9-dev Mar 20, 2026
4 of 5 checks passed
c121914yu added a commit that referenced this pull request Mar 21, 2026
* fix: image read and json error (Agent) (#6502)

* fix:
1.image read
2.JSON parsing error

* dataset cite and pause

* perf: plancall second parse

* add test

---------

Co-authored-by: archer <545436317@qq.com>

* master message

* remove invalid code

* feat(sre): integrate traces, logs, metrics into one sdk (#6580)

* fix: image read and json error (Agent) (#6502)

* fix:
1.image read
2.JSON parsing error

* dataset cite and pause

* perf: plancall second parse

* add test

---------

Co-authored-by: archer <545436317@qq.com>

* master message

* wip: otel sdk

* feat(sre): integrate traces, logs, metrics into one sdk

* fix(sre): use SpanStatusCode constants

* fix(sre): clarify step memory measurement

* update package

* fix: ts

---------

Co-authored-by: YeYuheng <57035043+YYH211@users.noreply.github.com>
Co-authored-by: archer <545436317@qq.com>

* doc

* sandbox in agent (#6579)

* doc

* update template

* fix: pr

* fix: sdk package

* update lock

* update next

* update dockerfile

* dockerfile

* dockerfile

* update sdk version

* update dockerefile

* version

---------

Co-authored-by: YeYuheng <57035043+YYH211@users.noreply.github.com>
Co-authored-by: Ryo <whoeverimf5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants