Spring cleaning.

[a-feld]

Some spring cleaning 😄

[jack-berg]

If you reorganized it as follows, we'd be able to independently test the preparation of the payload from the measurement / sending of the http request (and all the code would be indented one fewer tab):

func (e exporter) pushMetricData(ctx context.Context, md pdata.Metrics) (outputErr error) {
	details := newMetricMetadata(ctx)
	_, details.dataInputCount = md.MetricAndDataPointCount()
	bb := func () (telemetry.PayloadEntry, error) {
		return e.buildMetricPayload(details, ctx, md)
	}
	return e.export(&details, ctx, bb);
}

func (e exporter) buildMetricPayload(details exportMetadata, ctx context.Context, md pdata.Metrics) (telemetry.PayloadEntry, error) {
	var (
		errs  []error
		batch telemetry.MetricBatch
	)

	rms := md.ResourceMetrics()
	_, dpCount := md.MetricAndDataPointCount()
	batch.Metrics = make([]telemetry.Metric, 0, dpCount)
	for i := 0; i < rms.Len(); i++ {
		rm := rms.At(i)
		ilms := rm.InstrumentationLibraryMetrics()
		for j := 0; j < ilms.Len(); j++ {
			ilm := ilms.At(j)
			ms := ilm.Metrics()
			transform := newTransformer(rm.Resource(), ilm.InstrumentationLibrary())
			for k := 0; k < ms.Len(); k++ {
				m := ms.At(k)
				nrMetrics, err := transform.Metric(m)
				if err != nil {
					e.logger.Debug("Transform of metric failed.", zap.Error(err))
					errs = append(errs, err)
					continue
				}
				details.dataOutputCount += len(nrMetrics)
				batch.Metrics = append(batch.Metrics, nrMetrics...)
			}
		}
	}

	return &batch, consumererror.CombineErrors(errs)
}

[jack-berg]

Woops gotta change *exportMetadata to exportMetadata.

[a-feld]

Wouldn't that mean we wouldn't be able to modify attributes in the details? 🤔

[jack-berg]

Actually, maybe not. export has to have a pointer for exportMetadata because it needs to change its values.

I think that suggests we should use pointers to *exportMetadata throughout for consistency, including in metrics.go.

[jack-berg]

Do you want to return a pointer or value here? If pointer, should we make all the methods attached to type exporter work off pointers as well? Or does that not matter?

[a-feld]

If we change to a value type, that means we can no longer return nil. Is that preferable? 🤔