OEV-1648 Emit whitelisted EVM chain config metric - #548
Conversation
|
@cll-gg let's look at this after the weekend. No code should land on a Friday :) |
✅ API Diff Results -
|
mostlyconsistent
changed the title
| gauge, err := meter.Int64Gauge( | ||
| evmChainConfigInfoMetricName, | ||
| metric.WithDescription("Whitelisted EVM chain configuration; value is always 1, state is in the labels"), | ||
| metric.WithUnit("{info}"), | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create %s gauge: %w", evmChainConfigInfoMetricName, err) | ||
| } | ||
|
|
||
| gauge.Record(ctx, 1, metric.WithAttributes(chainConfigAttributes(chainID, txV2Enabled, dualBroadcast)...)) |
There was a problem hiding this comment.
Constructing and emitting together like this seems problematic. Can we construct the gauge once ahead of time?
There was a problem hiding this comment.
can you go into more details on why it is problematic?
| // hiccup must never block chain startup. | ||
| func (c *chain) emitChainConfigInfo(ctx context.Context, meter metric.Meter) { | ||
| if meter == nil { | ||
| meter = beholder.GetMeter() |
There was a problem hiding this comment.
We typically want to push these global beholder calls up/out to a higher level so that we have clean injection everywhere internal. It also keeps high level packages from being imported if we reference just the otel types.
| } | ||
|
|
||
| txV2 := c.cfg.EVM().Transactions().TransactionManagerV2() | ||
| if err := recordChainConfigInfo(ctx, meter, c.id.String(), txV2.Enabled(), derefBool(txV2.DualBroadcast())); err != nil { |
There was a problem hiding this comment.
IMHO the indirection makes this harder to read compared to inlining:
| if err := recordChainConfigInfo(ctx, meter, c.id.String(), txV2.Enabled(), derefBool(txV2.DualBroadcast())); err != nil { | |
| var dualBroadcast bool | |
| if db := txV2.DualBroadcast(); db != nil { | |
| dualBroadcast = *db | |
| } | |
| if err := recordChainConfigInfo(ctx, meter, c.id.String(), txV2.Enabled(), dualBroadcast); err != nil { |
But maybe a different name would be fine?
| if err := recordChainConfigInfo(ctx, meter, c.id.String(), txV2.Enabled(), derefBool(txV2.DualBroadcast())); err != nil { | |
| if err := recordChainConfigInfo(ctx, meter, c.id.String(), txV2.Enabled(), isTrue(txV2.DualBroadcast())); err != nil { |
|
|
||
| // Meter is used to report chain metrics. Defaults to the global beholder | ||
| // meter when unset. | ||
| // TODO: make this required once chainlink core passes it explicitly. |
There was a problem hiding this comment.
This TODO will still be part of the current ticket?
There was a problem hiding this comment.
its a chicken and egg problem. Once this lands, I can land a version of this smartcontractkit/chainlink#23726, and then undo the TODO :)
Up to you how many tickets you wanna have for the work.
3 participants
What
Emits SVR-relevant EVM chain config as an OpenTelemetry info gauge from each chain's startup path:
Value is always 1; all state is in the labels. The label whitelist is exhaustive and contains no URL.
Why
Since ~2026-07-20 the INCIDENT-2541 mitigation (drop filters in chip-ingress and beholder-gateway-logs) drops the whole NodeConfig beholder event, because that event carried the entire node TOML — including RPC/OFA URLs with embedded credentials. Collateral damage:
svr_beholder.node_configs(BigQuery) stopped ingesting.node_config_boot_time, which OFE derived from that event, went absent.