refactor: remove global metrics in store #14439

tac0turtle · 2022-12-28T13:12:44Z

Description

this pr is a first attempt at removing the global telemetry variable from usage in store.

Changes:

Store defines its own telemetry interface
MeasureSince does not take a time.Now() in order to allow application developers to set a NoOp metric gather for reduced allocation of time.Now().
Adds metrics to the creation of a store.

@alexanderbez @julienrbrt would like to hear from you if this is a direction I should continue, I didnt complete it to not spend too much time headed in a direction where I shouldn't.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

store/metrics/telemetry.go

+// MeasureSince provides a wrapper functionality for emitting a a time measure
+// metric with global labels (if any).
+func (m Metrics) MeasureSince(keys ...string) {
+	start := time.Now()


julienrbrt

lgtm. although if we need to extract telemetry again, I still think a package would make sense.

tac0turtle · 2022-12-30T13:32:54Z

we need to refactor how telemetry works in the sdk to spin it out. Right now it works like a global, which is fine for modules but we should define a telemetry interface as well.