diff --git a/cmd/otelcorecol/go.mod b/cmd/otelcorecol/go.mod index 026b7a9d89f..0c092b741d2 100644 --- a/cmd/otelcorecol/go.mod +++ b/cmd/otelcorecol/go.mod @@ -41,6 +41,7 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.15.15 // indirect github.com/knadh/koanf v1.5.0 // 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.4 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect diff --git a/cmd/otelcorecol/go.sum b/cmd/otelcorecol/go.sum index ad5e547fa81..968d732e024 100644 --- a/cmd/otelcorecol/go.sum +++ b/cmd/otelcorecol/go.sum @@ -272,6 +272,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= diff --git a/exporter/loggingexporter/go.mod b/exporter/loggingexporter/go.mod index 5d7129f35b9..df09756dc47 100644 --- a/exporter/loggingexporter/go.mod +++ b/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.70.0 go.opentelemetry.io/collector/component v0.70.0 diff --git a/exporter/loggingexporter/go.sum b/exporter/loggingexporter/go.sum index 6ad1b3ae0fe..7c298c52483 100644 --- a/exporter/loggingexporter/go.sum +++ b/exporter/loggingexporter/go.sum @@ -168,6 +168,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= diff --git a/exporter/loggingexporter/internal/otlptext/databuffer.go b/exporter/loggingexporter/internal/otlptext/databuffer.go index d6001d339fc..b446c3a0bc8 100644 --- a/exporter/loggingexporter/internal/otlptext/databuffer.go +++ b/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,16 +169,7 @@ 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() @@ -187,9 +177,10 @@ func (b *dataBuffer) logExponentialHistogramDataPoints(ps pmetric.ExponentialHis 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)) } } } diff --git a/exporter/loggingexporter/internal/otlptext/metrics.go b/exporter/loggingexporter/internal/otlptext/metrics.go index 639474663c8..c811b8f919a 100644 --- a/exporter/loggingexporter/internal/otlptext/metrics.go +++ b/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 +} diff --git a/exporter/loggingexporter/internal/otlptext/metrics_test.go b/exporter/loggingexporter/internal/otlptext/metrics_test.go index bb010764573..5b3eb89ec69 100644 --- a/exporter/loggingexporter/internal/otlptext/metrics_test.go +++ b/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 := 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) +} diff --git a/exporter/loggingexporter/internal/otlptext/testdata/metrics/metrics_with_all_types.out b/exporter/loggingexporter/internal/otlptext/testdata/metrics/metrics_with_all_types.out index db291f3f22d..09050b58703 100644 --- a/exporter/loggingexporter/internal/otlptext/testdata/metrics/metrics_with_all_types.out +++ b/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 diff --git a/internal/testdata/metric.go b/internal/testdata/metric.go index ce682d47d60..fbb8723157b 100644 --- a/internal/testdata/metric.go +++ b/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)