Add labeled metric types #97
Conversation
Dexterp37
force-pushed
the
labeled
branch
5 times, most recently
from
264eb36
to
5b35e84
Compare
glean/src/core/metrics/index.ts
Outdated
| * whatever is stored internally, without performing any specific | ||
| * validation. | ||
| */ | ||
| export class PassthroughMetric extends Metric<JSONValue, JSONValue> { |
There was a problem hiding this comment.
Reminder to self: I don't understand why we need the PassthorughMetric at all.
There was a problem hiding this comment.
Yeah I get it now. I was wrong when I assumed Labeled metrics did not need a Metric declaration. Even though we don't care about their internal representation, they need a Metric representation for generating the ping payload. I am sorry for the back and forth here, but I would prefer if instead of PassthrougMetric you made this LabeledMetric.
There was a problem hiding this comment.
Mh, too bad :) Now, I could do that and move it back to the labeled metric implementation file. Unfortunately doing so will trigger a circular dependency that breaks the TS build :(
There was a problem hiding this comment.
So, if you move this line to somewhere after you define the Metric abstract class the error goes away.
There was a problem hiding this comment.
I renamed this as requested and added a better comment.
This internally changes the id getter on the `MetricType` to read from the store in case of dyanmic labels, as done in the Glean SDK. This is required in order to attempt recording to Glean metrics before it gets initialized.
This supports string, boolean and counter labeled types and ports over tests from the Kotlin implementation in the SDK and the glean-core internals.
| @@ -10,6 +10,7 @@ | |||
| * [#101](https://github.com/mozilla/glean.js/pull/101): BUGFIX: Only validate Debug View Tag and Source Tags when they are present. | |||
| * [#101](https://github.com/mozilla/glean.js/pull/101): BUGFIX: Only validate Debug View Tag and Source Tags when they are present. | |||
There was a problem hiding this comment.
Why do we have two entries here... Whathever, can be fixed when we make the next release.
This introduces the labelled metric types for string, boolean and counter types. The implementation closely follows what's in the Glean SDK, with no major deviation.
Note that this required adding a couple more functions to the database.
TODO: