[yordis]

No description provided.

[cursor]

PR Summary

Introduces OTEL instrumentation for Commanded EventStore operations, with explicit opt-in configuration.

• New Commanded.OpenTelemetry.EventStore attaches telemetry handlers for ack_event, append_to_stream, read/record/delete_snapshot, stream_forward, subscribe/subscribe_to/unsubscribe, emitting internal spans with messaging/code attributes and robust error/exception handling
• Extends Commanded.OpenTelemetry.setup/1 schema and docs with event_store: [:disabled | []] (default :disabled), and wires to invoke EventStore.setup() when enabled
• Tests: comprehensive event_store_test.exs for attributes, error statuses, and exceptions; factory expanded with builders for EventStore metadata/telemetry events; aggregate tests updated to detach EventStore handlers; test domain structs derive Jason.Encoder for convenience

^{Written by Cursor Bugbot for commit 3f26c90. This will update automatically on new commits. Configure here.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

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 EventStore OpenTelemetry instrumentation: new Commanded.OpenTelemetry.EventStore module, a :event_store option and alias in Commanded.OpenTelemetry.setup/1, comprehensive tests and test-factory helpers, plus minor test-domain tweaks.

Changes

Cohort / File(s)	Summary
OpenTelemetry config lib/commanded/opentelemetry.ex	Added public alias EventStore, new NimbleOptions :event_store (default :disabled) and docs/examples update; setup/1 now invokes EventStore.setup() when enabled.
EventStore instrumentation lib/commanded/opentelemetry/event_store.ex	New module registering telemetry handlers for EventStore start/stop/exception events; starts/resumes/ends OpenTelemetry spans, records exceptions/errors, and sets span attributes (application, stream UUID, operation, versions, subscription/start_from, source UUID, etc.).
Instrumentation tests test/opentelemetry/event_store_test.exs	New test module verifying handler registration, idempotent setup, span creation for many EventStore actions, attribute assertions, error/exception cases, and teardown helpers.
Test factory helpers test/support/factory.ex	Added numerous builders for EventStore telemetry events and metadata (append/stream_forward/subscribe_to/record/read/ack/delete/unsubscribe/delete_subscription, etc.) used by tests.
Test domain updates test/support/test_domain.ex	Added @derive Jason.Encoder to several test structs to enable JSON encoding in tests.
Aggregate test adjustments test/opentelemetry/aggregate_test.exs	Added teardown for event_store telemetry handlers (detach_event_store_handlers/0) to avoid cross-test interference.

Sequence Diagram(s)

sequenceDiagram
    participant ES as EventStore (operation)
    participant TEL as :telemetry
    participant HND as Commanded.OpenTelemetry.EventStore
    participant OT as OpenTelemetry API

    ES->>TEL: emit [:commanded, :event_store, action, :start]
    TEL->>HND: handle start event
    HND->>HND: extract metadata & build attributes
    HND->>OT: start_span(name, attributes)

    ES->>TEL: emit [:commanded, :event_store, action, :stop]
    TEL->>HND: handle stop event
    HND->>HND: resume span, attach error attrs if present
    HND->>OT: set_status_if_error(), end_span()

    ES->>TEL: emit [:commanded, :event_store, action, :exception]
    TEL->>HND: handle exception event
    HND->>HND: normalize exception, record details, set error attrs
    HND->>OT: record_exception(), set_error_status(), end_span()

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat: implement OpenTelemetry instrumentation for aggregates #45 — Adds an instrumentation alias/option and extends Commanded.OpenTelemetry.setup/1 to initialize a submodule; similar integration pattern.
• feat: implement OpenTelemetry instrumentation for application #46 — Also adds new OpenTelemetry instrumentation modules and updates Commanded.OpenTelemetry wiring; closely related changes.
• feat: Add OpenTelemetry instrumentation for aggregate populate #47 — Modifies OpenTelemetry initialization to delegate setup to submodules; similar structural edits.

Poem

🐇 I scoped each stream and named each span,
I hopped through start and stop as only a rabbit can.
I sniffed for errors, recorded every trace,
Carrots mark the path through each traced space.
🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess relevance to the changeset.	Add a pull request description explaining the purpose, scope, and benefits of the OpenTelemetry EventStore instrumentation changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding OpenTelemetry instrumentation for EventStore operations across multiple files.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@test/support/factory.ex`:
- Around line 949-959: Group all clauses of build_telemetry_event/2 by moving
the block of event_store_* clauses (for example
build_telemetry_event(:event_store_append_to_stream_start, opts) and the other
build_telemetry_event(:event_store_*, opts) functions) so they appear
immediately after the existing build_telemetry_event clauses and before
build_account_scenario, ensuring no other unrelated function definitions sit
between them; this will remove the compiler warning about scattered clauses.

♻️ Duplicate comments (6)

test/support/test_domain.ex (1)

8-44: Consider removing @derive Jason.Encoder if unused by OpenTelemetry tests.

The Jason.Encoder derivations were flagged in a previous review. The OpenTelemetry instrumentation doesn't appear to serialize these structs to JSON—span attributes use atomic values directly. If these derivations are indeed unused, consider removing them to keep the PR focused.

test/opentelemetry/event_store_test.exs (2)

8-8: Verify Commanded.OpenTelemetryCase module name matches the actual definition.

A previous review flagged a potential mismatch between Commanded.OpenTelemetryCase used here and Commanded.OpenTelemetry.Case in the support file. Ensure the module names are consistent.

---

15-26: @events list is duplicated across multiple files.

This list is duplicated in the implementation module and test files. A previous review suggested consolidating this to reduce maintenance burden when adding new event types.

lib/commanded/opentelemetry/event_store.ex (3)

13-24: @events list is duplicated across multiple files.

This list is duplicated in the test file and factory test file. Consider exposing this as a public function or module attribute to allow tests to reference it directly.

---

26-42: Missing event store telemetry handler cleanup in shared test case.

A previous review noted that OpenTelemetryCase doesn't include event_store events in its detachment list. The tests work around this with per-test detach_handlers, but adding cleanup to the shared case would be more robust.

---

186-189: Inconsistent nil guard in extract_source_uuid.

The first clause guards against nil with when not is_nil(uuid), but the second clause for nested snapshot data lacks this guard. This was flagged in a previous review and creates inconsistent behavior.

🧹 Nitpick comments (1)

test/opentelemetry/event_store_test.exs (1)

359-365: Accessing internal tuple structures of OTel events is fragile.

The test relies on the internal structure of OpenTelemetry event tuples ({:event, _timestamp, :exception, attrs_tuple} and {:attributes, _, _, _, attrs_map}). These internal representations may change between library versions.

Consider using the public API or record accessors if available, or add a comment acknowledging this coupling to internal structures.

[cursor]

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