feat(ibc): migrate sei-ibc-go metrics to OpenTelemetry with dual emission#3543
feat(ibc): migrate sei-ibc-go metrics to OpenTelemetry with dual emission#3543amir-deris wants to merge 6 commits into
Conversation
PR SummaryLow Risk Overview New Reviewed by Cursor Bugbot for commit f416fdd. Bugbot is set up for automated code reviews on this repo. Configure here. |
amir-deris
changed the title
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3543 +/- ##
==========================================
- Coverage 59.12% 58.29% -0.84%
==========================================
Files 2218 2145 -73
Lines 183132 174524 -8608
==========================================
- Hits 108285 101743 -6542
+ Misses 65082 63730 -1352
+ Partials 9765 9051 -714
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f416fdd. Configure here.
| "ibc_transfer_packet_receive", | ||
| metric.WithDescription("Total amount of tokens received in IBC packet"), | ||
| metric.WithUnit("{token}"), | ||
| )), |
There was a problem hiding this comment.
Gauge metrics imply cumulative totals
Medium Severity
The new ibc_transfer_tx_msg and ibc_transfer_packet_receive instruments are Int64Gauge values that record the latest transfer amount per denom_class, but their names and descriptions read like cumulative totals. That diverges from other OTel amount gauges here (last_send_amount, last_withdraw_reward_amount) and can mislead new dashboards built on these series.
Triggered by learned rule: OTel metrics: guard attribute cardinality and use native types
Reviewed by Cursor Bugbot for commit f416fdd. Configure here.
| ibcTransferMetrics.ibcTransferSend.Add(ctx.Context(), 1, otelmetric.WithAttributes( | ||
| attribute.String(coretypes.LabelDestinationPort, destinationPort), | ||
| attribute.String(coretypes.LabelDestinationChannel, destinationChannel), | ||
| attribute.String(coretypes.LabelSource, source), |
There was a problem hiding this comment.
Why string label for a boolean value?
| } | ||
|
|
||
| ibcTransferMetrics.ibcTransferSend.Add(ctx.Context(), 1, otelmetric.WithAttributes( | ||
| attribute.String(coretypes.LabelDestinationPort, destinationPort), |
There was a problem hiding this comment.
Port is numeric? Could you review all attribute types relative to what they measure please?
3 participants
Summary
Adds OpenTelemetry instrumentation to
sei-ibc-goas part of the broader OTel metrics migration (PLT-427/PLT-428). Follows the same dual-emit pattern established in PLT-330 (x/evm) and PLT-353 (sei-cosmos): legacy telemetry calls are preserved alongside new OTel instruments until dashboards are verified and legacy can be removed.Changes
5 new
metrics.gofiles — each initializes an OTel meter and registers instruments:apps/transfer/keeperibc_transfertx_msg_ibc_transfer(gauge),ibc_transfer_packet_receive(gauge),ibc_transfer_send(counter),ibc_transfer_receive(counter)core/keeperibc_coretx_msg_ibc_recv_packet,ibc_timeout_packet,tx_msg_ibc_acknowledge_packet(counters)core/02-client/keeperibc_clientibc_client_create,ibc_client_update,ibc_client_upgrade,ibc_client_misbehaviour(counters)core/03-connection/keeperibc_connectionibc_connection_open_{init,try,ack,confirm}(counters)core/04-channel/keeperibc_channelibc_channel_{open,close}_{init,try,ack,confirm}(counters)11 source files modified — dual-emit added at existing telemetry call sites:
transfer/keeper/relay.go—sendTransfer,OnRecvPacketcore/keeper/msg_server.go—RecvPacket,Timeout,TimeoutOnClose,Acknowledgementcore/02-client/keeper/client.go—CreateClient,UpdateClient,UpgradeClient,CheckMisbehaviourAndUpdateStatecore/02-client/keeper/proposal.go—ClientUpdateProposalcore/03-connection/keeper/handshake.go—ConnOpenInit/Try/Ack/Confirmcore/04-channel/keeper/handshake.go—ChanOpenInit/Try/Ack/Confirm,ChanCloseInit/ConfirmLegacy calls are marked
TODO(PLT-428)for removal once OTel instruments are verified in production dashboards.