Skip to content

fix(telemetry): rename commanded.handler.lag to commanded.handler.processing_latency - #95

Merged
yordis merged 1 commit into
mainfrom
yordis/otel-lag
Apr 27, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/otel-lag

Conversation

@yordis

@yordis yordis commented Apr 27, 2026

Copy link
Copy Markdown
Member
  • "Lag" in messaging systems (Kafka, Pulsar, NATS, RabbitMQ) universally means offset-based distance (how many messages behind the stream head), not a duration. The existing commanded.handler.lag attribute measured delivery latency (event age at processing time), making the name misleading for anyone familiar with messaging conventions.
  • Establishes a clear naming convention: "latency" for time-based durations, "lag" reserved for future offset-based metrics (e.g., commanded.subscription.lag).

@cursor

cursor Bot commented Apr 27, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Medium risk because it changes the emitted OpenTelemetry attribute key, which can break downstream dashboards/alerts and any consumers expecting commanded.handler.lag, though runtime behavior is otherwise unchanged.

Overview
Renames the event-handler delivery-latency span attribute from commanded.handler.lag to commanded.handler.processing_latency and updates the event handler telemetry code to emit the new key for both single-event and batch spans.

Expands CommandedAttributes documentation to clearly distinguish latency (time duration) from lag (offset distance) and updates tests to assert the renamed attribute.

Reviewed by Cursor Bugbot for commit 7231c7a. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Apr 27, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@yordis has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 27 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 825cb40e-5750-497c-b4b3-6458f7320e23

📥 Commits

Reviewing files that changed from the base of the PR and between 6dae93d and 7231c7a.

📒 Files selected for processing (3)
  • lib/commanded/opentelemetry/commanded_attributes.ex
  • lib/commanded/opentelemetry/event_handler.ex
  • test/opentelemetry/event_handler_test.exs

Walkthrough

The pull request renames the OpenTelemetry span attribute from commanded.handler.lag to commanded.handler.processing_latency, updates documentation to clarify the distinction between lag (offset-based metrics) and latency (time durations), and modifies the event handler telemetry recording logic accordingly.

Changes

Cohort / File(s) Summary
Documentation
guides/howtos/setting-up-opentelemetry-tracing.md
Added detailed documentation of Commanded-specific OpenTelemetry span attributes for event handler spans, including the new commanded.handler.processing_latency attribute and clarification distinguishing event processing latency from consumer lag terminology.
OpenTelemetry Attributes
lib/commanded/opentelemetry/commanded_attributes.ex
Renamed public function commanded_handler_lag/0 to commanded_handler_processing_latency/0 with updated constant from :"commanded.handler.lag" to :"commanded.handler.processing_latency". Added "Latency vs Lag" naming rule documentation reserving "lag" for offset-based metrics and "latency" for time durations.
Event Handler Telemetry
lib/commanded/opentelemetry/event_handler.ex
Updated telemetry span attribute recording to use put_handler_processing_latency/2 instead of put_handler_lag/2 for both handle-stop and batch-stop event handlers.
Tests
test/opentelemetry/event_handler_test.exs
Updated expected OpenTelemetry span attribute assertions to use the new key "commanded.handler.processing_latency" instead of "commanded.handler.lag".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 Lag and latency, once confused,
Now clearly defined, no longer bruised!
We renamed the span with care and grace,
Offset and time each in their place. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: renaming the telemetry attribute from commanded.handler.lag to commanded.handler.processing_latency, which is the primary purpose of this PR.
Description check ✅ Passed The description clearly explains the rationale for the change, distinguishing between offset-based lag and time-based latency in messaging systems, and establishing naming conventions.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yordis/otel-lag

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.

@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: 1

🧹 Nitpick comments (1)
guides/howtos/setting-up-opentelemetry-tracing.md (1)

103-112: Optional: the attributes table is incomplete.

Single-event handler spans also emit commanded.event and commanded.event.number (see lib/commanded/opentelemetry/event_handler.ex lines 92–93), which aren't listed here. Not strictly part of this PR's scope, but since you're establishing this table, it's a good moment to round it out so users have a single source of truth.

Suggested addition
 | `commanded.handler.name` | string | The handler module name |
 | `commanded.handler.kind` | string | Type of handler (`event_handler`) |
+| `commanded.event` | string | The event struct name being processed |
+| `commanded.event.number` | integer | The event's global sequence number |
 | `commanded.stream.id` | string | The event's stream identifier |
 | `commanded.stream.version` | integer | The event's stream version |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@guides/howtos/setting-up-opentelemetry-tracing.md` around lines 103 - 112,
Add the two missing attributes to the attributes table: include
`commanded.event` (type: string) and `commanded.event.number` (type: integer)
with descriptions like "The event module/name emitted by the handler" and "The
ordinal number/index of the event within a single-event handler", respectively;
reference the emission in lib/commanded/opentelemetry/event_handler.ex (lines
~92–93) to ensure wording matches the actual emitted fields and append these
rows to the existing table so the table becomes a complete
single-source-of-truth for handler span attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/commanded/opentelemetry/commanded_attributes.ex`:
- Around line 189-203: Add a breaking-change note to CHANGELOG.md documenting
the telemetry attribute rename from :commanded.handler.lag to
:commanded.handler.processing_latency (exposed by the function
commanded_handler_processing_latency/0) and describe the impact on dashboards,
alerts, and span filters so integrators know to update their OTel consumers when
upgrading; place the entry under the current release section and mark it as a
breaking change with migration guidance to replace the old key with the new key.

---

Nitpick comments:
In `@guides/howtos/setting-up-opentelemetry-tracing.md`:
- Around line 103-112: Add the two missing attributes to the attributes table:
include `commanded.event` (type: string) and `commanded.event.number` (type:
integer) with descriptions like "The event module/name emitted by the handler"
and "The ordinal number/index of the event within a single-event handler",
respectively; reference the emission in
lib/commanded/opentelemetry/event_handler.ex (lines ~92–93) to ensure wording
matches the actual emitted fields and append these rows to the existing table so
the table becomes a complete single-source-of-truth for handler span attributes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b814d732-8de1-42ed-a22d-9ea575b5ed70

📥 Commits

Reviewing files that changed from the base of the PR and between 7b11e11 and 6dae93d.

📒 Files selected for processing (4)
  • guides/howtos/setting-up-opentelemetry-tracing.md
  • lib/commanded/opentelemetry/commanded_attributes.ex
  • lib/commanded/opentelemetry/event_handler.ex
  • test/opentelemetry/event_handler_test.exs

Comment thread lib/commanded/opentelemetry/commanded_attributes.ex
…cessing_latency

- "Lag" in messaging systems (Kafka, Pulsar, NATS) universally means offset-based
  distance, not a duration. The existing attribute measured delivery latency
  (event age at processing time), so the name was misleading.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis merged commit 8198884 into main Apr 27, 2026
3 checks passed
@yordis
yordis deleted the yordis/otel-lag branch April 27, 2026 18:27
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