Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR introduces transaction lifecycle metrics for TXMv2 by injecting a metrics recorder into TXM construction and surfacing lifecycle-failure counters across key broadcast/backfill and dual-broadcast paths.
Changes:
- Add a
txm.Metricsinterface with a default implementation, a noop fallback, and a new lifecycle-failure counter with stage labels. - Thread metrics through TXM/dual-broadcast constructors and record lifecycle-failure increments at several failure/limit stages.
- Update TXM unit/integration tests and OCR dual contract transmitter wiring to pass chain ID and metrics instances.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/txmgr/builder.go | Creates a TXM metrics instance and passes it into TXMv2 + dual-broadcast client selection. |
| pkg/txm/txm.go | Injects metrics into Txm, records lifecycle failures, and switches TXM metrics init responsibility to callers. |
| pkg/txm/metrics.go | Introduces Metrics interface, adds lifecycle-failure counter + stages, and provides noop metrics fallback. |
| pkg/txm/metrics_test.go | Updates metrics tests for new constructor signature and adds noop metrics test. |
| pkg/txm/txm_test.go | Updates TXM tests to pass noop metrics and removes direct mutation of tm.Metrics. |
| pkg/txm/integration-tests/integration_test.go | Wires metrics into integration TXM setup. |
| pkg/txm/clientwrappers/dualbroadcast/selector.go | Threads lifecycle metrics into meta-client construction. |
| pkg/txm/clientwrappers/dualbroadcast/meta_client.go | Records auction-stage lifecycle failure on Meta send-request errors. |
| pkg/transmitter/util.go | Passes chain ID into OCR dual contract transmitter construction. |
| pkg/transmitter/dual_contract_transmitter.go | Creates lifecycle metrics and records create-stage failures for primary/secondary transmissions. |
| pkg/transmitter/dual_contract_transmitter_test.go | Updates transmitter tests for the new chainID parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| t.SetNonce(address, pendingNonce) | ||
| t.lggr.Debugf("Set initial nonce for address: %v to %d", address, pendingNonce) | ||
| t.lggr.Infof("Set initial nonce for address: %v to %d", address, pendingNonce) |
There was a problem hiding this comment.
initializeNonce log level was changed from Debug to Info. Since this runs on every TXM start for every enabled address, this can noticeably increase log volume in production. Consider keeping this at Debug (or gating it behind a config / sampling) unless operators explicitly need it at Info.
| t.lggr.Infof("Set initial nonce for address: %v to %d", address, pendingNonce) | |
| t.lggr.Debugf("Set initial nonce for address: %v to %d", address, pendingNonce) |
| if unconfirmedCount > MaxInFlightTransactions { | ||
| t.metrics.IncrementLifecycleFailure(ctx, StageMaxInFlight) | ||
| t.lggr.Warnf("Reached transaction limit: %d for unconfirmed transactions", MaxInFlightTransactions) | ||
| return true, nil |
There was a problem hiding this comment.
New lifecycle-failure metric increments were added to core TXM control-flow branches (e.g., max-in-flight early returns). There are no unit tests asserting these counters are incremented for the relevant paths; since Metrics is now an interface, consider injecting a mock Metrics implementation in tests to verify the new instrumentation behaves as intended.
| a.metrics.emitAtlasError(ctx, "send_request", a.customURL, err, tx) | ||
| a.lifecycleMetrics.IncrementLifecycleFailure(ctx, txm.StageAuction) | ||
| return fmt.Errorf("error sending request for transactionID(%d): %w", tx.ID, errors.Join(err, ErrAuction)) |
There was a problem hiding this comment.
This new StageAuction lifecycle-failure increment isn’t covered by existing tests for MetaClient behavior. Consider adding a unit test for MetaClient.SendTransaction error paths that asserts the injected lifecycleMetrics is invoked (now that it’s a txm.Metrics interface).
No description provided.