#57 — Scope `MeteredHandler` metrics to top-level requests (tech debt) #57

New issue

Closed

opened 2026-08-18 13:13:20 +02:00 by lena · 1 comment

lena commented

2026-08-18 13:13:20 +02:00

(Migrated from git.butzei.de)

Feature `#57` — Scope `MeteredHandler` metrics to top-level requests (tech debt)

Requested by: Backend Engineer, identified during #56 code review, 2026-07-20
Priority: Could
Blocked by: none

Problem

MeteredHandler<TRequest, TResult> (CqsTodo/Decorators/MeteredHandler.cs) is registered globally
via services.Decorate(typeof(IHandler<,>), typeof(MeteredHandler<,>)) in Program.cs, so it wraps
every closed IHandler<,> — including internal plumbing handlers that a top-level handler calls
directly, e.g. AuthorizeTodoListAccessForCurrentUserQuery, AuthorizeTodoListIsNotArchivedQuery,
GetCurrentUserIdQuery.

Effect: one user-facing API call (e.g. CheckTodoCommand) emits 2-4 separate
cqstodo.commands.executed increments and duration-histogram samples — one for the business
command, plus one for each internal authorization/lookup query it happens to call through
IHandler<,>. A dashboard built on cqstodo.commands.executed expecting "one sample per API
operation" over-counts by 2-4x for anything that goes through the standard authorization chain,
and cqstodo.commands.duration mixes full-request latency with sub-millisecond internal-lookup
latency under the same instrument name.

Separately: auth-rejected calls still increment CommandsExecuted/CommandDuration (the decorator's
finally runs even when decoratee.Handle throws), but — unlike EmailsSent/PushDispatched — carry
no outcome tag, so a rejected/unauthorized call is indistinguishable from a successful one in the
metric.

Scope

Decide the fix at the right altitude: either (a) exclude the internal Authorize*QueryHandler /
GetCurrentUserIdQueryHandler-style plumbing handlers from decoration (e.g. an opt-out marker
interface or attribute), or (b) tag metrics with enough context (e.g. a "kind=internal" vs.
"kind=api" tag, or requiring the top-level request type) to make dashboards filterable without a
hardcoded allowlist.
Add an outcome tag (e.g. success/unauthorized/error) to CommandsExecuted/CommandDuration
so auth rejections are visible and distinguishable from successful invocations, consistent with how
EmailsSent/PushDispatched already do this.

Out of scope

Adopting [LoggerMessage] source generators for hot-path handlers (also flagged in the #56 review
as not yet applied despite the story's own design doc recommending it) — track separately if it
becomes a measured performance concern; not addressed by this story.

Agents involved

Architect — decide the decoration-scoping mechanism
Backend Engineer — implement
QA Agent — verify metric cardinality/tagging with a manual Seq/OTel check

Resolution (2026-07-21)

Full design/rationale in the sibling 57_metrics_decorator_scope_tech_debt_design.md. Summary:

(a) chosen, not (b): a new IInternalRequest marker interface (Common/Cqs/IInternalRequest.cs)
applied directly to each internal-plumbing request type's own declaration. MeteredHandler checks
it against TRequest (not the wrapped handler instance — see design doc for why that distinction
matters, caught during implementation via RevokeAllSessionsForCurrentUserCommand, the one type in
the audited set with its own WithAuthorization). 19 request types marked across both projects —
16 in the first pass, plus 3 more (CreateActivityEventCommand, CreateNotificationCommand,
SendEmailCommand) that a grep-based candidate search missed due to their multi-line primary
constructors, caught by /code-review before archiving. See the design doc for the full list and
the new regression test that now pins it down.
outcome tag (success/unauthorized/error) added to CommandsExecuted/CommandDuration,
matching the EmailsSent/PushDispatched pattern. unauthorized = UnauthorizedAccessException
or AuthenticationException (the same two types ExceptionHandler.cs maps to 401/403), error =
anything else. Exceptions always rethrown — the decorator only observes.
New CqsTodo.Tests/Decorators/MeteredHandlerTests.cs (no prior coverage existed for
MeteredHandler at all) — uses a real System.Diagnostics.Metrics.MeterListener against a real
CqsTodoMetrics instance to assert actually-emitted measurements, not mocked call counts. Runs with
no Docker dependency.
QA verification was via those automated MeterListener-based tests rather than a manual live
Seq/OTel check (no OTel collector/Seq instance available in this sandbox) — the tests assert the
same thing a manual dashboard check would (zero samples for internal requests, exactly one
counter+histogram sample per top-level call, correct outcome tag per exception type), just
repeatably and in CI.
Spotted but explicitly deferred (kept this diff scoped to the story): GetCurrentUserIdQuery is the
one public internal-only request type not in EndpointRouteBuilderExtensions's HTTP exclusion
list — a different, lower-severity instance of the same category of bug fixed in #59's review.
Drafted as docs/features/new/61_bug_getcurrentuseridquery_missing_from_endpoint_exclusion_list.md.

# Feature `#57` — Scope `MeteredHandler` metrics to top-level requests (tech debt) **Requested by:** Backend Engineer, identified during `#56` code review, 2026-07-20 **Priority:** Could **Blocked by:** none --- ## Problem `MeteredHandler<TRequest, TResult>` (`CqsTodo/Decorators/MeteredHandler.cs`) is registered globally via `services.Decorate(typeof(IHandler<,>), typeof(MeteredHandler<,>))` in `Program.cs`, so it wraps every closed `IHandler<,>` — including internal plumbing handlers that a top-level handler calls directly, e.g. `AuthorizeTodoListAccessForCurrentUserQuery`, `AuthorizeTodoListIsNotArchivedQuery`, `GetCurrentUserIdQuery`. Effect: one user-facing API call (e.g. `CheckTodoCommand`) emits 2-4 separate `cqstodo.commands.executed` increments and duration-histogram samples — one for the business command, plus one for each internal authorization/lookup query it happens to call through `IHandler<,>`. A dashboard built on `cqstodo.commands.executed` expecting "one sample per API operation" over-counts by 2-4x for anything that goes through the standard authorization chain, and `cqstodo.commands.duration` mixes full-request latency with sub-millisecond internal-lookup latency under the same instrument name. Separately: auth-rejected calls still increment `CommandsExecuted`/`CommandDuration` (the decorator's `finally` runs even when `decoratee.Handle` throws), but — unlike `EmailsSent`/`PushDispatched` — carry no `outcome` tag, so a rejected/unauthorized call is indistinguishable from a successful one in the metric. ## Scope - Decide the fix at the right altitude: either (a) exclude the internal `Authorize*QueryHandler` / `GetCurrentUserIdQueryHandler`-style plumbing handlers from decoration (e.g. an opt-out marker interface or attribute), or (b) tag metrics with enough context (e.g. a "kind=internal" vs. "kind=api" tag, or requiring the top-level request type) to make dashboards filterable without a hardcoded allowlist. - Add an `outcome` tag (e.g. `success`/`unauthorized`/`error`) to `CommandsExecuted`/`CommandDuration` so auth rejections are visible and distinguishable from successful invocations, consistent with how `EmailsSent`/`PushDispatched` already do this. ## Out of scope - Adopting `[LoggerMessage]` source generators for hot-path handlers (also flagged in the `#56` review as not yet applied despite the story's own design doc recommending it) — track separately if it becomes a measured performance concern; not addressed by this story. ## Agents involved - **Architect** — decide the decoration-scoping mechanism - **Backend Engineer** — implement - **QA Agent** — verify metric cardinality/tagging with a manual Seq/OTel check --- ## Resolution (2026-07-21) Full design/rationale in the sibling `57_metrics_decorator_scope_tech_debt_design.md`. Summary: - **(a) chosen**, not (b): a new `IInternalRequest` marker interface (`Common/Cqs/IInternalRequest.cs`) applied directly to each internal-plumbing request type's own declaration. `MeteredHandler` checks it against `TRequest` (not the wrapped handler instance — see design doc for why that distinction matters, caught during implementation via `RevokeAllSessionsForCurrentUserCommand`, the one type in the audited set with its own `WithAuthorization`). 19 request types marked across both projects — 16 in the first pass, plus 3 more (`CreateActivityEventCommand`, `CreateNotificationCommand`, `SendEmailCommand`) that a grep-based candidate search missed due to their multi-line primary constructors, caught by `/code-review` before archiving. See the design doc for the full list and the new regression test that now pins it down. - `outcome` tag (`success`/`unauthorized`/`error`) added to `CommandsExecuted`/`CommandDuration`, matching the `EmailsSent`/`PushDispatched` pattern. `unauthorized` = `UnauthorizedAccessException` or `AuthenticationException` (the same two types `ExceptionHandler.cs` maps to 401/403), `error` = anything else. Exceptions always rethrown — the decorator only observes. - New `CqsTodo.Tests/Decorators/MeteredHandlerTests.cs` (no prior coverage existed for `MeteredHandler` at all) — uses a real `System.Diagnostics.Metrics.MeterListener` against a real `CqsTodoMetrics` instance to assert actually-emitted measurements, not mocked call counts. Runs with no Docker dependency. - QA verification was via those automated `MeterListener`-based tests rather than a manual live Seq/OTel check (no OTel collector/Seq instance available in this sandbox) — the tests assert the same thing a manual dashboard check would (zero samples for internal requests, exactly one counter+histogram sample per top-level call, correct `outcome` tag per exception type), just repeatably and in CI. - Spotted but explicitly deferred (kept this diff scoped to the story): `GetCurrentUserIdQuery` is the one public internal-only request type *not* in `EndpointRouteBuilderExtensions`'s HTTP exclusion list — a different, lower-severity instance of the same category of bug fixed in `#59`'s review. Drafted as `docs/features/new/61_bug_getcurrentuseridquery_missing_from_endpoint_exclusion_list.md`.

lena commented

2026-08-18 13:13:21 +02:00

(Migrated from git.butzei.de)

design (57_metrics_decorator_scope_tech_debt_design.md)

Design: `#57` — Scope `MeteredHandler` metrics to top-level requests

Current state (confirmed by reading the code)

MeteredHandler<TRequest,TResult> (CqsTodo/Decorators/MeteredHandler.cs) is applied globally via
builder.Services.Decorate(typeof(IHandler<,>), typeof(MeteredHandler<,>)) in Program.cs. Decorate
wraps every distinct closed IHandler<TReq,TRes> registration in the container — not just the
handlers reachable as HTTP endpoints. Two mechanisms cause a single user-facing API call to fan out
into several separately-metered handler invocations:

WithAuthorization's AuthorizationAndValidationDecorator<TRequest,TResult,TAuthCommand>
resolves its authHandler dependency as IHandler<TAuthCommand, Unit> — a distinct closed-generic
registration from the outer IHandler<TRequest,TResult> it decorates, and therefore wrapped by its
own separate MeteredHandler instance. E.g. DeleteCurrentUserAccountCommand (two
WithAuthorization calls) emits its own sample plus one sample each for
AuthorizeIsCurrentUserAuthenticatedQuery and AuthorizeCurrentPasswordQuery.
Plumbing handlers invoked directly as a constructor-injected IHandler<,> dependency from
inside another handler's Handle body — e.g. GetCurrentUserIdQueryHandler (used by nearly every
handler to resolve the current user), GetTodoQueryHandler (re-fetches a DTO after a mutation for
change-broadcast), HashPasswordQueryHandler/GeneratePasswordSaltQueryHandler — each such call
inside a top-level handler's execution also gets its own MeteredHandler wrapper.

Confirmed not in scope: CreateDueNotificationsCommand and PurgeStalePushSubscriptionsCommand
(DueNotificationJob, a background job) are each invoked directly by the job's own scheduling loop,
not chained from within another CQS handler — each genuinely is its own independent, periodically-run
unit of work, so keeping them separately metered is correct and desired, not part of the over-counting
problem.

Decision — option (a): opt-out marker interface on the request type, checked against `TRequest`

public interface IInternalRequest : IRequest; — an empty marker, added to
Common/Cqs/IInternalRequest.cs (same folder as IHandler<,>/IRequest in IRequest.cs;
Common.Cqs is already a solution-wide implicit using via Directory.Build.props, so every
project picks it up with no new using directives needed).

MeteredHandler<TRequest,TResult> checks a private static readonly bool IsInternalRequest = typeof(TRequest).IsAssignableTo(typeof(IInternalRequest)) — computed once per closed generic
instantiation, not per call — and if true, calls straight through with zero metrics recorded: no
stopwatch, no counter/histogram writes.

Marked on the request type, not the handler class — this matters, not just style. An earlier
draft of this design put the marker on the handler class instead, checked via
decoratee is IInternalHandler inside MeteredHandler.Handle. That breaks for any handler that also
has its own WithAuthorization: SetupHandler's ConfigureDi step (which applies
WithAuthorization's AuthorizationAndValidationDecorator) runs before Program.cs's single global
Decorate(typeof(IHandler<,>), typeof(MeteredHandler<,>)) call, so by the time MeteredHandler wraps
that closed IHandler<,>, the decoratee it receives is the AuthorizationAndValidationDecorator
instance, not the marked handler underneath — an instance-based marker on the handler is invisible
through that extra layer. Caught by tracing RevokeAllSessionsForCurrentUserCommandHandler (the one
handler in the audited set below with its own WithAuthorization) through the actual decoration
order in Program.cs before implementing, not after. Checking TRequest instead sidesteps the
problem entirely — the request type is fixed at the generic-instantiation level and is completely
unaffected by how many decorators end up wrapping the handler instance at runtime.

Rejected alternative — attribute ([InternalHandler]) checked via GetCustomAttribute at
Handle()-time: functionally equivalent once also moved to the request type, but needs its own cache
to avoid repeated reflection; the interface check is a plain static bool field, simpler for the same
outcome.

Rejected alternative — reuse EndpointRouteBuilderExtensions's existing internal-only exclusion list
(also request-type based, used to keep certain commands off HTTP): that list encodes a different,
narrower concern ("must not be a POST endpoint") that happens to overlap partially with "is plumbing
for metrics purposes" but isn't the same set — e.g. GetTodoQuery is already internal (naturally
excluded from HTTP mapping by the IsPublic filter, was never on that list), while conversely a type
can need public visibility for cross-assembly reasons (as #59's session commands do) without that
bearing on whether it's a top-level operation. A dedicated marker declared directly on each request
type, at its own declaration site, keeps the "is this plumbing" decision next to the type it
describes instead of accumulating in a second list that must be kept in sync by hand — precisely the
fragility already called out as accepted debt in the #59 review ([[05_security_agent_memory]]).

Request types marked IInternalRequest (audited by tracing each one's only call sites — confirmed
each is dispatched exclusively from inside another handler's Handle body, or from
SessionMaintenanceMiddleware on every request, never as its own top-level/HTTP/scheduled operation):

CqsTodo project: AuthorizeTodoListAccessForCurrentUserQuery, AuthorizeCurrentPasswordQuery,
AuthorizeIsUserQuery, AuthorizeTodoListOwnerAccessForCurrentUserQuery,
AuthorizeTodoListIsNotArchivedQuery, AuthorizeIsEmailVerifiedQuery,
AuthorizeIsCurrentUserAuthenticatedQuery, GetTodoQuery, HashPasswordQuery,
GeneratePasswordSaltQuery, GetSessionsRevokedBeforeUtcQuery,
RevokeAllSessionsForCurrentUserCommand, SendEmailVerificationCommand, GetCurrentUserIdQuery,
SetCurrentUserIdCommand, DestroyCurrentSessionCommand (the last three defined in CqsTodo but
handled in CqsTodo.WebApi, since their handlers need IHttpContextAccessor/ISession),
CreateActivityEventCommand, CreateNotificationCommand, SendEmailCommand.

The first audit pass missed the last three. The initial grep-based sweep for candidates
(internal record.*: IRequest on one line) doesn't match a multi-line primary constructor —
CreateActivityEventCommand, CreateNotificationCommand, and SendEmailCommand all declare their
constructor parameters across several lines with : IRequest<...> on a later line, so they were
invisible to that grep and got skipped in the first implementation pass. Caught by a
/code-review correctness-angle pass (not by the implementing agent's own audit) before this story
was archived. Closed two ways: (1) fixed — all three now carry IInternalRequest; (2) a new
CqsTodo.Tests/Decorators/MeteredHandlerTests.cs test
(KnownInternalRequestType_ImplementsIInternalRequest, [TestCaseSource] over the full 19-type list
above) pins this exact set down, so if one of these 19 ever loses the marker, CI fails — though it
can't catch a brand new internal-only type that never gets added to the list in the first place;
that's still a manual step, same residual risk already accepted for the marker mechanism itself (see
above) and for the EndpointRouteBuilderExtensions exclusion list before it.

Two internal-visibility request types were confirmed not internal-plumbing and deliberately left
unmarked: CreateDueNotificationsCommand and PurgeStalePushSubscriptionsCommand (both dispatched
directly by DueNotificationJob's own scheduling loop, not chained from another handler — each is
genuinely its own independent, periodically-run unit of work, so keeping them separately metered is
correct).

Decision — `outcome` tag on `CommandsExecuted`/`CommandDuration`

Matches the existing EmailsSent/PushDispatched pattern (outcome tag, string values). For the one
remaining metered sample per top-level call (the composed handler, including any chained
WithAuthorization steps that now run inside it without their own separate sample), classify:

success — no exception.
unauthorized — UnauthorizedAccessException or System.Security.Authentication.AuthenticationException
escapes. These are the two exception types the existing Authorize*QueryHandler family and
password-verification handlers consistently throw for auth rejections (confirmed by grep across
CqsTodo/Features/Authorization/*.cs), and are exactly the two types
CqsTodo.WebApi/ExceptionHandler.cs maps to 401/403 today — reusing the same two types keeps the
metric's "unauthorized" bucket aligned with what the API actually returns as an auth failure,
without duplicating ExceptionHandler's full status-code switch (that lives in the WebApi project;
MeteredHandler lives in the core project and has no dependency on it).
error — anything else (EntityNotFoundException, validation exceptions, unexpected 500s). Not
split further — the story asks for a 3-bucket success/unauthorized/error scheme, not a full
status-code taxonomy; a finer split can be a future story if a real dashboarding need shows up.

The exception is always rethrown — the decorator only observes and tags, never swallows.

Testing approach

New CqsTodo.Tests/Decorators/MeteredHandlerTests.cs (no existing coverage for MeteredHandler at
all — gap predates this story, from #56). CqsTodoMetrics is a concrete sealed class wrapping a real
System.Diagnostics.Metrics.Meter; Moq can't intercept Counter<T>.Add/Histogram<T>.Record (not
virtual), so tests use a real CqsTodoMetrics instance plus a System.Diagnostics.Metrics.MeterListener
subscribed to "CqsTodo" to capture actually-recorded measurements — verifying real emitted values
rather than mocked call counts. Covers: an IInternalRequest-marked request type records zero
measurements; a non-marked request type records exactly one CommandsExecuted/CommandDuration pair
tagged outcome=success on success; outcome=unauthorized for UnauthorizedAccessException /
AuthenticationException (exception still propagates); outcome=error for any other exception
(propagates); no Docker dependency, runs everywhere including this sandbox.

Out of scope (per story)

[LoggerMessage] source generators — tracked separately, not touched here.

**design** (`57_metrics_decorator_scope_tech_debt_design.md`) # Design: `#57` — Scope `MeteredHandler` metrics to top-level requests ## Current state (confirmed by reading the code) `MeteredHandler<TRequest,TResult>` (`CqsTodo/Decorators/MeteredHandler.cs`) is applied globally via `builder.Services.Decorate(typeof(IHandler<,>), typeof(MeteredHandler<,>))` in `Program.cs`. `Decorate` wraps **every** distinct closed `IHandler<TReq,TRes>` registration in the container — not just the handlers reachable as HTTP endpoints. Two mechanisms cause a single user-facing API call to fan out into several separately-metered handler invocations: 1. **`WithAuthorization`'s `AuthorizationAndValidationDecorator<TRequest,TResult,TAuthCommand>`** resolves its `authHandler` dependency as `IHandler<TAuthCommand, Unit>` — a distinct closed-generic registration from the outer `IHandler<TRequest,TResult>` it decorates, and therefore wrapped by its own separate `MeteredHandler` instance. E.g. `DeleteCurrentUserAccountCommand` (two `WithAuthorization` calls) emits its own sample plus one sample each for `AuthorizeIsCurrentUserAuthenticatedQuery` and `AuthorizeCurrentPasswordQuery`. 2. **Plumbing handlers invoked directly as a constructor-injected `IHandler<,>` dependency from inside another handler's `Handle` body** — e.g. `GetCurrentUserIdQueryHandler` (used by nearly every handler to resolve the current user), `GetTodoQueryHandler` (re-fetches a DTO after a mutation for change-broadcast), `HashPasswordQueryHandler`/`GeneratePasswordSaltQueryHandler` — each such call inside a top-level handler's execution also gets its own `MeteredHandler` wrapper. Confirmed *not* in scope: `CreateDueNotificationsCommand` and `PurgeStalePushSubscriptionsCommand` (`DueNotificationJob`, a background job) are each invoked directly by the job's own scheduling loop, not chained from within another CQS handler — each genuinely is its own independent, periodically-run unit of work, so keeping them separately metered is correct and desired, not part of the over-counting problem. ## Decision — option (a): opt-out marker interface on the *request type*, checked against `TRequest` `public interface IInternalRequest : IRequest;` — an empty marker, added to `Common/Cqs/IInternalRequest.cs` (same folder as `IHandler<,>`/`IRequest` in `IRequest.cs`; `Common.Cqs` is already a solution-wide implicit `using` via `Directory.Build.props`, so every project picks it up with no new `using` directives needed). `MeteredHandler<TRequest,TResult>` checks a `private static readonly bool IsInternalRequest = typeof(TRequest).IsAssignableTo(typeof(IInternalRequest))` — computed once per closed generic instantiation, not per call — and if true, calls straight through with **zero** metrics recorded: no stopwatch, no counter/histogram writes. **Marked on the request type, not the handler class — this matters, not just style.** An earlier draft of this design put the marker on the handler class instead, checked via `decoratee is IInternalHandler` inside `MeteredHandler.Handle`. That breaks for any handler that also has its own `WithAuthorization`: `SetupHandler`'s `ConfigureDi` step (which applies `WithAuthorization`'s `AuthorizationAndValidationDecorator`) runs before `Program.cs`'s single global `Decorate(typeof(IHandler<,>), typeof(MeteredHandler<,>))` call, so by the time `MeteredHandler` wraps that closed `IHandler<,>`, the `decoratee` it receives is the `AuthorizationAndValidationDecorator` instance, not the marked handler underneath — an instance-based marker on the handler is invisible through that extra layer. Caught by tracing `RevokeAllSessionsForCurrentUserCommandHandler` (the one handler in the audited set below with its own `WithAuthorization`) through the actual decoration order in `Program.cs` before implementing, not after. Checking `TRequest` instead sidesteps the problem entirely — the request type is fixed at the generic-instantiation level and is completely unaffected by how many decorators end up wrapping the handler instance at runtime. Rejected alternative — attribute (`[InternalHandler]`) checked via `GetCustomAttribute` at `Handle()`-time: functionally equivalent once also moved to the request type, but needs its own cache to avoid repeated reflection; the interface check is a plain static bool field, simpler for the same outcome. Rejected alternative — reuse `EndpointRouteBuilderExtensions`'s existing internal-only exclusion list (also request-type based, used to keep certain commands off HTTP): that list encodes a different, narrower concern ("must not be a `POST` endpoint") that happens to overlap partially with "is plumbing for metrics purposes" but isn't the same set — e.g. `GetTodoQuery` is already `internal` (naturally excluded from HTTP mapping by the `IsPublic` filter, was never on that list), while conversely a type can need public visibility for cross-assembly reasons (as `#59`'s session commands do) without that bearing on whether it's a top-level *operation*. A dedicated marker declared directly on each request type, at its own declaration site, keeps the "is this plumbing" decision next to the type it describes instead of accumulating in a second list that must be kept in sync by hand — precisely the fragility already called out as accepted debt in the `#59` review (`[[05_security_agent_memory]]`). **Request types marked `IInternalRequest`** (audited by tracing each one's only call sites — confirmed each is dispatched exclusively from inside another handler's `Handle` body, or from `SessionMaintenanceMiddleware` on every request, never as its own top-level/HTTP/scheduled operation): `CqsTodo` project: `AuthorizeTodoListAccessForCurrentUserQuery`, `AuthorizeCurrentPasswordQuery`, `AuthorizeIsUserQuery`, `AuthorizeTodoListOwnerAccessForCurrentUserQuery`, `AuthorizeTodoListIsNotArchivedQuery`, `AuthorizeIsEmailVerifiedQuery`, `AuthorizeIsCurrentUserAuthenticatedQuery`, `GetTodoQuery`, `HashPasswordQuery`, `GeneratePasswordSaltQuery`, `GetSessionsRevokedBeforeUtcQuery`, `RevokeAllSessionsForCurrentUserCommand`, `SendEmailVerificationCommand`, `GetCurrentUserIdQuery`, `SetCurrentUserIdCommand`, `DestroyCurrentSessionCommand` (the last three defined in `CqsTodo` but handled in `CqsTodo.WebApi`, since their handlers need `IHttpContextAccessor`/`ISession`), `CreateActivityEventCommand`, `CreateNotificationCommand`, `SendEmailCommand`. **The first audit pass missed the last three.** The initial grep-based sweep for candidates (`internal record.*: IRequest` on one line) doesn't match a multi-line primary constructor — `CreateActivityEventCommand`, `CreateNotificationCommand`, and `SendEmailCommand` all declare their constructor parameters across several lines with `: IRequest<...>` on a later line, so they were invisible to that grep and got skipped in the first implementation pass. Caught by a `/code-review` correctness-angle pass (not by the implementing agent's own audit) before this story was archived. Closed two ways: (1) fixed — all three now carry `IInternalRequest`; (2) a new `CqsTodo.Tests/Decorators/MeteredHandlerTests.cs` test (`KnownInternalRequestType_ImplementsIInternalRequest`, `[TestCaseSource]` over the full 19-type list above) pins this exact set down, so if one of these 19 ever loses the marker, CI fails — though it can't catch a *brand new* internal-only type that never gets added to the list in the first place; that's still a manual step, same residual risk already accepted for the marker mechanism itself (see above) and for the `EndpointRouteBuilderExtensions` exclusion list before it. Two `internal`-visibility request types were confirmed **not** internal-plumbing and deliberately left unmarked: `CreateDueNotificationsCommand` and `PurgeStalePushSubscriptionsCommand` (both dispatched directly by `DueNotificationJob`'s own scheduling loop, not chained from another handler — each is genuinely its own independent, periodically-run unit of work, so keeping them separately metered is correct). ## Decision — `outcome` tag on `CommandsExecuted`/`CommandDuration` Matches the existing `EmailsSent`/`PushDispatched` pattern (`outcome` tag, string values). For the one remaining metered sample per top-level call (the composed handler, including any chained `WithAuthorization` steps that now run *inside* it without their own separate sample), classify: - `success` — no exception. - `unauthorized` — `UnauthorizedAccessException` or `System.Security.Authentication.AuthenticationException` escapes. These are the two exception types the existing `Authorize*QueryHandler` family and password-verification handlers consistently throw for auth rejections (confirmed by grep across `CqsTodo/Features/Authorization/*.cs`), and are exactly the two types `CqsTodo.WebApi/ExceptionHandler.cs` maps to 401/403 today — reusing the same two types keeps the metric's "unauthorized" bucket aligned with what the API actually returns as an auth failure, without duplicating `ExceptionHandler`'s full status-code switch (that lives in the WebApi project; `MeteredHandler` lives in the core project and has no dependency on it). - `error` — anything else (`EntityNotFoundException`, validation exceptions, unexpected 500s). Not split further — the story asks for a 3-bucket `success`/`unauthorized`/`error` scheme, not a full status-code taxonomy; a finer split can be a future story if a real dashboarding need shows up. The exception is always rethrown — the decorator only observes and tags, never swallows. ## Testing approach New `CqsTodo.Tests/Decorators/MeteredHandlerTests.cs` (no existing coverage for `MeteredHandler` at all — gap predates this story, from `#56`). `CqsTodoMetrics` is a concrete sealed class wrapping a real `System.Diagnostics.Metrics.Meter`; Moq can't intercept `Counter<T>.Add`/`Histogram<T>.Record` (not virtual), so tests use a real `CqsTodoMetrics` instance plus a `System.Diagnostics.Metrics.MeterListener` subscribed to `"CqsTodo"` to capture actually-recorded measurements — verifying real emitted values rather than mocked call counts. Covers: an `IInternalRequest`-marked request type records zero measurements; a non-marked request type records exactly one `CommandsExecuted`/`CommandDuration` pair tagged `outcome=success` on success; `outcome=unauthorized` for `UnauthorizedAccessException` / `AuthenticationException` (exception still propagates); `outcome=error` for any other exception (propagates); no Docker dependency, runs everywhere including this sandbox. ## Out of scope (per story) `[LoggerMessage]` source generators — tracked separately, not touched here.