Use the github.com/lightstep/go-expohisto exponential histogram mapping functions

cmd/otelcorecol/go.mod

@@ -40,6 +40,7 @@ require (
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/klauspost/compress v1.15.13 // indirect
 	github.com/knadh/koanf v1.4.4 // indirect
+	github.com/lightstep/go-expohisto v1.0.0 // indirect
 	github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.2 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect

cmd/otelcorecol/go.sum

@@ -271,6 +271,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/lightstep/go-expohisto v1.0.0 h1:UPtTS1rGdtehbbAF7o/dhkWLTDI73UifG8LbfQI7cA4=
+github.com/lightstep/go-expohisto v1.0.0/go.mod h1:xDXD0++Mu2FOaItXtdDfksfgxfV0z1TMPa+e/EUd0cs=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=

exporter/loggingexporter/go.mod

@@ -3,6 +3,7 @@ module go.opentelemetry.io/collector/exporter/loggingexporter
 go 1.18
 
 require (
+	github.com/lightstep/go-expohisto v1.0.0
 	github.com/stretchr/testify v1.8.1
 	go.opentelemetry.io/collector v0.67.0
 	go.opentelemetry.io/collector/component v0.67.0

exporter/loggingexporter/go.sum

@@ -174,6 +174,8 @@ github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfn
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/lightstep/go-expohisto v1.0.0 h1:UPtTS1rGdtehbbAF7o/dhkWLTDI73UifG8LbfQI7cA4=
+github.com/lightstep/go-expohisto v1.0.0/go.mod h1:xDXD0++Mu2FOaItXtdDfksfgxfV0z1TMPa+e/EUd0cs=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
 github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=

exporter/loggingexporter/internal/otlptext/databuffer.go

@@ -17,7 +17,6 @@ package otlptext // import "go.opentelemetry.io/collector/exporter/loggingexport
 import (
 	"bytes"
 	"fmt"
-	"math"
 	"strings"
 
 	"go.opentelemetry.io/collector/pdata/pcommon"
@@ -170,26 +169,18 @@ func (b *dataBuffer) logExponentialHistogramDataPoints(ps pmetric.ExponentialHis
 			b.logEntry("Max: %f", p.Max())
 		}
 
-		scale := int(p.Scale())
-		factor := math.Ldexp(math.Ln2, -scale)
-		// Note: the equation used here, which is
-		//   math.Exp(index * factor)
-		// reports +Inf as the _lower_ boundary of the bucket nearest
-		// infinity, which is incorrect and can be addressed in various
-		// ways.  The OTel-Go implementation of this histogram pending
-		// in https://github.com/open-telemetry/opentelemetry-go/pull/2393
-		// uses a lookup table for the last finite boundary, which can be
-		// easily computed using `math/big` (for scales up to 20).
+		m := newExpoHistoMapping(p.Scale())
 
 		negB := p.Negative().BucketCounts()
 		posB := p.Positive().BucketCounts()
 
 		for i := 0; i < negB.Len(); i++ {
 			pos := negB.Len() - i - 1
 			index := p.Negative().Offset() + int32(pos)
-			lower := math.Exp(float64(index) * factor)
-			upper := math.Exp(float64(index+1) * factor)
-			b.logEntry("Bucket (%f, %f], Count: %d", -upper, -lower, negB.At(pos))
+			b.logEntry("Bucket [%s, %s), Count: %d",
+				m.stringLowerBoundary(index+1, true),
+				m.stringLowerBoundary(index, true),
+				negB.At(pos))
 		}
 
 		if p.ZeroCount() != 0 {
@@ -198,9 +189,10 @@ func (b *dataBuffer) logExponentialHistogramDataPoints(ps pmetric.ExponentialHis
 
 		for pos := 0; pos < posB.Len(); pos++ {
 			index := p.Positive().Offset() + int32(pos)
-			lower := math.Exp(float64(index) * factor)
-			upper := math.Exp(float64(index+1) * factor)
-			b.logEntry("Bucket [%f, %f), Count: %d", lower, upper, posB.At(pos))
+			b.logEntry("Bucket (%s, %s], Count: %d",
+				m.stringLowerBoundary(index, false),
+				m.stringLowerBoundary(index+1, false),
+				posB.At(pos))
 		}
 	}
 }

exporter/loggingexporter/internal/otlptext/metrics.go

@@ -14,7 +14,23 @@
 
 package otlptext // import "go.opentelemetry.io/collector/exporter/loggingexporter/internal/otlptext"
 
-import "go.opentelemetry.io/collector/pdata/pmetric"
+import (
+	"fmt"
+
+	expohisto "github.com/lightstep/go-expohisto/mapping"
+	"github.com/lightstep/go-expohisto/mapping/exponent"
+	"github.com/lightstep/go-expohisto/mapping/logarithm"
+
+	"go.opentelemetry.io/collector/pdata/pmetric"
+)
+
+// greatestBoundary equals and is the smallest unrepresentable float64
+// greater than 1.  See TestLastBoundary for the definition in code.
+const greatestBoundary = "1.79769e+308"
+
+// boundaryFormat is used with Sprintf() to format exponential
+// histogram boundaries.
+const boundaryFormat = "%.6g"
 
 // NewTextMetricsMarshaler returns a pmetric.Marshaler to encode to OTLP text bytes.
 func NewTextMetricsMarshaler() pmetric.Marshaler {
@@ -50,3 +66,80 @@ func (textMetricsMarshaler) MarshalMetrics(md pmetric.Metrics) ([]byte, error) {
 
 	return buf.buf.Bytes(), nil
 }
+
+type expoHistoMapping struct {
+	scale   int32
+	mapping expohisto.Mapping
+}
+
+func newExpoHistoMapping(scale int32) expoHistoMapping {
+	m := expoHistoMapping{
+		scale: scale,
+	}
+	if scale >= exponent.MinScale && scale <= exponent.MaxScale {
+		m.mapping, _ = exponent.NewMapping(scale)
+	} else if scale >= logarithm.MinScale && scale <= logarithm.MaxScale {
+		m.mapping, _ = logarithm.NewMapping(scale)
+	}
+	return m
+}
+
+func (ehm expoHistoMapping) stringLowerBoundary(idx int32, neg bool) string {
+	// Use the go-expohisto mapping functions provided the scale and
+	// index are in range.
+	if ehm.mapping != nil {
+		if bound, err := ehm.mapping.LowerBoundary(idx); err == nil {
+			// If the boundary is a subnormal number, fmt.Sprintf()
+			// won't help, use the generic fallback below.
+			if bound >= 0x1p-1022 {
+				if neg {
+					bound = -bound
+				}
+				return fmt.Sprintf(boundaryFormat, bound)
+			}
+		}
+	}
+
+	var s string
+	switch {
+	case idx == 0:
+		s = "1"
+	case idx > 0:
+		// Note: at scale 20, the value (1<<30) leads to exponent 1024
+		// The following expression generalizes this for valid scales.
+		if ehm.scale >= -10 && ehm.scale <= 20 && int64(idx)<<(20-ehm.scale) == 1<<30 {
+			// Important special case equal to 0x1p1024 is
+			// the upper boundary of the last valid bucket
+			// at all scales.
+			s = greatestBoundary
+		} else {
+			s = "OVERFLOW"
+		}
+	default:
+		// Note: corner cases involving subnormal values may
+		// be handled here.  These are considered out of range
+		// by the go-expohisto mapping functions, which will return
+		// an underflow error for buckets that are entirely
+		// outside the normal range.  These measurements are not
+		// necessarily invalid, but they are extra work to compute.
+		//
+		// There is one case that falls through to this branch
+		// to describe the lower boundary of a bucket
+		// containing a normal value.  Because the value at
+		// the subnormal boundary 0x1p-1022 falls into a
+		// bucket (X, 0x1p-1022], where the subnormal value X
+		// depends on scale.  The fallthrough here means we
+		// print that bucket as "(UNDERFLOW, 2.22507e-308]",
+		// (note 0x1p-1022 == 2.22507e-308).  This is correct
+		// with respect to the reference implementation, which
+		// first rounds subnormal values up to 0x1p-1022.  The
+		// result (for the reference implementation) is all
+		// subnormal values will fall into the bucket that
+		// prints "(UNDERFLOW, 2.22507e-308]".
+		s = "UNDERFLOW"
+	}
+	if neg {
+		s = "-" + s
+	}
+	return s
+}

exporter/loggingexporter/internal/otlptext/metrics_test.go

@@ -15,11 +15,15 @@
 package otlptext
 
 import (
+	"fmt"
+	"math/big"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/lightstep/go-expohisto/mapping/exponent"
+	"github.com/lightstep/go-expohisto/mapping/logarithm"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -65,3 +69,105 @@ func TestMetricsText(t *testing.T) {
 		})
 	}
 }
+
+func TestExpoHistoOverflowMapping(t *testing.T) {
+	// Compute the string value of 0x1p+1024
+	expect := expectGreatestBoundary()
+
+	// For positive scales, the +Inf threshold happens at 1024<<scale
+	for scale := logarithm.MinScale; scale <= logarithm.MaxScale; scale++ {
+		m := newExpoHistoMapping(scale)
+		threshold := int32(1024) << scale
+
+		// Check that this can't be mapped to a float64.
+		_, err := m.mapping.LowerBoundary(threshold)
+		require.Error(t, err)
+
+		// Require the special case.
+		require.Equal(t, expect, m.stringLowerBoundary(threshold, false))
+		require.Equal(t, "-"+expect, m.stringLowerBoundary(threshold, true))
+
+		require.Equal(t, "OVERFLOW", m.stringLowerBoundary(threshold+1, false))
+		require.Equal(t, "-OVERFLOW", m.stringLowerBoundary(threshold+1, true))
+	}
+	// For scales <= 0, the +Inf threshold happens at 1024>>-scale
+	for scale := exponent.MinScale; scale <= exponent.MaxScale; scale++ {
+		m := newExpoHistoMapping(scale)
+		threshold := int32(1024) >> -scale
+
+		require.Equal(t, expect, m.stringLowerBoundary(threshold, false))
+		require.Equal(t, "-"+expect, m.stringLowerBoundary(threshold, true))
+
+		require.Equal(t, "OVERFLOW", m.stringLowerBoundary(threshold+1, false))
+		require.Equal(t, "-OVERFLOW", m.stringLowerBoundary(threshold+1, true))
+	}
+
+	// For an invalid scale, any positive index overflows
+	invalid := newExpoHistoMapping(100)
+
+	require.Equal(t, "OVERFLOW", invalid.stringLowerBoundary(1, false))
+	require.Equal(t, "-OVERFLOW", invalid.stringLowerBoundary(1, true))
+
+	// But index 0 always works
+	require.Equal(t, "1", invalid.stringLowerBoundary(0, false))
+	require.Equal(t, "-1", invalid.stringLowerBoundary(0, true))
+}
+
+func TestExpoHistoUnderflowMapping(t *testing.T) {
+	// For all valid scales
+	for scale := int32(-10); scale <= 20; scale++ {
+		m := newExpoHistoMapping(scale)
+
+		// The following +1 gives us the index whose lower
+		// boundary equals 0x1p-1022.
+		idx := m.mapping.MapToIndex(0x1p-1022) + 1
+		lb, err := m.mapping.LowerBoundary(idx)
+		require.NoError(t, err)
+
+		require.Equal(t, fmt.Sprintf(boundaryFormat, lb), m.stringLowerBoundary(idx, false))
+		require.Equal(t, fmt.Sprintf("-"+boundaryFormat, lb), m.stringLowerBoundary(idx, true))
+
+		require.Equal(t, "UNDERFLOW", m.stringLowerBoundary(idx-1, false))
+		require.Equal(t, "-UNDERFLOW", m.stringLowerBoundary(idx-1, true))
+	}
+
+	// For an invalid scale, any positive index overflows
+	invalid := newExpoHistoMapping(100)
+
+	require.Equal(t, "UNDERFLOW", invalid.stringLowerBoundary(-1, false))
+	require.Equal(t, "-UNDERFLOW", invalid.stringLowerBoundary(-1, true))
+}
+
+// expectGreatestBoundary is the definitive logic to compute the
+// decimal value for 0x1p1024, which has float64 representation equal
+// to +Inf so we can't use ordinary logic to format.
+func expectGreatestBoundary() string {
+	b := big.NewFloat(1)
+	b.SetMantExp(b, 1024)
+	return b.Text('g', 6)
+}
+
+// TestBoundaryFormatting verifies greatestBoundary is hard-coded
+// correctly and formatted like normal boundaries.
+func TestBoundaryFormatting(t *testing.T) {
+	// Require that the formatting of greatestBoundary should
+	// match the formatting of normal boundaries.  Change the call
+	// to b.Text() in expectGreatestBoundary to match
+	// boundaryFormat then update this test.
+	require.Equal(t, boundaryFormat, "%.6g")
+
+	// Require the formatting of "1" to match the boundaryFormat.
+	invalid := newExpoHistoMapping(100)
+	require.Equal(t, fmt.Sprintf(boundaryFormat, 1.0), invalid.stringLowerBoundary(0, false))
+
+	// Require the correct hard-coded value.
+	require.Equal(t, expectGreatestBoundary(), greatestBoundary)
+
+	// Verify that 0x1p-1022 falls into a bucket that formats like
+	// (UNDERFLOW, 2.22507e-308]
+	valid := newExpoHistoMapping(0)
+	boundIdx := valid.stringLowerBoundary(-1022, false)
+	lowIdx := valid.stringLowerBoundary(-1023, false)
+	require.Equal(t, fmt.Sprintf(boundaryFormat, 0x1p-1022), boundIdx)
+	require.Equal(t, "UNDERFLOW", lowIdx)
+}

exporter/loggingexporter/internal/otlptext/testdata/metrics/metrics_with_all_types.out

@@ -127,11 +127,11 @@ StartTimestamp: 2020-02-11 20:26:12.000000321 +0000 UTC
 Timestamp: 2020-02-11 20:26:13.000000789 +0000 UTC
 Count: 5
 Sum: 0.150000
-Bucket (-1.414214, -1.000000], Count: 1
-Bucket (-1.000000, -0.707107], Count: 1
+Bucket [-1.41421, -1), Count: 1
+Bucket [-1, -0.707107), Count: 1
 Bucket [0, 0], Count: 1
-Bucket [1.414214, 2.000000), Count: 1
-Bucket [2.000000, 2.828427), Count: 1
+Bucket (1.41421, 2], Count: 1
+Bucket (2, 2.82843], Count: 1
 ExponentialHistogramDataPoints #1
 Data point attributes:
      -> label-2: Str(label-value-2)
@@ -142,8 +142,8 @@ Sum: 1.250000
 Min: 0.000000
 Max: 1.000000
 Bucket [0, 0], Count: 1
-Bucket [0.250000, 1.000000), Count: 1
-Bucket [1.000000, 4.000000), Count: 1
+Bucket (0.25, 1], Count: 1
+Bucket (1, 4], Count: 1
 Metric #6
 Descriptor:
      -> Name: summary

internal/testdata/metric.go

@@ -218,19 +218,25 @@ func initExponentialHistogramMetric(hm pmetric.Metric) {
 	hdp0.SetZeroCount(1)
 	hdp0.SetScale(1)
 
-	// positive index 1 and 2 are values sqrt(2), 2 at scale 1
+	// at scale 1,
+	// positive bucket index 0 is (1, sqrt(2)] (not represented)
+	// positive bucket index 1 is (sqrt(2), 2]
+	// positive bucket index 2 is (2, 2*sqrt(2)]
 	hdp0.Positive().SetOffset(1)
 	hdp0.Positive().BucketCounts().FromRaw([]uint64{1, 1})
-	// negative index -1 and 0 are values -1/sqrt(2), -1 at scale 1
+
+	// at scale 1
+	// negative bucket index 0 is -(1, sqrt(2)]
+	// negative bucket index -1 is -(sqrt(2)/2, 1]
 	hdp0.Negative().SetOffset(-1)
 	hdp0.Negative().BucketCounts().FromRaw([]uint64{1, 1})
 
 	// The above will print:
-	// Bucket (-1.414214, -1.000000], Count: 1
-	// Bucket (-1.000000, -0.707107], Count: 1
+	// Bucket [-1.41421, -1), Count: 1
+	// Bucket [-1, -0.707107), Count: 1
 	// Bucket [0, 0], Count: 1
-	// Bucket [0.707107, 1.000000), Count: 1
-	// Bucket [1.000000, 1.414214), Count: 1
+	// Bucket (1.41421, 2], Count: 1
+	// Bucket (2, 2.82843], Count: 1
 
 	hdp1 := hdps.AppendEmpty()
 	initMetricAttributes2(hdp1.Attributes())
@@ -249,8 +255,8 @@ func initExponentialHistogramMetric(hm pmetric.Metric) {
 
 	// The above will print:
 	// Bucket [0, 0], Count: 1
-	// Bucket [0.250000, 1.000000), Count: 1
-	// Bucket [1.000000, 4.000000), Count: 1
+	// Bucket (0.25, 1], Count: 1
+	// Bucket (1, 4], Count: 1
 
 	exemplar := hdp1.Exemplars().AppendEmpty()
 	exemplar.SetTimestamp(metricExemplarTimestamp)