diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index aa991fc..8049c68 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/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 diff --git a/kotlin/goodmetrics/src/main/kotlin/goodmetrics/downstream/OpentelemetryClient.kt b/kotlin/goodmetrics/src/main/kotlin/goodmetrics/downstream/OpentelemetryClient.kt index 29b81b9..80e4ab6 100644 --- a/kotlin/goodmetrics/src/main/kotlin/goodmetrics/downstream/OpentelemetryClient.kt +++ b/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. } ) diff --git a/kotlin/goodmetrics/src/main/kotlin/goodmetrics/pipeline/Aggregator.kt b/kotlin/goodmetrics/src/main/kotlin/goodmetrics/pipeline/Aggregator.kt index 717b041..2251f10 100644 --- a/kotlin/goodmetrics/src/main/kotlin/goodmetrics/pipeline/Aggregator.kt +++ b/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 = ConcurrentHashMap(), diff --git a/kotlin/goodmetrics/src/test/kotlin/goodmetrics/pipeline/AggregatorKtTest.kt b/kotlin/goodmetrics/src/test/kotlin/goodmetrics/pipeline/AggregatorKtTest.kt index b2330cb..aff0f83 100644 --- a/kotlin/goodmetrics/src/test/kotlin/goodmetrics/pipeline/AggregatorKtTest.kt +++ b/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) + } }