work around lightstep histogram issue

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.2-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-7.5-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

kotlin/goodmetrics/src/main/kotlin/goodmetrics/downstream/OpentelemetryClient.kt

@@ -23,6 +23,7 @@ import goodmetrics.io.opentelemetry.proto.resource.v1.resource
 import goodmetrics.pipeline.AggregatedBatch
 import goodmetrics.pipeline.Aggregation
 import goodmetrics.pipeline.bucket
+import goodmetrics.pipeline.bucketBelow
 import io.grpc.CallOptions
 import io.grpc.ClientInterceptor
 import io.grpc.Deadline
@@ -255,7 +256,25 @@ class OpentelemetryClient(
                 startTimeUnixNano = timestampNanos - (System.nanoTime() - startNanoTime) // approximate, whatever.
                 timeUnixNano = timestampNanos
                 count = 1
-                explicitBounds.add(bucket(value).toDouble())
+
+                val bucketValue = bucket(value)
+                if (0 < bucketValue) {
+                    // This little humdinger is here so Lightstep can interpret the boundary for the _real_ measurement
+                    // below. It's similar to the 0 that opentelemetry demands, but different in that it is actually a
+                    // reasonable ask.
+                    // Lightstep has an internal representation of histograms & while I don't pretend  to understand
+                    // how they've implemented them, they told me that they interpret the absence of a lower bounding
+                    // bucket as an infinite lower bound. That's not consistent with my read of otlp BUT it makes
+                    // infinitely more sense than imposing an upper infinity bucket upon your protocol.
+                    // Prometheus is a cataclysm from which there is no redemption: It ruins developers' minds with
+                    // its broken and much lauded blunders; it shames my profession by its protocol as well as those
+                    // spawned through its vile influence and disappoints the thoughtful by its existence.
+                    // But, you know, this particular thing for Lightstep seems fine because there's technical merit.
+                    explicitBounds.add(bucketBelow(value).toDouble())
+                    bucketCounts.add(0)
+                }
+
+                explicitBounds.add(bucketValue.toDouble())
                 bucketCounts.add(1)
                 bucketCounts.add(0) // otlp go die in a fire
             }
@@ -275,9 +294,22 @@ class OpentelemetryClient(
                 startTimeUnixNano = timestampNanos - aggregationWidth.inWholeNanoseconds
                 timeUnixNano = timestampNanos
                 val sorted = this@asOtlpHistogram.bucketCounts.toSortedMap()
+
+                if (sorted.isNotEmpty() && 0 < sorted.firstKey()) {
+                    // Again, a reasonable request from Lightstep to work around their lower-infinity interpretation
+                    // of histogram data. Goodmetrics sends sparse histograms and if lightstep has further issues
+                    // parsing these I might have to expand this to fill in 0's for "missing" buckets or delimit
+                    // empty ranges with 0'd out buckets. They haven't asked for that yet though so I'm not doing it.
+                    explicitBounds.add(bucketBelow(sorted.firstKey()).toDouble())
+                    bucketCounts.add(0)
+                }
+
                 count = this@asOtlpHistogram.bucketCounts.values.sumOf { it.sum() }
-                explicitBounds.addAll(sorted.keys.asSequence().map { it.toDouble() }.asIterable())
-                bucketCounts.addAll(sorted.values.asSequence().map { it.sum() }.asIterable())
+                for ((bucket, count) in sorted) {
+                    explicitBounds.add(bucket.toDouble())
+                    bucketCounts.add(count.sum())
+                }
+
                 bucketCounts.add(0) // because OTLP is _stupid_ and defined histogram format to have an implicit infinity bucket.
             }
         )

kotlin/goodmetrics/src/main/kotlin/goodmetrics/pipeline/Aggregator.kt

@@ -140,7 +140,7 @@ fun Metrics.dimensionPosition(): DimensionPosition {
 }
 
 fun bucket(value: Long): Long {
-    if (value < 100L) return value
+    if (value < 100L) return max(0, value)
     val power = log(value.toDouble(), 10.0)
     val effectivePower = max(0, (power - 1).toInt())
     val trashColumn = 10.0.pow(effectivePower).toLong()
@@ -152,6 +152,16 @@ fun bucket(value: Long): Long {
     }
 }
 
+fun bucketBelow(valueIn: Long): Long {
+    val value = valueIn - 1
+    if (value < 100L) return max(0, value)
+    val power = log(value.toDouble(), 10.0)
+    val effectivePower = max(0, (power - 0.00001 - 1).toInt())
+    val trashColumn = 10.0.pow(effectivePower).toLong()
+    val trash = value % trashColumn
+    return value - trash
+}
+
 sealed interface Aggregation {
     data class Histogram(
         val bucketCounts: ConcurrentHashMap<Long, LongAdder> = ConcurrentHashMap(),

kotlin/goodmetrics/src/test/kotlin/goodmetrics/pipeline/AggregatorKtTest.kt

@@ -7,6 +7,9 @@ class AggregatorKtTest {
     private fun assertBucket(expected: Long, value: Long) {
         assertEquals(expected, bucket(value), "bucket( $value )")
     }
+    private fun assertBucketBelow(expected: Long, value: Long) {
+        assertEquals(expected, bucketBelow(value), "bucketBelow( $value )")
+    }
 
     @Test
     fun testBucket() {
@@ -21,8 +24,38 @@ class AggregatorKtTest {
         assertBucket(110, 101)
         assertBucket(110, 109)
         assertBucket(110, 110)
+        assertBucket(120, 120)
+        assertBucket(130, 121)
+        assertBucket(130, 123)
+        assertBucket(130, 129)
+        assertBucket(130, 130)
         assertBucket(120, 111)
         assertBucket(8900, 8891)
         assertBucket(8900, 8891)
     }
+
+    @Test
+    fun testBucketBelow() {
+        assertBucketBelow(0, 0)
+        assertBucketBelow(0, 1)
+        assertBucketBelow(1, 2)
+        assertBucketBelow(8, 9)
+        assertBucketBelow(9, 10)
+        assertBucketBelow(10, 11)
+        assertBucketBelow(98, 99)
+        assertBucketBelow(99, 100)
+        assertBucketBelow(100, 101)
+        assertBucketBelow(100, 109)
+        assertBucketBelow(100, 110)
+        assertBucketBelow(110, 111)
+        assertBucketBelow(110, 120)
+        assertBucketBelow(120, 121)
+        assertBucketBelow(120, 123)
+        assertBucketBelow(120, 129)
+        assertBucketBelow(120, 130)
+        assertBucketBelow(130, 131)
+        assertBucketBelow(8800, 8891)
+        assertBucketBelow(8800, 8891)
+        assertBucketBelow(8900, 8901)
+    }
 }