From ecbf479f8075ff49839aab384d602647bf24c2f4 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Tue, 9 Jul 2024 21:39:26 +0300 Subject: [PATCH] refactor(core)!: move telemetry into internal folder (#6763) * refactor(core)move telemetry into internal folder, move metric.go in separate folder for core/02-client. * add changelog entry. * chore: msg_server.go -> packet.go --- CHANGELOG.md | 1 + .../transfer/internal/telemetry/telemetry.go | 18 +++--- modules/core/02-client/keeper/client.go | 49 ++-------------- modules/core/02-client/types/metrics.go | 9 --- modules/core/internal/telemetry/client.go | 58 +++++++++++++++++++ modules/core/internal/telemetry/packet.go | 44 ++++++++++++++ modules/core/keeper/msg_server.go | 50 ++-------------- modules/core/{types => metrics}/metrics.go | 11 +++- 8 files changed, 133 insertions(+), 107 deletions(-) delete mode 100644 modules/core/02-client/types/metrics.go create mode 100644 modules/core/internal/telemetry/client.go create mode 100644 modules/core/internal/telemetry/packet.go rename modules/core/{types => metrics}/metrics.go (63%) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1b08292819..bcad9641dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (23-commmitment) [\#6644](https://github.com/cosmos/ibc-go/pull/6644) Introduce commitment/v2 `MerklePath` to include `repeated bytes` in favour of `repeated string`. This supports using merkle path keys which include non UTF-8 encoded runes. * (23-commmitment) [\#6633](https://github.com/cosmos/ibc-go/pull/6633) MerklePath has been changed to use `repeated bytes` in favour of `repeated strings`. * (apps/27-interchain-accounts) [\#6749](https://github.com/cosmos/ibc-go/pull/6749) The ICA controller `NewIBCMiddleware` constructor function sets by default the auth module to nil. +* (core, core/02-client) [\#6763](https://github.com/cosmos/ibc-go/pull/6763) Move prometheus metric labels for 02-client and core into a separate `metrics` package on core. * (core/02-client) [\#6777](https://github.com/cosmos/ibc-go/pull/6777) The `NewClientProposalHandler` of `02-client` has been removed. ### State Machine Breaking diff --git a/modules/apps/transfer/internal/telemetry/telemetry.go b/modules/apps/transfer/internal/telemetry/telemetry.go index 74a7b15dfa9..38414636de0 100644 --- a/modules/apps/transfer/internal/telemetry/telemetry.go +++ b/modules/apps/transfer/internal/telemetry/telemetry.go @@ -10,13 +10,13 @@ import ( "github.com/cosmos/cosmos-sdk/telemetry" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - coretypes "github.com/cosmos/ibc-go/v8/modules/core/types" + coremetrics "github.com/cosmos/ibc-go/v8/modules/core/metrics" ) func ReportTransfer(sourcePort, sourceChannel, destinationPort, destinationChannel string, tokens types.Tokens) { labels := []metrics.Label{ - telemetry.NewLabel(coretypes.LabelDestinationPort, destinationPort), - telemetry.NewLabel(coretypes.LabelDestinationChannel, destinationChannel), + telemetry.NewLabel(coremetrics.LabelDestinationPort, destinationPort), + telemetry.NewLabel(coremetrics.LabelDestinationChannel, destinationChannel), } for _, token := range tokens { @@ -25,11 +25,11 @@ func ReportTransfer(sourcePort, sourceChannel, destinationPort, destinationChann telemetry.SetGaugeWithLabels( []string{"tx", "msg", "ibc", "transfer"}, float32(amount.Int64()), - []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, token.Denom.Path())}, + []metrics.Label{telemetry.NewLabel(coremetrics.LabelDenom, token.Denom.Path())}, ) } - labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, fmt.Sprintf("%t", !token.Denom.HasPrefix(sourcePort, sourceChannel)))) + labels = append(labels, telemetry.NewLabel(coremetrics.LabelSource, fmt.Sprintf("%t", !token.Denom.HasPrefix(sourcePort, sourceChannel)))) } telemetry.IncrCounterWithLabels( @@ -41,9 +41,9 @@ func ReportTransfer(sourcePort, sourceChannel, destinationPort, destinationChann func ReportOnRecvPacket(sourcePort, sourceChannel string, token types.Token) { labels := []metrics.Label{ - telemetry.NewLabel(coretypes.LabelSourcePort, sourcePort), - telemetry.NewLabel(coretypes.LabelSourceChannel, sourceChannel), - telemetry.NewLabel(coretypes.LabelSource, fmt.Sprintf("%t", token.Denom.HasPrefix(sourcePort, sourceChannel))), + telemetry.NewLabel(coremetrics.LabelSourcePort, sourcePort), + telemetry.NewLabel(coremetrics.LabelSourceChannel, sourceChannel), + telemetry.NewLabel(coremetrics.LabelSource, fmt.Sprintf("%t", token.Denom.HasPrefix(sourcePort, sourceChannel))), } // Transfer amount has already been parsed in caller. transferAmount, _ := sdkmath.NewIntFromString(token.Amount) @@ -53,7 +53,7 @@ func ReportOnRecvPacket(sourcePort, sourceChannel string, token types.Token) { telemetry.SetGaugeWithLabels( []string{"ibc", types.ModuleName, "packet", "receive"}, float32(transferAmount.Int64()), - []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, denomPath)}, + []metrics.Label{telemetry.NewLabel(coremetrics.LabelDenom, denomPath)}, ) } diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index 3ed572520f0..1aa3da3524a 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -1,15 +1,13 @@ package keeper import ( - metrics "github.com/hashicorp/go-metrics" - errorsmod "cosmossdk.io/errors" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" "github.com/cosmos/ibc-go/v8/modules/core/exported" + "github.com/cosmos/ibc-go/v8/modules/core/internal/telemetry" ) // CreateClient generates a new client identifier and invokes the associated light client module in order to @@ -40,11 +38,7 @@ func (k *Keeper) CreateClient(ctx sdk.Context, clientType string, clientState, c initialHeight := clientModule.LatestHeight(ctx, clientID) k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", initialHeight.String()) - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "create"}, - 1, - []metrics.Label{telemetry.NewLabel(types.LabelClientType, clientType)}, - ) + defer telemetry.ReportCreateClient(clientType) emitCreateClientEvent(ctx, clientID, clientType, initialHeight) @@ -77,15 +71,7 @@ func (k *Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg export k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID) - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "misbehaviour"}, - 1, - []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, clientType), - telemetry.NewLabel(types.LabelClientID, clientID), - telemetry.NewLabel(types.LabelMsgType, "update"), - }, - ) + defer telemetry.ReportUpdateClient(foundMisbehaviour, clientType, clientID) emitSubmitMisbehaviourEvent(ctx, clientID, clientType) @@ -96,15 +82,7 @@ func (k *Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg export k.Logger(ctx).Info("client state updated", "client-id", clientID, "heights", consensusHeights) - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "update"}, - 1, - []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, clientType), - telemetry.NewLabel(types.LabelClientID, clientID), - telemetry.NewLabel(types.LabelUpdateType, "msg"), - }, - ) + defer telemetry.ReportUpdateClient(foundMisbehaviour, clientType, clientID) // emitting events in the keeper emits for both begin block and handler client updates emitUpdateClientEvent(ctx, clientID, clientType, consensusHeights, k.cdc, clientMsg) @@ -140,14 +118,7 @@ func (k *Keeper) UpgradeClient( latestHeight := clientModule.LatestHeight(ctx, clientID) k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", latestHeight.String()) - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "upgrade"}, - 1, - []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, clientType), - telemetry.NewLabel(types.LabelClientID, clientID), - }, - ) + defer telemetry.ReportUpgradeClient(clientType, clientID) emitUpgradeClientEvent(ctx, clientID, clientType, latestHeight) @@ -190,15 +161,7 @@ func (k *Keeper) RecoverClient(ctx sdk.Context, subjectClientID, substituteClien k.Logger(ctx).Info("client recovered", "client-id", subjectClientID) - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "update"}, - 1, - []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, clientType), - telemetry.NewLabel(types.LabelClientID, subjectClientID), - telemetry.NewLabel(types.LabelUpdateType, "recovery"), - }, - ) + defer telemetry.ReportRecoverClient(clientType, subjectClientID) // emitting events in the keeper for recovering clients emitRecoverClientEvent(ctx, subjectClientID, clientType) diff --git a/modules/core/02-client/types/metrics.go b/modules/core/02-client/types/metrics.go deleted file mode 100644 index 879e79a40d1..00000000000 --- a/modules/core/02-client/types/metrics.go +++ /dev/null @@ -1,9 +0,0 @@ -package types - -// Prometheus metric labels. -const ( - LabelClientType = "client_type" - LabelClientID = "client_id" - LabelUpdateType = "update_type" - LabelMsgType = "msg_type" -) diff --git a/modules/core/internal/telemetry/client.go b/modules/core/internal/telemetry/client.go new file mode 100644 index 00000000000..91a7d47804d --- /dev/null +++ b/modules/core/internal/telemetry/client.go @@ -0,0 +1,58 @@ +package telemetry + +import ( + metrics "github.com/hashicorp/go-metrics" + + "github.com/cosmos/cosmos-sdk/telemetry" + + ibcmetrics "github.com/cosmos/ibc-go/v8/modules/core/metrics" +) + +func ReportCreateClient(clientType string) { + telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "create"}, + 1, + []metrics.Label{telemetry.NewLabel(ibcmetrics.LabelClientType, clientType)}, + ) +} + +func ReportUpdateClient(foundMisbehaviour bool, clientType, clientID string) { + labels := []metrics.Label{ + telemetry.NewLabel(ibcmetrics.LabelClientType, clientType), + telemetry.NewLabel(ibcmetrics.LabelClientID, clientID), + } + + var updateType string + if foundMisbehaviour { + labels = append(labels, telemetry.NewLabel(ibcmetrics.LabelMsgType, "update")) + updateType = "misbehaviour" + } else { + labels = append(labels, telemetry.NewLabel(ibcmetrics.LabelUpdateType, "msg")) + updateType = "update" + } + + telemetry.IncrCounterWithLabels([]string{"ibc", "client", updateType}, 1, labels) +} + +func ReportUpgradeClient(clientType, clientID string) { + telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "upgrade"}, + 1, + []metrics.Label{ + telemetry.NewLabel(ibcmetrics.LabelClientType, clientType), + telemetry.NewLabel(ibcmetrics.LabelClientID, clientID), + }, + ) +} + +func ReportRecoverClient(clientType, subjectClientID string) { + telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "update"}, + 1, + []metrics.Label{ + telemetry.NewLabel(ibcmetrics.LabelClientType, clientType), + telemetry.NewLabel(ibcmetrics.LabelClientID, subjectClientID), + telemetry.NewLabel(ibcmetrics.LabelUpdateType, "recovery"), + }, + ) +} diff --git a/modules/core/internal/telemetry/packet.go b/modules/core/internal/telemetry/packet.go new file mode 100644 index 00000000000..229f6c857e8 --- /dev/null +++ b/modules/core/internal/telemetry/packet.go @@ -0,0 +1,44 @@ +package telemetry + +import ( + metrics "github.com/hashicorp/go-metrics" + + "github.com/cosmos/cosmos-sdk/telemetry" + + "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + ibcmetrics "github.com/cosmos/ibc-go/v8/modules/core/metrics" +) + +func ReportRecvPacket(packet types.Packet) { + telemetry.IncrCounterWithLabels( + []string{"tx", "msg", "ibc", types.EventTypeRecvPacket}, + 1, + addPacketLabels(packet), + ) +} + +func ReportTimeoutPacket(packet types.Packet, timeoutType string) { + labels := append(addPacketLabels(packet), telemetry.NewLabel(ibcmetrics.LabelTimeoutType, timeoutType)) + telemetry.IncrCounterWithLabels( + []string{"ibc", "timeout", "packet"}, + 1, + labels, + ) +} + +func ReportAcknowledgePacket(packet types.Packet) { + telemetry.IncrCounterWithLabels( + []string{"tx", "msg", "ibc", types.EventTypeAcknowledgePacket}, + 1, + addPacketLabels(packet), + ) +} + +func addPacketLabels(packet types.Packet) []metrics.Label { + return []metrics.Label{ + telemetry.NewLabel(ibcmetrics.LabelSourcePort, packet.SourcePort), + telemetry.NewLabel(ibcmetrics.LabelSourceChannel, packet.SourceChannel), + telemetry.NewLabel(ibcmetrics.LabelDestinationPort, packet.DestinationPort), + telemetry.NewLabel(ibcmetrics.LabelDestinationChannel, packet.DestinationChannel), + } +} diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index a30e96d1ed6..3851dc47c2b 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -4,11 +4,8 @@ import ( "context" "errors" - metrics "github.com/hashicorp/go-metrics" - errorsmod "cosmossdk.io/errors" - "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" @@ -17,6 +14,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" + "github.com/cosmos/ibc-go/v8/modules/core/internal/telemetry" coretypes "github.com/cosmos/ibc-go/v8/modules/core/types" ) @@ -510,16 +508,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack } } - defer telemetry.IncrCounterWithLabels( - []string{"tx", "msg", "ibc", channeltypes.EventTypeRecvPacket}, - 1, - []metrics.Label{ - telemetry.NewLabel(coretypes.LabelSourcePort, msg.Packet.SourcePort), - telemetry.NewLabel(coretypes.LabelSourceChannel, msg.Packet.SourceChannel), - telemetry.NewLabel(coretypes.LabelDestinationPort, msg.Packet.DestinationPort), - telemetry.NewLabel(coretypes.LabelDestinationChannel, msg.Packet.DestinationChannel), - }, - ) + defer telemetry.ReportRecvPacket(msg.Packet) ctx.Logger().Info("receive packet callback succeeded", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "result", channeltypes.SUCCESS.String()) @@ -581,17 +570,7 @@ func (k *Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (* return nil, errorsmod.Wrap(err, "timeout packet callback failed") } - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "timeout", "packet"}, - 1, - []metrics.Label{ - telemetry.NewLabel(coretypes.LabelSourcePort, msg.Packet.SourcePort), - telemetry.NewLabel(coretypes.LabelSourceChannel, msg.Packet.SourceChannel), - telemetry.NewLabel(coretypes.LabelDestinationPort, msg.Packet.DestinationPort), - telemetry.NewLabel(coretypes.LabelDestinationChannel, msg.Packet.DestinationChannel), - telemetry.NewLabel(coretypes.LabelTimeoutType, "height"), - }, - ) + defer telemetry.ReportTimeoutPacket(msg.Packet, "height") ctx.Logger().Info("timeout packet callback succeeded", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "result", channeltypes.SUCCESS.String()) @@ -656,17 +635,7 @@ func (k *Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTime return nil, errorsmod.Wrap(err, "timeout on close callback failed") } - defer telemetry.IncrCounterWithLabels( - []string{"ibc", "timeout", "packet"}, - 1, - []metrics.Label{ - telemetry.NewLabel(coretypes.LabelSourcePort, msg.Packet.SourcePort), - telemetry.NewLabel(coretypes.LabelSourceChannel, msg.Packet.SourceChannel), - telemetry.NewLabel(coretypes.LabelDestinationPort, msg.Packet.DestinationPort), - telemetry.NewLabel(coretypes.LabelDestinationChannel, msg.Packet.DestinationChannel), - telemetry.NewLabel(coretypes.LabelTimeoutType, "channel-closed"), - }, - ) + defer telemetry.ReportTimeoutPacket(msg.Packet, "channel-closed") ctx.Logger().Info("timeout on close callback succeeded", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "result", channeltypes.SUCCESS.String()) @@ -723,16 +692,7 @@ func (k *Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAck return nil, errorsmod.Wrap(err, "acknowledge packet callback failed") } - defer telemetry.IncrCounterWithLabels( - []string{"tx", "msg", "ibc", channeltypes.EventTypeAcknowledgePacket}, - 1, - []metrics.Label{ - telemetry.NewLabel(coretypes.LabelSourcePort, msg.Packet.SourcePort), - telemetry.NewLabel(coretypes.LabelSourceChannel, msg.Packet.SourceChannel), - telemetry.NewLabel(coretypes.LabelDestinationPort, msg.Packet.DestinationPort), - telemetry.NewLabel(coretypes.LabelDestinationChannel, msg.Packet.DestinationChannel), - }, - ) + defer telemetry.ReportAcknowledgePacket(msg.Packet) ctx.Logger().Info("acknowledgement succeeded", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "result", channeltypes.SUCCESS.String()) diff --git a/modules/core/types/metrics.go b/modules/core/metrics/metrics.go similarity index 63% rename from modules/core/types/metrics.go rename to modules/core/metrics/metrics.go index 9fcd348a7d6..d2ae5cfb6ac 100644 --- a/modules/core/types/metrics.go +++ b/modules/core/metrics/metrics.go @@ -1,7 +1,16 @@ -package types +package metrics // Prometheus metric labels. const ( + // 02-client labels + + LabelClientType = "client_type" + LabelClientID = "client_id" + LabelUpdateType = "update_type" + LabelMsgType = "msg_type" + + // Message server labels + LabelSourcePort = "source_port" LabelSourceChannel = "source_channel" LabelDestinationPort = "destination_port"