Metrics Implementation

[lzchen]

This PR is the first part of finishing [#155]. Implements the Metric SDK in the following ways:

1. Meter implementation + loader
2. Metric instruments (gauge, counter, measure)
3. Handles for metric instruments
4. RecordBatch

Additional changes include:

1. Using Tuple instead of List for label values
2. Using a generic update function as an interface to the add(), set() and record() functions for the metric handles.
3. Example included in example app.

TODOs:

1. Implement record() behaviour for MeasureMetric
2. Observer + callback mechanism for Gauge RFC
3. Label sets + required/optional label keys + record directly through metric instrument RFC
4. Locks for handles?

[c24t]

I'm still reviewing this, but it looks like DefaultMetric (and etc.) addresses my main concern. After a quick pass I don't see any obvious blockers.

[c24t]

Not to bikeshed on type annotations, but you don't need this or MetricT below unless you want the generic type for something. E.g. to declare that a function's return type matches one of its argument's type. As I understand it Union[int, float] would do the same thing here.

[lzchen]

I think according to @Oberon00 , mypy cannot deduce the actual return type of the parameter/function if Union is used, although I'm not really sure now what is meant by that. As well, sphinx does not seem to be able to read TypeVar in the docs.

This comment, I was using Union previously.

[Oberon00]

Yes that seems indeed unused. Maybe I mixed something up in that comment, I'll have a closer look later. But as this PR discussion is getting too long and this is certainly no blocker, I guess we can make a follow-up for that.

EDIT: See #160 (comment), they would make sense.

[c24t]

Why move these in this PR if you're not using them?

[lzchen]

Currently, the Meter implementation has no concept of limits on certain data objects (handle, metric), but could possibly have those limitations in the future, in which it can leverage these data structures. They also seem reusable enough to put in utils.py. I remember there was a comment about moving it to a common place. I don't have a strong opinion though.

[Oberon00]

Yes that seems indeed unused. Maybe I mixed something up in that comment, I'll have a closer look later. But as this PR discussion is getting too long and this is certainly no blocker, I guess we can make a follow-up for that.

EDIT: See #160 (comment), they would make sense.

[Oberon00]

Suggested change

	) -> "Metric":
	) -> metrics_api.MetricT:

https://docs.python.org/3/library/typing.html#generics

And now I remember the reason for ValueT: I thought we should make a MetricT[ValueT] to make mypy detect all cases where we could run into the "wrong value type" check at lint-time.

[c24t]

Nice SDK test improvements. No more blocking comments from me, thanks for tackling this beast of a PR.

Some things to follow up on after this is merged:

• implement measure's record
• follow up on Metrics specification updates opentelemetry-specification#250 and ticket any changes to the spec added after this PR
• same thing, but compare to Golang metrics prototype opentelemetry-go#100
• improve API tests to check that e.g. DefaultMetricHandle methods are no-ops

[lzchen]

@c24t
Can you explain a bit more about adding no-op tests for the DefaultMetricHandle methods?
Currently, DefaultMetricHandle does not have any methods. Are you referring to the case where users try to use an actual handle counterhandle.add but they accidently loaded the default?

[c24t]

Are you referring to the case where users try to use an actual handle counterhandle.add but they accidently loaded the default?

Sorry, my comment wasn't clear. That's exactly what I mean -- testing that using DefaultMetricHandle in place of a CounterHandle (or etc.) doesn't crash the app or give the user back the wrong type of object (especially null) for some call.

Since nothing is using data yet, I think your tests actually do cover this, but we'll have to revisit when we hook up exporters.