[Bug]: telemetry in msg handlers runs multiple times leading to unexpected metric values

[migueldingli1997]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

Message logic is executed at least twice, once in checkTx and once during block finalization. Telemetry logic does not distinguish between these two modes and runs regardless, meaning that counters are incremented at least twice.

One example of this is the new_account counter which is incremented when we send tokens to a new account. The result is that this counter actually increments by 2 with each new account.

I suppose it could increment even more times if we run in other modes, such as to simulate a transaction, but I haven't experimented with this.

Cosmos SDK Version

0.50.9

How to reproduce?

• Run a chain with Prometheus telemetry enabled
• Send tokens to an account that does not exist
• Look up the new_account metric at :1317/metrics?format=prometheus

[julienrbrt]

I am not sure how best we can fix it.
IMHO we shouldn't need to query the transaction mode to enrich the metrics each time.
On the other hand, do we only want to collect metrics on DeliverTx? I would guess not as well.

[kocubinski]

In general it's the telemetry system's job to report what is happening, so if the same code path is being executed multiple times during execution telemetry should log it. Maybe a discriminator label can be attached for execution mode?

In particular though, this metric name seems off, it's incorrect to assume a new account is created every time coins are sent to it and it doesn't already exist (if I read that right).

cosmos-sdk/x/bank/keeper/send.go

Line 231 in 8bfcf55

defer telemetry.IncrCounter(1, "new", "account")

[julienrbrt]

I somewhat agree with @kocubinski above. Still not convinced about adding a round-trip to the tx service, however.
I'd say probably, if you want accuracy, you should be listening to events.
We can add a note on this behavior in the documentation (https://docs.cosmos.network/main/learn/advanced/telemetry).
Would it be an acceptable solution for you?

[migueldingli1997]

Apologies for the late reply! Thanks for your consideration 🙏