Some metrics followup #177
Conversation
krnowak
requested review from
bogdandrutu,
jmacd,
pjanotti,
rghetia,
sjkaris and
tedsuo
as code owners
krnowak
force-pushed
the
krnowak/metrics
branch
from
7730d18
to
29788ac
Compare
| measurementBatches []batch | ||
| observers observerMap | ||
| } | ||
| ) |
There was a problem hiding this comment.
Should mockMeter be defined in internal/metric package (like mock tracer)?
There was a problem hiding this comment.
I wonder if it should be in a test-support package, not an internal one, so that users can write mock tests of their own easily. Either way, let's leave a TODO behind and not block this PR.
There was a problem hiding this comment.
Actually, that was the first thing I did. But then I got import cycles (tests in api/metric package were importing internal/metric package to get the mock meter, and that in turn imported api/metric to implement the Meter interface). So annoying. So to resolve that I would either need to make tests a different package or put the mock meter into api/metric. I went with the latter for now.
There was a problem hiding this comment.
We can move the tersts in api/metric out of the "metric" package, into "metrics_test", to break this cycle. Let's leave it for a future TODO.
| measurementBatches []batch | ||
| observers observerMap | ||
| } | ||
| ) |
There was a problem hiding this comment.
I wonder if it should be in a test-support package, not an internal one, so that users can write mock tests of their own easily. Either way, let's leave a TODO behind and not block this PR.
|
@krnowak can you please resolve the conflict? |
krnowak
force-pushed
the
krnowak/metrics
branch
from
29788ac
to
4535726
Compare
|
Updated. |
| measurementBatches []batch | ||
| observers observerMap | ||
| } | ||
| ) |
There was a problem hiding this comment.
We can move the tersts in api/metric out of the "metric" package, into "metrics_test", to break this cycle. Let's leave it for a future TODO.
I know that the spec is going to have more changes, but I had some more work. Basically tests, more docs and some observer changes.