Backport caller-side standalone Nexus operation metrics (emitted from lib in CHASM) to the HSM (in-workflow) implementation

[tekkaya]

What changed?

Backport the caller-side Nexus operation metrics from the CHASM implementation to the legacy HSM (in-workflow) implementation, so in-workflow Nexus operations emit the same metrics standalone (CHASM) operations already do: the nexus_operation_success/_fail/_cancel/_timeout counters and the schedule_to_close/schedule_to_start/start_to_close latency histograms.

Three commits:

1. HSM NodeBackend accessors — declare the existing GetWorkflowType() / GetNamespaceEntry() on hsm.NodeBackend (+ Node.WorkflowTypeName() / NamespaceName()), so components/nexusoperations can read the parent workflow type/namespace without importing service/history/workflow (a dependency cycle).
2. Executor-path emission — new components/nexusoperations/metrics.go reusing the chasm/lib/nexusoperation metric defs. Emitted at the once-only live executor sites (sync success, async start, sync/non-retryable failure & cancellation, timeout timers), never in *EventDefinition.Apply (see double-counting note below).
3. Completion-path emission — the async completion handler (an fx-provided struct that injects the metrics handler + tag config into the history handler) emits the terminal outcome + schedule-to-start; adds functional coverage for both engines.

Why?

In-workflow (HSM) Nexus operations had no caller-side metrics while standalone (CHASM) operations did. This brings them to parity ahead of the HSM→CHASM migration.

Why emit at the live executor/completion sites, not in *EventDefinition.Apply?

The executor and completion handlers run once, on the active cluster, for a given operation — the natural place to count a terminal outcome. EventDefinition.Apply, by contrast, re-runs the state transition in paths where we must not re-count:

• Workflow reset — always rebuilds mutable state through mutable_state_rebuilder.ApplyEvents → EventDefinition.Apply, regardless of replication mode. This is the unconditional reason.
• Event-based replication — when history.enableTransitionHistory=false, a standby cluster replays history events through Apply. (With the default enableTransitionHistory=true, HSM state is replicated state-based via SyncHSMState, which bypasses Apply — so this case only applies in the non-default fallback mode.)

Emitting at the active live sites is therefore correct under both replication modes and on reset. These counters are best-effort: history task processing is at-least-once, so a transient retry before commit can re-emit — the same property the existing server-side activity completion metrics have.

How did you test it?

• built
• run locally and tested manually
• covered by existing tests
• added new unit test(s)
• added new functional test(s)

Unit: components/nexusoperations, chasm/lib/nexusoperation, service/history/hsm. Functional: TestNexusOperationCallerMetrics passes for both HSM and CHASM (in-process sqlite).

Potential risks

• Metrics-only change — no change to operation processing. Emission is confined to the active live sites (never *EventDefinition.Apply), so neither state-based nor event-based replication, nor workflow reset, double-counts a terminal outcome.
• HSM in-workflow ops have no terminate transition, so nexus_operation_terminate stays ~zero on the HSM path (documented asymmetry vs CHASM).

[bergundy]

Only blocking comment is on the impl tag, otherwise mostly LGTM. I'm less pedantic here since this is all deprecated code.

[bergundy]

It would be cleaner to check if the state machine is in started state already at the call site instead of propagating this boolean IMHO, separate concerns.

[tekkaya]

Updated

[bergundy]

This is a hacky way of doing things but it's fine given that this whole framework is deprecated.

[tekkaya]

I totally agree

[bergundy]

Instead of issuing these record call in every call site, it would have been more manageable to inject the metrics handler to the transition functions as we did with the CHASM implementation. It's way more contained and easy to reason about this way.

[tekkaya]

If my understanding is correct;

• HSM: the EventDefinition contract says Apply is "Triggered during replication and workflow reset", and our *EventDefinition.Apply run the transitions (FailedEventDefinition.Apply → transitionOperation → TransitionFailed.Apply). So the transitions can re-run when events are re-applied and we can emit metrics again on those paths.
• CHASM: replication is state-based, rebuilt is done via Node.ApplySnapshot/ApplyMutation applying serialized node state, so the transitions don't re-run there. Iiuc that's why emitting inside the CHASM transitions is safe.

This is the reason why metric emission is added to the executor/completion call sites. To move it into the transitions and reapply safely we will have to thread the metrics handler through the Event* structs and set it only on the live paths. Though given this code is deprecated I leaned toward the current approach but happy to go in the suggested direction if you prefer.

[tekkaya]

Also there is an alternative based on your comment to emit metrics from transition functions in this draft PR on based top of this one. If that approach is preferable I can go in that direction: #10823

[bergundy]

HSM is also state based nowadays. But the issue on workflow reset and conflict resolution exists in both implementations apparently. I am wondering if this is also a problem for example for workflow activities.

Good catch!

[tekkaya]

I tried to dig into this against how activities handle the same thing and I've also adjusted the code so the emission timing matches activities. Summary below.

Reset / conflict resolution

Both rebuild/reapply through Apply, which doesn't emit — so the machinery itself never double-counts. The only re-emissions are genuine live re-executions: a still-pending op re-run on the reset branch, or an op driven to completion on two diverged branches. Both are real executions and equally true for activities, and Validate() keeps a rebuilt terminal op from being re-driven.

Activities

Yes, they have the identical characteristics, and they emit on two timings depending on the path:

• Completion / failure / cancel go through the RespondActivityTask{Completed,Failed,Canceled} handlers, which record the metric only after GetAndUpdateWorkflowWithNew returns nil (and gated by GetActiveNamespace) — i.e. post-commit, active-only.
• Timeout is recorded inside the active timer executor (timer_queue_active_task_executor.go, processSingleActivityTimeoutTask) before the updateWorkflowExecution commit — i.e. pre-commit.
• Neither is ever called from the ApplyActivityTask*Event rebuild path.

Changes in the latest commit to match activities behaviour

I moved emission to after the write transaction commits on every Nexus path where we own the env.Access boundary — sync success, async start, sync failure/cancel, below-min timeout (all via saveResult), and the async completion handler. There the metric inputs are captured inside the closure and emitted only once env.Access returns nil, so a failed commit (which retries the task) doesn't double-count.

The timeout timer tasks stay pre-commit, because the HSM task framework owns their transaction and there's no post-commit hook (similar to activity-timeout metric has in the active timer executor)

So Nexus caller-side emission is now structurally the same as in-workflow activity emission: post-commit where we own the transaction, pre-commit only for the timer-driven timeout

About double-counting

The one possibility is the timer-timeout path: emission is an in-process side effect that runs before the commit, so a commit failure leading to a task retry can cause re-emit.

Nexus caller-side metrics

Outcome	Where it emits	Commit timing
Sync success	saveResult → deferredOperationMetric	post-commit
Async start (schedule-to-start)	saveResult (started state)	post-commit
Sync non-retryable failure / cancel	saveResult (via handleStartOperationError)	post-commit
Below-min-timeout	saveResult (state TIMED_OUT)	post-commit
Async completion success/fail/cancel (+ fabricated schedule-to-start)	CompletionHandler.Handle	post-commit
Timeout timers (schedule-to-close / schedule-to-start / start-to-close)	executeOperationTimeout	pre-commit

In-workflow activity metrics

Outcome	Where it emits	Commit timing
Completion / failure / cancel	RespondActivityTask{Completed,Failed,Canceled} (after GetAndUpdateWorkflowWithNew, gated by GetActiveNamespace)	post-commit
Timeout	active timer executor (timer_queue_active_task_executor.go, processSingleActivityTimeoutTask)	pre-commit

[tekkaya]

Changed return nil → return err here. The identical hsm.MachineData[Operation](node) read inside fabricateStartedEventIfMissing (just below) already propagates the error, so swallowing it here was inconsistent and a failed read returned nil (success) from the Access callback, reporting the completion as handled without applying it.

Was the original return nil intentional? I can revert if there's a reason I'm missing.

[bergundy]

Good catch!