refactor(metrics): New* constructors use GetOrRegister* functions

[qdm12]

Why this should be merged

• Avoid conflicts for metrics used both in consumers of libevm and in libevm
• See fix(metrics): coreth metrics do not conflict with libevm metrics coreth#848

The alternative would be to change all occurrences of metrics.New* by metrics.GetOrRegister* in both the consumer code and in the libevm code (because it's unpredictable which one would register first), but that produces a lot more diffs.

Just for historical purposes, I did try to have a custom registry on a consumer (coreth) for all metrics but:

• if the same metric is used in both the consumer and libevm (I think there are a few, given CI fails quite a bit), we would lose metrics from the libevm side because it uses another registry (and not sure we can merge both metrics from 2 different registries)
• it introduces a few hundred lines of diffs which is undesirable on the consumer side (coreth) at this time

How this works

• Change all metrics.New* function implementations to use metrics.GetOrRegister* functions.
• Add GetOrRegisterOrOverrideCounter because counter metrics in consumers are defined as timer metrics in libevm, so consumers need to use this function when this happens.

How this was tested

• CI passing
• GetOrRegisterOrOverrideCounter needs test

[ARR4N]

Is this a correct understanding of the status quo?

1. Either libevm or a consumer calls, for example, NewRegisteredCounter("foo", nil).
2. The other party then calls the exact same function but r.Register() returns an error that isn't checked.
3. The second caller is now holding an unregistered Counter so all of its recordings are dropped.

Is there a strict need for consumers to use the same metrics? What problems would be caused by name-spacing the metric name to guarantee no conflict?

EDIT: I hadn't considered the yet-to-transition packages that exist in coreth and will eventually be deleted in lieu of their libevm counterparts. I think this pattern is a better approach as it leaves libevm unchanged and is simply a global sed on coreth along with imports of libevm packages we'll be adding anyway.

[ARR4N]

The alternative would be to change all occurrences of metrics.New* by metrics.GetOrRegister* in both the consumer code and in the libevm code (because it's unpredictable which one would register first)

The order can be forced with a blank import for side effects. Here's a pattern that I think has the desired effect without the need to change libevm at all. It assumes that we know the correspondence between libevm and coreth metrics but (a) they should always be in the mirrored packages, and (b) our refactoring approach always involves the coreth package importing its libevm mirror just as in the pattern so we're eventually going to force the ordering anyway. Once the ordering is guaranteed, it's just a matter of changing New*() to GetOrRegister*() in the consumer only.

[qdm12]

Packages which are:

• defining global metrics using metrics.New* functions
• defined in coreth
• imported by coreth from libevm

are the following:

• core
• core/state/snapshot
• rpc
• triedb/hashdb
• triedb/pathdb

Now, the good thing is all their metrics are unexported, so that reduces the problem scope.

However, say for the core package, it's rather tough to find out if a coreth package other than core transitively imports libevm's core without importing coreth's core package. This could already be the case, and even if it is not, it could well be in the future. If, say coreth/xyz, imports libevm/xyz which itself imports libevm/core, it's not a direct import but this may execute before coreth/core (depending on compilation and other things outside our control I would say)

EDIT: using underscore imports in those packages above + GetOrRegister in consumers solves this

[ARR4N]

EDIT: using underscore imports in those packages above + GetOrRegister in consumers solves this

Closing in favour of ava-labs/coreth#860.