Support Points aggregator

[zlozano]

We use the exact selector implementation which ultimately surfaces Measures via a Point aggregator.

https://github.com/open-telemetry/opentelemetry-go/blob/master/sdk/metric/selector/simple/simple.go#L110

With this, we can export our raw points as a histogram or timing. It's also important that these types get exported as a histogram, rather than a gauge since the collector aggregations get applied to histograms:

https://github.com/DataDog/datadog-agent/blob/master/pkg/config/config_template.yaml#L128

I propose we amend the current implementation to also include the points aggregator similar to the original otel implementation: https://github.com/open-telemetry/opentelemetry-go/blob/v0.2.3/exporters/metric/internal/statsd/conn.go#L231

[zlozano]

Also, we should discontinue use of using the gauge metric type for all values, and use the correct value given the record type (e.g. count, histogram, etc.)

While it may not matter from the vendor perspective and in writing graphs, the datadog agent collects and reports on these values differently, and can cause incorrect data depending on what the flush intervals are set to e.g. gauge vs. count submission: https://docs.datadoghq.com/developers/metrics/types/?tab=gauge#example

The agent will only report the last value for gauge types in a recording window while reporting the sum aggregation for counter types.

[jmacd]

As written to Marwan on Gitter:

in the main export logic, there is a switch that uses the concrete types of the aggregators you're likely to see. another approach may be more future-proof. there are interfaces declared in sdk/export/metric/aggregator that aggregators are expected to implement, and if you switch on the interfaces instead of the concrete type the code will work for other configured aggregators. this is a future concern, not a big deal.

[marwan-at-work]

@jmacd I'd love to see an example of concrete type switching, as I couldn't find it.

In the meantime, I have added the aggregator.Points to the switch statement which can either submit Distribution or Histogram types here: 7691e46

[jmacd]

Oh, you've done it correctly. I'm not sure what I was looking at.

[jmacd]

@zlozano I could point to open-telemetry/oteps#88 on the topic of how to map into Datadog, or to open-telemetry/opentelemetry-specification#382 about configuring OTel itself.

[chrisleavoy]

I tested this out and observations are being aggregated via aggregator.Points and exported as histrograms when i expect them to result in a datadog type of gauge.

[zlozano]

@chrisleavoy in order to do this, we will need to create our own selector which uses the LastValue aggregator for the Observer measures

e.g. https://github.com/open-telemetry/opentelemetry-go/blob/master/sdk/metric/selector/simple/simple.go#L105

Except instead of fallthrough, we'd select LastValue. IIUC open-telemetry/oteps#88 should have more context on this, but I haven't had time to read through it yet

[chrisleavoy]

@zlozano cheers thanks.. got it working now.

[marwan-at-work]

For now this should be fixed by 7691e46 and the custom aggregator selector that @zlozano mentioned. I think maybe the library should export its own selector so users know they need to use it until open-telemetry/oteps#88 is resolved.

Definitely feel free to re-open the issue if anything comes up

Thanks!