perf: 遥测上传失败节点 - #303
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
嗨,我在这里给出了一些总体反馈:
active_task和nested_entries这部分逻辑假设Context::run_task的执行是单线程并且严格串行的;如果存在任务重叠或可重入的可能性,最好在文档中明确说明这一假设,或者增加保护措施,以避免将节点失败错误地归属到错误的父 span。- 在
record_failed_node中,failed_nodes会在检查是否存在task_span之前就递增,这意味着有些失败会被计数但不会真正体现为 span;可以考虑在成功获取父 span 之后再进行限流计数,这样该计数器就能反映真正上报的节点 span。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `active_task` and `nested_entries` logic assumes a single-threaded, strictly serial `Context::run_task` flow; if there is any chance of overlapping tasks or re-entrancy, it would be safer to document that assumption clearly or add guards to avoid mis-attributing node failures to the wrong parent span.
- In `record_failed_node`, `failed_nodes` is incremented before checking whether a `task_span` exists, which means some failures are counted but never materialize as spans; consider moving the rate-limiting increment after successfully obtaining the parent span so the counter reflects actual reported node spans.帮我变得更有用!请对每条评论点击 👍 或 👎,我会根据你的反馈改进后续的代码评审。
Original comment in English
Hey - I've left some high level feedback:
- The
active_taskandnested_entrieslogic assumes a single-threaded, strictly serialContext::run_taskflow; if there is any chance of overlapping tasks or re-entrancy, it would be safer to document that assumption clearly or add guards to avoid mis-attributing node failures to the wrong parent span. - In
record_failed_node,failed_nodesis incremented before checking whether atask_spanexists, which means some failures are counted but never materialize as spans; consider moving the rate-limiting increment after successfully obtaining the parent span so the counter reflects actual reported node spans.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `active_task` and `nested_entries` logic assumes a single-threaded, strictly serial `Context::run_task` flow; if there is any chance of overlapping tasks or re-entrancy, it would be safer to document that assumption clearly or add guards to avoid mis-attributing node failures to the wrong parent span.
- In `record_failed_node`, `failed_nodes` is incremented before checking whether a `task_span` exists, which means some failures are counted but never materialize as spans; consider moving the rate-limiting increment after successfully obtaining the parent span so the counter reflects actual reported node spans.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
opus 5.0

Summary by Sourcery
在遥测中将失败的流水线节点作为任务 span 下的子 span 进行附加,以为 Maa 任务提供详细、可追踪的失败链路(包括嵌套流水线和时间信息),同时对每个任务的 span 数量施加限制。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Attach failed pipeline nodes as child spans under task spans in telemetry to provide a detailed, traceable failure chain for Maa tasks, including nested pipelines and timing information, while enforcing per-task span limits.
New Features:
Enhancements: