feat(telemetry): add aggregate_module to load/populate/snapshot telemetry metadata - #69
Conversation
…etry metadata
Enables OTel span names to follow the {operation} {destination} convention
and surfaces messaging.destination.name + code.namespace attributes.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryMedium Risk Overview Updates OpenTelemetry instrumentation for these operations to name spans as Reviewed by Cursor Bugbot for commit 295e911. Bugbot is set up for automated code reviews on this repo. Configure here. |
WalkthroughThe PR extends telemetry metadata across aggregate lifecycle events to include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
🧹 Nitpick comments (1)
lib/commanded/opentelemetry/aggregate_populate.ex (1)
33-33: Make aggregate module extraction resilient to missing metadata.Direct
meta.aggregate_moduleaccess can raise if metadata is ever partial/malformed. A guarded extraction with fallback avoids handler crashes and avoids blank/ambiguous span names.♻️ Proposed hardening
- aggregate_module_name = Helpers.module_name(meta.aggregate_module) + aggregate_module_name = + meta + |> Map.get(:aggregate_module) + |> Helpers.module_name() + |> Kernel.||("unknown.aggregate") @@ - "load #{aggregate_module_name}", + "load #{aggregate_module_name}", @@ - aggregate_module_name = Helpers.module_name(meta.aggregate_module) + aggregate_module_name = + meta + |> Map.get(:aggregate_module) + |> Helpers.module_name() + |> Kernel.||("unknown.aggregate") @@ - "populate #{aggregate_module_name}", + "populate #{aggregate_module_name}",Also applies to: 49-50, 98-99, 117-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commanded/opentelemetry/aggregate_populate.ex` at line 33, The code directly accesses meta.aggregate_module which can crash if meta is nil/partial; update all occurrences (e.g., the assignment creating aggregate_module_name and the similar uses at the other noted spots) to safely fetch the field (use Map.get(meta, :aggregate_module) or pattern-match meta to optional keys) and provide a clear fallback value (e.g., a string like "unknown_aggregate" or derive from another safe field) before passing into Helpers.module_name so span names won’t be blank or raise when metadata is malformed; update every instance referenced (the lines creating aggregate_module_name and the two other similar blocks) to use the guarded fetch and fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/commanded/opentelemetry/aggregate_populate.ex`:
- Line 33: The code directly accesses meta.aggregate_module which can crash if
meta is nil/partial; update all occurrences (e.g., the assignment creating
aggregate_module_name and the similar uses at the other noted spots) to safely
fetch the field (use Map.get(meta, :aggregate_module) or pattern-match meta to
optional keys) and provide a clear fallback value (e.g., a string like
"unknown_aggregate" or derive from another safe field) before passing into
Helpers.module_name so span names won’t be blank or raise when metadata is
malformed; update every instance referenced (the lines creating
aggregate_module_name and the two other similar blocks) to use the guarded fetch
and fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78d08e69-d65a-45ad-a3f1-bd63187edd31
📒 Files selected for processing (7)
lib/commanded/aggregates/aggregate.exlib/commanded/aggregates/aggregate_state_builder.exlib/commanded/opentelemetry/aggregate_populate.exlib/commanded/opentelemetry/aggregate_snapshot.extest/opentelemetry/aggregate_populate_test.exstest/opentelemetry/aggregate_snapshot_test.exstest/support/factory.ex
aggregate_modulefield was already present in the%Aggregate{}struct but was omitted from the telemetry metadata emitted byAggregateStateBuilderanddo_take_snapshot/1, leavingload,populate, andsnapshotspans without a destination{operation} {destination}convention (load MyApp.BankAccount,populate MyApp.BankAccount,snapshot MyApp.BankAccount) consistent with howexecute,dispatch, andhandlespans are already namedmessaging.destination.nameandcode.namespaceare now surfaced as span attributes for these three operations, matching the attribute coverage already present onexecutespans