Skip to content

feat: implement OpenTelemetry instrumentation for aggregates - #45

Merged
yordis merged 1 commit into
mainfrom
otel-aggregate
Jan 17, 2026
Merged

yordis merged 1 commit into
mainfrom
otel-aggregate

Conversation

@yordis

@yordis yordis commented Jan 17, 2026

Copy link
Copy Markdown
Member

Signed-off-by: Yordis Prieto yordis.prieto@gmail.com

@cursor

cursor Bot commented Jan 17, 2026 •

Copy link
Copy Markdown

PR Summary

Introduces full OpenTelemetry tracing for aggregates and consolidates tracing utilities.

  • New Commanded.OpenTelemetry.Aggregate attaches telemetry to [:commanded, :aggregate, :execute] to create consumer spans, propagate/clear context, set event counts, and record exceptions
  • Add Commanded.OpenTelemetry.Helpers for trace context attach/extract, error formatting, and module/struct name helpers; EventHandler refactored to use it and adopts semconv span names (e.g., "handle <module>", "batch <module>")
  • Extend Commanded.OpenTelemetry.setup/1 with aggregate option (:disabled or default enabled) alongside existing event_handler options
  • Tests: new aggregate_test.exs, expanded event_handler_test.exs, factory/build helpers, and test router; verify span attributes, error statuses, links/parenting across :link | :child | :none, and stale context clearing
  • Update mix.exs test paths to include test/opentelemetry/support

Written by Cursor Bugbot for commit 839010f. This will update automatically on new commits. Configure here.

@coderabbitai

coderabbitai Bot commented Jan 17, 2026 •

Copy link
Copy Markdown

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds OpenTelemetry aggregate execute instrumentation and shared Helpers; refactors EventHandler to use Helpers; exposes aggregate configuration in setup; adds tests, factories, and test router to validate aggregate tracing and context propagation.

Changes

Cohort / File(s) Summary
Aggregate instrumentation
lib/commanded/opentelemetry/aggregate.ex
New module that attaches telemetry handlers for `[:commanded, :aggregate, :execute, :start
Shared helpers
lib/commanded/opentelemetry/helpers.ex
New Helpers module for context propagation (attach_ctx/1, build_headers_from_metadata/1), error typing/formatting (to_error_type/2, format_error/1), and naming utilities (module_name/1, struct_name/1).
Setup/config
lib/commanded/opentelemetry.ex
Added Aggregate alias and NimbleOptions :aggregate option (type {:in, [:disabled, []]}, default []); setup/1 conditionally initializes aggregate tracing unless :disabled.
EventHandler refactor
lib/commanded/opentelemetry/event_handler.ex
Replaced local context/error utilities with Helpers.*, updated span naming to OTEL SemConv forms (e.g., "handle ..." / "batch ..."), and removed duplicated private functions.
Tests: aggregate
test/opentelemetry/aggregate_test.exs
New comprehensive tests covering setup, attributes, success/error paths, event counts, traceparent/tracestate propagation, edge cases, and exception handling.
Tests: event handler updates
test/opentelemetry/event_handler_test.exs
Updated expectations for span names and full attribute maps, expanded exception and trace relationship tests, and refactored test scaffolding (+324/-252).
Test router & factories
test/opentelemetry/support/test_router.ex, test/support/factory.ex
Added TestRouter for command dispatch; extended Factory with projector-aware builders and aggregate telemetry metadata/builders.
Test infra config
mix.exs, test/support/opentelemetry_case.ex
Included test/opentelemetry/support in elixirc_paths for test/bench and added aggregate execute events to teardown telemetry cleanup.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client/Command
    participant Router as Commanded Router
    participant Aggregate as Aggregate Executor
    participant Telemetry as Telemetry Emitter
    participant OTel as OTel Aggregate Handler
    participant Tracer as OTEL Tracer
    participant Span as Span

    Client->>Router: dispatch(command)
    Router->>Aggregate: execute(command)
    Aggregate->>Telemetry: emit [:commanded, :aggregate, :execute, :start]
    Telemetry->>OTel: handle start event
    OTel->>OTel: extract_trace_context(metadata)
    OTel->>Tracer: start_consumer_span("execute <aggregate>")
    Tracer->>Span: create(name, attributes)
    Aggregate->>Aggregate: apply events
    Aggregate->>Telemetry: emit [:commanded, :aggregate, :execute, :stop]
    Telemetry->>OTel: handle stop event
    OTel->>Span: set_attribute(event.count)
    OTel->>Span: end()
    alt Exception occurs
        Aggregate-->>Telemetry: emit [:commanded, :aggregate, :execute, :exception]
        Telemetry->>OTel: handle exception event
        OTel->>Span: set_attribute(error.*)
        OTel->>Span: set_status(error)
        OTel->>Span: end()
    end
    Router-->>Client: result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐇 I hopped through spans with whiskers twitching bright,

Traced every command from morning through night,
Context stitched neatly, no stray span in sight,
Helpers at paw to make tracing just right,
A carrot of tests — hops of pure delight!

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description contains only a signed-off-by line with no substantive information about the changes, making it too vague to convey meaningful information about the changeset. Provide a meaningful description of the changes, including the purpose of the instrumentation and how to use the new aggregate tracing configuration.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: implementing OpenTelemetry instrumentation for aggregates, which is the primary focus of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Comment thread lib/commanded/opentelemetry/aggregate.ex Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Comment thread test/opentelemetry/aggregate_test.exs Outdated
Comment thread lib/commanded/opentelemetry/aggregate.ex Outdated
@yordis
yordis force-pushed the otel-aggregate branch 7 times, most recently from 4c8e219 to 287c8d8 Compare January 17, 2026 08:05
@yordis
yordis marked this pull request as ready for review January 17, 2026 08:06
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis enabled auto-merge (squash) January 17, 2026 08:09
@yordis
yordis disabled auto-merge January 17, 2026 08:09

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/support/factory.ex (1)

116-190: Align projector-specific recorded_event defaults with event_type.
build_recorded_event(:account_projector|:transaction_projector, opts) overrides event_type to MyApp.Events.* and stream_id but leaves data defaulting to TestDomain.AccountOpened from the generic builder. This creates a mismatch where the recorded event's event_type string does not correspond to its actual data struct, which can cause confusing failures if tests or helpers inspect recorded_event.data. Provide projector-specific default data structures (or require callers to pass them explicitly) to keep the event type and data struct in sync.

🧹 Nitpick comments (1)
lib/commanded/opentelemetry/helpers.ex (1)

39-60: Unreachable exception clause due to pattern matching order.

%{__struct__: _} matches exceptions too, so the %{__exception__: true} clause never runs. Consider reordering or removing the redundant clause to avoid dead code.

♻️ Suggested reorder to avoid the dead clause
-def to_error_type(%{__struct__: module}, _tracer_id), do: to_string(module)
-
-def to_error_type(%{__exception__: true} = exception, _tracer_id),
-  do: to_string(exception.__struct__)
+def to_error_type(%{__exception__: true} = exception, _tracer_id),
+  do: to_string(exception.__struct__)
+
+def to_error_type(%{__struct__: module}, _tracer_id), do: to_string(module)

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.

1 participant