metric/selector: return LastValue aggregator for Observer kinds

[marwan-at-work]

Disclaimer: I am not very familiar with the OT spec and codebase, so please take a look and let me know if I'm missing something :)

What I have observed is that Observer kinds end up Exporting a aggregator.MinMaxSumCount/Distribution measures when they should probably be aggregator.LastValue.

Therefore, if I am writing an Exporter, I will not know if a value needs to be exported as a Gauge type on the backend unless I check against aggregator.LastValue.

[krnowak]

It was argued that it's more useful for observers to use the mmsc aggregation rather than the last value aggregation (CC @jmacd). But this behaviour can be overridden by passing your own selector, like:

// this is stolen from exporters/metric/stdout/stdout.go
func NewExportPipeline(config stdout.Config, period time.Duration) (*push.Controller, error) {
	if config.LabelEncoder == nil {
		config.LabelEncoder = export.NewDefaultLabelEncoder()
	}
	exporter, err := stdout.NewRawExporter(config)
	if err != nil {
		return nil, err
	}
	selector := myselector.New()
	batcher := ungrouped.New(selector, config.LabelEncoder, true)
	pusher := push.New(batcher, exporter, period)
	pusher.Start()
	return pusher, nil
}

The changes in the otlp transform look useful. CC @MrAlias

[marwan-at-work]

@krnowak thanks for the feedback, a couple of questions:

1. Your suggestion means that an Exporter will force a user to choose their own Selector, is that okay?

2. I also don't see why an observer should be an mmsc, since an observer is defined in the OT spec as a LastValue, why not just use that? The MMSC that the exporter gets looks pretty odd, because the Callback gets called only once per Export invocation and therefore the mmsc will always look like this {min: 3, max: 3, sum: 3, count: 1} -- assuming the callback called result.Observe(3).

[krnowak]

@krnowak thanks for the feedback, a couple of questions:

1. Your suggestion means that an Exporter will force a user to choose their own Selector, is that okay?

Not really. The NewExportPipeline is more of a convenience function. Usually, if you don't like what the convenience function provides you, you can write your own exporter setup, which means creating an exporter, a selector, a batcher and some kind of controller that will drive the sdk.

2. I also don't see why an observer should be an mmsc, since an observer is defined in the OT spec as a LastValue, why not just use that? The MMSC that the exporter gets looks pretty odd, because the Callback gets called only _once_ per Export invocation and therefore the mmsc will always look like this `{min: 3, max: 3, sum: 3, count: 1}` -- assuming the callback called `result.Observe(3)`.

Over time the aggregation should be more like {min: 3, max: 42, sum: 123, count: 6} for some combination of result.Observe calls.

[marwan-at-work]

@krnowak

Over time the aggregation should be more like {min: 3, max: 42, sum: 123, count: 6} for some combination of result.Observe calls.

Would love to see an example of how that would happen :)

Thanks again!

[jmacd]

One of the contentions raised in January over what the spec currently says is that LastValue is not often useful for spatial aggregation. There are parts of OTEP 88 that are not reflected in the specification yet, which refine the Observer specification to say clearly that they are permitted only one value per collection interval (the current "last" value).

Using MMSC as the default aggregator does mean that without any spatial aggregation, you get (as noted above) a checkpoint w/ count=1 and min=max=sum. This may seem redundant, but if you merge aggregators across spatial dimensions (i.e., Labels) in a single collection interval, the output of aggregation can have count>1.

This also allows aggregating over time to export a distribution, but the "last" value is still well defined in each collection interval for a given label set. I imagine that an exporter that wants traditional gauge semantics could just export the sum, under the assumption that count==1, and that would be all that's needed.

[jmacd]

This topic is documented in OTEP 88 and I've spoken with @marwan-at-work about it. I will close this and we may continue the discussion here: open-telemetry/opentelemetry-specification#549