Remove the MinMaxSumCount from cortex and datadog exporter (#1554)

* Remove the MinMaxSumCount from cortex * add changelog * remove wantMMSCTimeSeries unittest * fix ExampleExporter unit test error * fix num of aggregations to 4. * update changelog * change the instrument into Count instead of Histogram * improve time to 2 sec.
open-telemetry · Jan 10, 2022 · 390c3c6 · 390c3c6
1 parent 576a237
commit 390c3c6
Show file tree

Hide file tree

Showing 7 changed files with 11 additions and 173 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Change the `http-server-duration` instrument in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` to record milliseconds instead of microseconds match what is specified in the OpenTelemetry specification. (#1414, #1537)
 
+### Removed
+
+- Remove the MinMaxSumCount from cortex and datadog exporter. (#1554)
+
 ## [1.3.0/0.28.0] - 2021-12-10
 
 ### ⚠️ Notice ⚠️

diff --git a/exporters/metric/cortex/README.md b/exporters/metric/cortex/README.md
@@ -277,12 +277,11 @@ that instruments are mapped to aggregations as shown in the table below.
 | ValueObserver     | Histogram   |
 
 Although only the `Sum` and `Histogram` aggregations are currently being used, the
-exporter supports 5 different aggregations:
+exporter supports 4 different aggregations:
 1. `Sum`
 2. `LastValue`
-3. `MinMaxSumCount`
-4. `Distribution`
-5. `Histogram`
+3. `Distribution`
+4. `Histogram`
 
 ## Error Handling
 In general, errors are returned to the calling function / method. Eventually, errors make

diff --git a/exporters/metric/cortex/cortex.go b/exporters/metric/cortex/cortex.go
@@ -160,13 +160,6 @@ func (e *Exporter) ConvertToTimeSeries(res *resource.Resource, checkpointSet exp
 					return err
 				}
 				timeSeries = append(timeSeries, tSeries)
-				if minMaxSumCount, ok := agg.(aggregation.MinMaxSumCount); ok {
-					tSeries, err := convertFromMinMaxSumCount(edata, minMaxSumCount)
-					if err != nil {
-						return err
-					}
-					timeSeries = append(timeSeries, tSeries...)
-				}
 			} else if lastValue, ok := agg.(aggregation.LastValue); ok {
 				tSeries, err := convertFromLastValue(edata, lastValue)
 				if err != nil {
@@ -238,42 +231,6 @@ func convertFromLastValue(edata exportData, lastValue aggregation.LastValue) (pr
 	return tSeries, nil
 }
 
-// convertFromMinMaxSumCount returns 4 TimeSeries for the min, max, sum, and count from the mmsc aggregation
-func convertFromMinMaxSumCount(edata exportData, minMaxSumCount aggregation.MinMaxSumCount) ([]prompb.TimeSeries, error) {
-	numberKind := edata.Descriptor().NumberKind()
-
-	// Convert Min
-	min, err := minMaxSumCount.Min()
-	if err != nil {
-		return nil, err
-	}
-	name := sanitize(edata.Descriptor().Name() + "_min")
-	minTimeSeries := createTimeSeries(edata, min, numberKind, attribute.String("__name__", name))
-
-	// Convert Max
-	max, err := minMaxSumCount.Max()
-	if err != nil {
-		return nil, err
-	}
-	name = sanitize(edata.Descriptor().Name() + "_max")
-	maxTimeSeries := createTimeSeries(edata, max, numberKind, attribute.String("__name__", name))
-
-	// Convert Count
-	count, err := minMaxSumCount.Count()
-	if err != nil {
-		return nil, err
-	}
-	name = sanitize(edata.Descriptor().Name() + "_count")
-	countTimeSeries := createTimeSeries(edata, number.NewInt64Number(int64(count)), number.Int64Kind, attribute.String("__name__", name))
-
-	// Return all timeSeries
-	tSeries := []prompb.TimeSeries{
-		minTimeSeries, maxTimeSeries, countTimeSeries,
-	}
-
-	return tSeries, nil
-}
-
 // convertFromHistogram returns len(histogram.Buckets) timeseries for a histogram aggregation
 func convertFromHistogram(edata exportData, histogram aggregation.Histogram) ([]prompb.TimeSeries, error) {
 	var timeSeries []prompb.TimeSeries

diff --git a/exporters/metric/cortex/cortex_test.go b/exporters/metric/cortex/cortex_test.go
@@ -105,12 +105,6 @@ func TestConvertToTimeSeries(t *testing.T) {
 			want:       wantLastValueTimeSeries,
 			wantLength: 1,
 		},
-		{
-			name:       "convertFromMinMaxSumCount",
-			input:      getMMSCReader(t, 123.456, 876.543),
-			want:       wantMMSCTimeSeries,
-			wantLength: 4,
-		},
 		{
 			name:       "convertFromHistogram",
 			input:      getHistogramReader(t),

diff --git a/exporters/metric/cortex/testutil_test.go b/exporters/metric/cortex/testutil_test.go
@@ -30,7 +30,6 @@ import (
 	"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/histogram"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue"
-	"go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/sum"
 	controller "go.opentelemetry.io/otel/sdk/metric/controller/basic"
 	processor "go.opentelemetry.io/otel/sdk/metric/processor/basic"
@@ -55,11 +54,6 @@ func (testAggregatorSelector) AggregatorFor(desc *sdkapi.Descriptor, aggPtrs ...
 		for i := range aggPtrs {
 			*aggPtrs[i] = &aggs[i]
 		}
-	case strings.HasSuffix(desc.Name(), "_mmsc"):
-		aggs := minmaxsumcount.New(len(aggPtrs), desc)
-		for i := range aggPtrs {
-			*aggPtrs[i] = &aggs[i]
-		}
 	case strings.HasSuffix(desc.Name(), "_lastvalue"):
 		aggs := lastvalue.New(len(aggPtrs))
 		for i := range aggPtrs {
@@ -115,21 +109,6 @@ func getLastValueReader(t *testing.T, values ...int64) export.InstrumentationLib
 	return cont
 }
 
-// getMMSCReader returns a checkpoint set with a minmaxsumcount aggregation record
-func getMMSCReader(t *testing.T, values ...float64) export.InstrumentationLibraryReader {
-	ctx, meter, cont := testMeter(t)
-
-	histo := metric.Must(meter).NewFloat64Histogram("metric_mmsc")
-
-	for _, value := range values {
-		histo.Record(ctx, value)
-	}
-
-	require.NoError(t, cont.Collect(ctx))
-
-	return cont
-}
-
 // getHistogramReader returns a checkpoint set with a histogram aggregation record
 func getHistogramReader(t *testing.T) export.InstrumentationLibraryReader {
 	ctx, meter, cont := testMeter(t)
@@ -186,73 +165,6 @@ var wantLastValueTimeSeries = []*prompb.TimeSeries{
 	},
 }
 
-var wantMMSCTimeSeries = []*prompb.TimeSeries{
-	{
-		Labels: []prompb.Label{
-			{
-				Name:  "R",
-				Value: "V",
-			},
-			{
-				Name:  "__name__",
-				Value: "metric_mmsc",
-			},
-		},
-		Samples: []prompb.Sample{{
-			Value: 999.999,
-			// Timestamp: this test verifies real timestamps
-		}},
-	},
-	{
-		Labels: []prompb.Label{
-			{
-				Name:  "R",
-				Value: "V",
-			},
-			{
-				Name:  "__name__",
-				Value: "metric_mmsc_min",
-			},
-		},
-		Samples: []prompb.Sample{{
-			Value: 123.456,
-			// Timestamp: this test verifies real timestamps
-		}},
-	},
-	{
-		Labels: []prompb.Label{
-			{
-				Name:  "R",
-				Value: "V",
-			},
-			{
-				Name:  "__name__",
-				Value: "metric_mmsc_max",
-			},
-		},
-		Samples: []prompb.Sample{{
-			Value: 876.543,
-			// Timestamp: this test verifies real timestamps
-		}},
-	},
-	{
-		Labels: []prompb.Label{
-			{
-				Name:  "R",
-				Value: "V",
-			},
-			{
-				Name:  "__name__",
-				Value: "metric_mmsc_count",
-			},
-		},
-		Samples: []prompb.Sample{{
-			Value: 2,
-			// Timestamp: this test verifies real timestamps
-		}},
-	},
-}
-
 var wantHistogramTimeSeries = []*prompb.TimeSeries{
 	{
 		Labels: []prompb.Label{

diff --git a/exporters/metric/datadog/datadog.go b/exporters/metric/datadog/datadog.go
@@ -123,30 +123,6 @@ func (e *Exporter) Export(ctx context.Context, res *resource.Resource, ilr expor
 						return fmt.Errorf("error submitting %s point: %w", name, err)
 					}
 				}
-			case aggregation.MinMaxSumCount:
-				type record struct {
-					name string
-					f    func() (number.Number, error)
-				}
-				recs := []record{
-					{
-						name: name + ".min",
-						f:    agg.Min,
-					},
-					{
-						name: name + ".max",
-						f:    agg.Max,
-					},
-				}
-				for _, rec := range recs {
-					val, err := rec.f()
-					if err != nil {
-						return fmt.Errorf("error getting MinMaxSumCount value for %s: %w", name, err)
-					}
-					if err := e.client.Gauge(rec.name, metricValue(r.Descriptor().NumberKind(), val), tags, rate); err != nil {
-						return fmt.Errorf("error submitting %s point: %w", name, err)
-					}
-				}
 			case aggregation.Sum:
 				val, err := agg.Sum()
 				if err != nil {

diff --git a/exporters/metric/datadog/example_test.go b/exporters/metric/datadog/example_test.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"net"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/DataDog/datadog-go/statsd"
@@ -72,7 +71,7 @@ func ExampleExporter() {
 		defer func() { handleErr(cont.Stop(ctx)) }()
 		global.SetMeterProvider(cont)
 		meter := global.Meter("marwandist")
-		m := metric.Must(meter).NewInt64Histogram("myrecorder")
+		m := metric.Must(meter).NewInt64Counter("mycounter")
 		meter.RecordBatch(context.Background(), []attribute.KeyValue{attribute.Int("l", 1)},
 			m.Measurement(1), m.Measurement(50), m.Measurement(100))
 	}()
@@ -90,21 +89,18 @@ func ExampleExporter() {
 			// specifics of OpenTelemetry aggregator calculations
 			// "max" is something that will always exist and always be the same
 			statLine := string(d)
-			if strings.HasPrefix(statLine, "myrecorder.max") {
-				fmt.Println(statLine)
-				return
-			}
+			fmt.Println(statLine)
 		case <-timedOutChan:
 			_, _ = fmt.Fprintln(os.Stderr, "Server timed out waiting for packets")
 			return
-		case <-time.After(1 * time.Second):
+		case <-time.After(2 * time.Second):
 			fmt.Println("no data received after 1 second")
 			return
 		}
 	}
 
 	// Output:
-	// myrecorder.max:100|g|#env:dev,l:1,service.name:ExampleExporter,telemetry.sdk.language:go,telemetry.sdk.name:opentelemetry,telemetry.sdk.version:1.2.0
+	// mycounter:151|c|#env:dev,l:1,service.name:ExampleExporter,telemetry.sdk.language:go,telemetry.sdk.name:opentelemetry,telemetry.sdk.version:1.2.0
 	//
 }