send block creation events for sub-blocks as well#3247
Conversation
WalkthroughThis pull request adds telemetry tracking to differentiate between top-level block creation and sub-block creation in the wcore package. Changes include: adding Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/wcore/block.go (1)
24-39:⚠️ Potential issue | 🟠 MajorClarify telemetry intent for
CreateSubBlock— asymmetry with top-level path.The PR adds unconditional telemetry to
CreateSubBlockwithout an opt-out flag. This differs from the top-level path:CreateBlockWithTelemetryexposes arecordTelemetryparameter (thoughCreateBlockitself always passestrue), butCreateSubBlockhas no such option.Before merging, confirm whether:
- Sub-block telemetry inflation is intentional and acceptable (each sub-block emits
action:createblockandRenderersactivity).- If frequent sub-block creation is expected, whether filtering at the reporting layer (
block:subblock=true) is preferred to avoid skewing historicalaction:createblockmetrics, or whether a flag-based opt-out (likeCreateBlockWithTelemetry) is better.The code search shows the only caller is
CreateSubBlockCommand(RPC handler), so this is not driven directly by vdom itself — but understanding the typical frequency of sub-block creation will inform the decision.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/wcore/block.go` around lines 24 - 39, CreateSubBlock unconditionally calls recordBlockCreationTelemetry which asymmetrically inflates telemetry versus the top-level CreateBlockWithTelemetry; update CreateSubBlock to accept an opt-out telemetry flag (or honor a blockDef.Meta key like "block:subblock" / "recordTelemetry") and pass that flag through to decide whether to call recordBlockCreationTelemetry, or alternatively annotate the emitted telemetry with a "block:subblock=true" tag so reporting can filter it; change the CreateSubBlock signature and its caller CreateSubBlockCommand (and any other callers) to provide the flag or ensure Meta contains the filtering tag, and keep recordBlockCreationTelemetry usage consistent with CreateBlockWithTelemetry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/wcore/block.go`:
- Around line 24-39: CreateSubBlock unconditionally calls
recordBlockCreationTelemetry which asymmetrically inflates telemetry versus the
top-level CreateBlockWithTelemetry; update CreateSubBlock to accept an opt-out
telemetry flag (or honor a blockDef.Meta key like "block:subblock" /
"recordTelemetry") and pass that flag through to decide whether to call
recordBlockCreationTelemetry, or alternatively annotate the emitted telemetry
with a "block:subblock=true" tag so reporting can filter it; change the
CreateSubBlock signature and its caller CreateSubBlockCommand (and any other
callers) to provide the flag or ensure Meta contains the filtering tag, and keep
recordBlockCreationTelemetry usage consistent with CreateBlockWithTelemetry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1d7f85c7-08b2-44fa-8270-8769c5fb1743
📒 Files selected for processing (3)
.gitignorepkg/telemetry/telemetrydata/telemetrydata.gopkg/wcore/block.go
need to understand vdom usage