Migrate precompiles/bank and giga/deps metrics to OTel (PLT-912) #3859
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package tasks | ||
|
|
||
| import ( | ||
| "go.opentelemetry.io/otel" | ||
| "go.opentelemetry.io/otel/metric" | ||
| ) | ||
|
|
||
| // taskMetrics mirrors sei-cosmos/tasks/metrics.go's instruments of the same | ||
| // name/scope so the sei-cosmos and giga fork schedulers merge into single | ||
| // scheduler_retries/scheduler_incarnations series. Keep description/unit | ||
| // byte-identical across both declarations or the OTel SDK stops deduping. | ||
| var ( | ||
| meter = otel.Meter("seicosmos_tasks") | ||
|
There was a problem hiding this comment. [nit] This file is byte-identical to There was a problem hiding this comment. [nit] This file is a byte-identical copy of There was a problem hiding this comment. [suggestion] This file is byte-identical to This PR is itself the demonstration: it had to edit the |
||
|
|
||
| taskMetrics = struct { | ||
| retries metric.Int64Counter | ||
| incarnations metric.Int64Counter | ||
| }{ | ||
| retries: must(meter.Int64Counter( | ||
| "scheduler_retries", | ||
|
There was a problem hiding this comment. [suggestion] This PR establishes a good convention for The two files are byte-identical today, so this is purely preventative — but it's exactly the drift the bank mirror test exists to catch, and the failure mode is silent (a split series, discovered on a dashboard rather than in CI). Suggest exporting Relevant here: the |
||
| metric.WithDescription("Number of OCC scheduler transaction retries"), | ||
| metric.WithUnit("{count}"), | ||
| )), | ||
| incarnations: must(meter.Int64Counter( | ||
| "scheduler_incarnations", | ||
| metric.WithDescription("Sum of per-round maximum incarnations in the OCC scheduler"), | ||
| metric.WithUnit("{count}"), | ||
| )), | ||
| } | ||
| ) | ||
|
|
||
| func must[V any](v V, err error) V { | ||
|
There was a problem hiding this comment. [nit] This makes four identical copies of |
||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| return v | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package keeper | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "os" | ||
| "runtime/debug" | ||
|
|
||
| "go.opentelemetry.io/otel" | ||
| "go.opentelemetry.io/otel/metric" | ||
|
|
||
| "github.com/sei-protocol/sei-chain/sei-cosmos/telemetry" | ||
| ) | ||
|
|
||
| // BankNewAccount* must stay byte-identical to the same consts in | ||
| // sei-cosmos/x/bank/keeper and utils/metrics so the three Int64Counter | ||
| // declarations merge into one series. | ||
| const ( | ||
| BankNewAccountMeter = "seicosmos_x_bank_keeper" | ||
| BankNewAccountName = "bank_new_account" | ||
| BankNewAccountDescription = "Number of new accounts created during bank transfers" | ||
| BankNewAccountUnit = "{count}" | ||
| ) | ||
|
|
||
| // bankMetrics.newAccount mirrors sei-cosmos/x/bank/keeper/metrics.go's | ||
| // instrument of the same name/scope so the two dual-emit paths merge into a | ||
|
There was a problem hiding this comment. [nit] "the two dual-emit paths" — there are three declarations of this instrument (this file, |
||
| // single bank_new_account series. | ||
| var ( | ||
| meter = otel.Meter(BankNewAccountMeter) | ||
|
|
||
| bankMetrics = struct { | ||
| newAccount metric.Int64Counter | ||
| }{ | ||
| newAccount: must(meter.Int64Counter( | ||
| BankNewAccountName, | ||
| metric.WithDescription(BankNewAccountDescription), | ||
| metric.WithUnit(BankNewAccountUnit), | ||
| )), | ||
| } | ||
| ) | ||
|
|
||
| func must[V any](v V, err error) V { | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| return v | ||
| } | ||
|
|
||
| // recordNewAccounts dual-emits the legacy new-account counter and its OTel | ||
| // counterpart (bank_new_account). Runs from consensus-critical send paths, so | ||
| // a telemetry fault here must not panic into the caller. | ||
| func recordNewAccounts(ctx context.Context, count int64) { | ||
|
There was a problem hiding this comment. [nit] This is a byte-for-byte copy of Also note the mirror test pins the four consts but not this function body, so the dual-emit logic itself can drift between the fork and upstream without CI noticing. |
||
| if count <= 0 { | ||
|
There was a problem hiding this comment. [nit] |
||
| return | ||
| } | ||
| defer func() { | ||
| if e := recover(); e != nil { | ||
|
There was a problem hiding this comment. [nit] |
||
| fmt.Fprintf(os.Stderr, "telemetry panic: %v\n%s", e, debug.Stack()) | ||
| } | ||
| }() | ||
| // TODO(PLT-353): remove once bank_new_account verified | ||
| func() { | ||
| defer func() { | ||
| if e := recover(); e != nil { | ||
| fmt.Fprintf(os.Stderr, "telemetry panic: %v\n%s", e, debug.Stack()) | ||
| } | ||
| }() | ||
| telemetry.IncrCounter(float32(count), "new", "account") | ||
| }() | ||
| bankMetrics.newAccount.Add(ctx, count) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] This file is a byte-identical copy of
sei-cosmos/tasks/metrics.go(same blob), but it's the only one of the three mirrored metrics files without the "same name/scope so the series merge; keep description/unit byte-identical or the OTel SDK stops deduping" comment thatgiga/deps/xbank/keeper/metrics.goandsei-cosmos/x/bank/keeper/metrics.goboth carry. It's also the copy holding the correctedscheduler_incarnationsdescription that must stay in lockstep with sei-cosmos — the place drift is most likely. Please add the same comment here.