Skip to content

Commit

Permalink
Add Base2 prefix to internal exponential histogram classes (#5179)
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-berg authored Feb 7, 2023
1 parent 07e5654 commit feb0297
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 94 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
import io.opentelemetry.sdk.metrics.View;
import io.opentelemetry.sdk.metrics.internal.view.AttributesProcessor;
import io.opentelemetry.sdk.metrics.internal.view.Base2ExponentialHistogramAggregation;
import io.opentelemetry.sdk.metrics.internal.view.ExplicitBucketHistogramAggregation;
import io.opentelemetry.sdk.metrics.internal.view.ExponentialHistogramAggregation;
import io.opentelemetry.sdk.metrics.internal.view.RegisteredView;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
Expand Down Expand Up @@ -230,7 +230,7 @@ void toAggregation_GoodConfig() {
assertThat(
ViewConfig.toAggregation(
"base2_exponential_bucket_histogram", ImmutableMap.of("max_buckets", 20)))
.isInstanceOf(ExponentialHistogramAggregation.class)
.isInstanceOf(Base2ExponentialHistogramAggregation.class)
.extracting("maxBuckets", as(InstanceOfAssertFactories.INTEGER))
.isEqualTo(20);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public static class ThreadState {

private double[] values;
private final AtomicLong valueIndex = new AtomicLong();
private ExponentialHistogramIndexer indexer;
private Base2ExponentialHistogramIndexer indexer;

@Setup(Level.Trial)
public final void setup() {
Expand All @@ -46,7 +46,7 @@ public final void setup() {
for (int i = 0; i < numValues; i++) {
values[i] = random.nextDouble() * 1000;
}
indexer = ExponentialHistogramIndexer.get(scale);
indexer = Base2ExponentialHistogramIndexer.get(scale);
}

public void compute() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ public enum HistogramAggregationParam {
ExplicitBucketHistogramUtils.createBoundaryArray(Collections.emptyList()),
ExemplarReservoir::doubleNoSamples)),
EXPONENTIAL_SMALL_CIRCULAR_BUFFER(
new DoubleExponentialHistogramAggregator(ExemplarReservoir::doubleNoSamples, 20, 0)),
new DoubleBase2ExponentialHistogramAggregator(ExemplarReservoir::doubleNoSamples, 20, 0)),
EXPONENTIAL_CIRCULAR_BUFFER(
new DoubleExponentialHistogramAggregator(ExemplarReservoir::doubleNoSamples, 160, 0));
new DoubleBase2ExponentialHistogramAggregator(ExemplarReservoir::doubleNoSamples, 160, 0));

private final Aggregator<?, ?> aggregator;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
package io.opentelemetry.sdk.metrics;

import io.opentelemetry.sdk.metrics.data.MetricDataType;
import io.opentelemetry.sdk.metrics.internal.view.Base2ExponentialHistogramAggregation;
import io.opentelemetry.sdk.metrics.internal.view.DefaultAggregation;
import io.opentelemetry.sdk.metrics.internal.view.DropAggregation;
import io.opentelemetry.sdk.metrics.internal.view.ExplicitBucketHistogramAggregation;
import io.opentelemetry.sdk.metrics.internal.view.ExponentialHistogramAggregation;
import io.opentelemetry.sdk.metrics.internal.view.LastValueAggregation;
import io.opentelemetry.sdk.metrics.internal.view.SumAggregation;
import java.util.List;
Expand Down Expand Up @@ -74,7 +74,7 @@ static Aggregation explicitBucketHistogram(List<Double> bucketBoundaries) {
* default {@code maxBuckets} and {@code maxScale}.
*/
static Aggregation base2ExponentialBucketHistogram() {
return ExponentialHistogramAggregation.getDefault();
return Base2ExponentialHistogramAggregation.getDefault();
}

/**
Expand All @@ -88,6 +88,6 @@ static Aggregation base2ExponentialBucketHistogram() {
* performance of computing bucket index is improved when scale is {@code <= 0}.
*/
static Aggregation base2ExponentialBucketHistogram(int maxBuckets, int maxScale) {
return ExponentialHistogramAggregation.create(maxBuckets, maxScale);
return Base2ExponentialHistogramAggregation.create(maxBuckets, maxScale);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

final class ExponentialHistogramIndexer {
final class Base2ExponentialHistogramIndexer {

private static final Map<Integer, ExponentialHistogramIndexer> cache = new ConcurrentHashMap<>();
private static final Map<Integer, Base2ExponentialHistogramIndexer> cache =
new ConcurrentHashMap<>();

/** Bit mask used to isolate exponent of IEEE 754 double precision number. */
private static final long EXPONENT_BIT_MASK = 0x7FF0000000000000L;
Expand All @@ -35,14 +36,14 @@ final class ExponentialHistogramIndexer {
private final int scale;
private final double scaleFactor;

private ExponentialHistogramIndexer(int scale) {
private Base2ExponentialHistogramIndexer(int scale) {
this.scale = scale;
this.scaleFactor = computeScaleFactor(scale);
}

/** Get an indexer for the given scale. Indexers are cached and reused for performance. */
static ExponentialHistogramIndexer get(int scale) {
return cache.computeIfAbsent(scale, unused -> new ExponentialHistogramIndexer(scale));
static Base2ExponentialHistogramIndexer get(int scale) {
return cache.computeIfAbsent(scale, unused -> new Base2ExponentialHistogramIndexer(scale));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
import javax.annotation.Nullable;

/**
* Aggregator that generates exponential histograms.
* Aggregator that generates base2 exponential histograms.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class DoubleExponentialHistogramAggregator
public final class DoubleBase2ExponentialHistogramAggregator
implements Aggregator<ExponentialHistogramPointData, DoubleExemplarData> {

private final Supplier<ExemplarReservoir<DoubleExemplarData>> reservoirSupplier;
Expand All @@ -45,7 +45,7 @@ public final class DoubleExponentialHistogramAggregator
*
* @param reservoirSupplier Supplier of exemplar reservoirs per-stream.
*/
public DoubleExponentialHistogramAggregator(
public DoubleBase2ExponentialHistogramAggregator(
Supplier<ExemplarReservoir<DoubleExemplarData>> reservoirSupplier,
int maxBuckets,
int maxScale) {
Expand Down Expand Up @@ -78,8 +78,8 @@ public MetricData toMetricData(
static final class Handle
extends AggregatorHandle<ExponentialHistogramPointData, DoubleExemplarData> {
private final int maxBuckets;
@Nullable private DoubleExponentialHistogramBuckets positiveBuckets;
@Nullable private DoubleExponentialHistogramBuckets negativeBuckets;
@Nullable private DoubleBase2ExponentialHistogramBuckets positiveBuckets;
@Nullable private DoubleBase2ExponentialHistogramBuckets negativeBuckets;
private long zeroCount;
private double sum;
private double min;
Expand Down Expand Up @@ -129,7 +129,7 @@ protected synchronized ExponentialHistogramPointData doAggregateThenMaybeReset(
}

private static ExponentialHistogramBuckets resolveBuckets(
@Nullable DoubleExponentialHistogramBuckets buckets, int scale, boolean reset) {
@Nullable DoubleBase2ExponentialHistogramBuckets buckets, int scale, boolean reset) {
if (buckets == null) {
return EmptyExponentialHistogramBuckets.get(scale);
}
Expand All @@ -154,20 +154,20 @@ protected synchronized void doRecordDouble(double value) {
count++;

int c = Double.compare(value, 0);
DoubleExponentialHistogramBuckets buckets;
DoubleBase2ExponentialHistogramBuckets buckets;
if (c == 0) {
zeroCount++;
return;
} else if (c > 0) {
// Initialize positive buckets at current scale, if needed
if (positiveBuckets == null) {
positiveBuckets = new DoubleExponentialHistogramBuckets(scale, maxBuckets);
positiveBuckets = new DoubleBase2ExponentialHistogramBuckets(scale, maxBuckets);
}
buckets = positiveBuckets;
} else {
// Initialize negative buckets at current scale, if needed
if (negativeBuckets == null) {
negativeBuckets = new DoubleExponentialHistogramBuckets(scale, maxBuckets);
negativeBuckets = new DoubleBase2ExponentialHistogramBuckets(scale, maxBuckets);
}
buckets = negativeBuckets;
}
Expand Down Expand Up @@ -214,7 +214,7 @@ static ExponentialHistogramBuckets get(int scale) {
return ZERO_BUCKETS.computeIfAbsent(
scale,
scale1 ->
new AutoValue_DoubleExponentialHistogramAggregator_EmptyExponentialHistogramBuckets(
new AutoValue_DoubleBase2ExponentialHistogramAggregator_EmptyExponentialHistogramBuckets(
scale1, 0, Collections.emptyList(), 0));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,37 @@
import javax.annotation.Nullable;

/**
* This class handles the operations for recording, scaling, and exposing data related to the
* This class handles the operations for recording, scaling, and exposing data related to the base2
* exponential histogram.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
final class DoubleExponentialHistogramBuckets implements ExponentialHistogramBuckets {
final class DoubleBase2ExponentialHistogramBuckets implements ExponentialHistogramBuckets {

private AdaptingCircularBufferCounter counts;
private int scale;
private ExponentialHistogramIndexer exponentialHistogramIndexer;
private Base2ExponentialHistogramIndexer base2ExponentialHistogramIndexer;
private long totalCount;

DoubleExponentialHistogramBuckets(int scale, int maxBuckets) {
DoubleBase2ExponentialHistogramBuckets(int scale, int maxBuckets) {
this.counts = new AdaptingCircularBufferCounter(maxBuckets);
this.scale = scale;
this.exponentialHistogramIndexer = ExponentialHistogramIndexer.get(this.scale);
this.base2ExponentialHistogramIndexer = Base2ExponentialHistogramIndexer.get(this.scale);
this.totalCount = 0;
}

// For copying
DoubleExponentialHistogramBuckets(DoubleExponentialHistogramBuckets buckets) {
DoubleBase2ExponentialHistogramBuckets(DoubleBase2ExponentialHistogramBuckets buckets) {
this.counts = new AdaptingCircularBufferCounter(buckets.counts);
this.scale = buckets.scale;
this.exponentialHistogramIndexer = buckets.exponentialHistogramIndexer;
this.base2ExponentialHistogramIndexer = buckets.base2ExponentialHistogramIndexer;
this.totalCount = buckets.totalCount;
}

/** Returns a copy of this bucket. */
DoubleExponentialHistogramBuckets copy() {
return new DoubleExponentialHistogramBuckets(this);
DoubleBase2ExponentialHistogramBuckets copy() {
return new DoubleBase2ExponentialHistogramBuckets(this);
}

/** Resets all counters in this bucket set to zero, but preserves scale. */
Expand All @@ -56,7 +56,7 @@ boolean record(double value) {
// Guarded by caller. If passed 0 it would be a bug in the SDK.
throw new IllegalStateException("Illegal attempted recording of zero at bucket level.");
}
int index = exponentialHistogramIndexer.computeIndex(value);
int index = base2ExponentialHistogramIndexer.computeIndex(value);
boolean recordingSuccessful = this.counts.increment(index, 1);
if (recordingSuccessful) {
totalCount++;
Expand Down Expand Up @@ -121,7 +121,7 @@ void downscale(int by) {
}

this.scale = this.scale - by;
this.exponentialHistogramIndexer = ExponentialHistogramIndexer.get(this.scale);
this.base2ExponentialHistogramIndexer = Base2ExponentialHistogramIndexer.get(this.scale);
}

/**
Expand All @@ -135,7 +135,7 @@ void downscale(int by) {
*
* @param other the histogram that will be merged into this one
*/
void mergeInto(DoubleExponentialHistogramBuckets other) {
void mergeInto(DoubleBase2ExponentialHistogramBuckets other) {
if (other.counts.isEmpty()) {
return;
}
Expand Down Expand Up @@ -190,7 +190,7 @@ public int getScale() {
* @return The required scale reduction in order to fit the value in these buckets.
*/
int getScaleReduction(double value) {
long index = exponentialHistogramIndexer.computeIndex(value);
long index = base2ExponentialHistogramIndexer.computeIndex(value);
long newStart = Math.min(index, counts.getIndexStart());
long newEnd = Math.max(index, counts.getIndexEnd());
return getScaleReduction(newStart, newEnd);
Expand All @@ -209,10 +209,10 @@ int getScaleReduction(long newStart, long newEnd) {

@Override
public boolean equals(@Nullable Object obj) {
if (!(obj instanceof DoubleExponentialHistogramBuckets)) {
if (!(obj instanceof DoubleBase2ExponentialHistogramBuckets)) {
return false;
}
DoubleExponentialHistogramBuckets other = (DoubleExponentialHistogramBuckets) obj;
DoubleBase2ExponentialHistogramBuckets other = (DoubleBase2ExponentialHistogramBuckets) obj;
// Don't need to compare getTotalCount() because equivalent bucket counts
// imply equivalent overall count.
// Additionally, we compare the "semantics" of bucket counts, that is
Expand All @@ -232,7 +232,7 @@ public boolean equals(@Nullable Object obj) {
* <li>Offset does NOT need to be the same
* </ul>
*/
private boolean sameBucketCounts(DoubleExponentialHistogramBuckets other) {
private boolean sameBucketCounts(DoubleBase2ExponentialHistogramBuckets other) {
if (this.totalCount != other.totalCount) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import io.opentelemetry.sdk.metrics.data.PointData;
import io.opentelemetry.sdk.metrics.internal.aggregator.Aggregator;
import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorFactory;
import io.opentelemetry.sdk.metrics.internal.aggregator.DoubleExponentialHistogramAggregator;
import io.opentelemetry.sdk.metrics.internal.aggregator.DoubleBase2ExponentialHistogramAggregator;
import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter;
import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarReservoir;
Expand All @@ -26,18 +26,18 @@
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class ExponentialHistogramAggregation implements Aggregation, AggregatorFactory {
public final class Base2ExponentialHistogramAggregation implements Aggregation, AggregatorFactory {

private static final int DEFAULT_MAX_BUCKETS = 160;
private static final int DEFAULT_MAX_SCALE = 20;

private static final Aggregation DEFAULT =
new ExponentialHistogramAggregation(DEFAULT_MAX_BUCKETS, DEFAULT_MAX_SCALE);
new Base2ExponentialHistogramAggregation(DEFAULT_MAX_BUCKETS, DEFAULT_MAX_SCALE);

private final int maxBuckets;
private final int maxScale;

private ExponentialHistogramAggregation(int maxBuckets, int maxScale) {
private Base2ExponentialHistogramAggregation(int maxBuckets, int maxScale) {
this.maxBuckets = maxBuckets;
this.maxScale = maxScale;
}
Expand All @@ -60,15 +60,15 @@ public static Aggregation getDefault() {
public static Aggregation create(int maxBuckets, int maxScale) {
checkArgument(maxBuckets >= 1, "maxBuckets must be > 0");
checkArgument(maxScale <= 20 && maxScale >= -10, "maxScale must be -10 <= x <= 20");
return new ExponentialHistogramAggregation(maxBuckets, maxScale);
return new Base2ExponentialHistogramAggregation(maxBuckets, maxScale);
}

@Override
@SuppressWarnings("unchecked")
public <T extends PointData, U extends ExemplarData> Aggregator<T, U> createAggregator(
InstrumentDescriptor instrumentDescriptor, ExemplarFilter exemplarFilter) {
return (Aggregator<T, U>)
new DoubleExponentialHistogramAggregator(
new DoubleBase2ExponentialHistogramAggregator(
() ->
ExemplarReservoir.filtered(
exemplarFilter,
Expand All @@ -93,7 +93,7 @@ public boolean isCompatibleWithInstrument(InstrumentDescriptor instrumentDescrip

@Override
public String toString() {
return "ExponentialHistogramAggregation{maxBuckets="
return "Base2ExponentialHistogramAggregation{maxBuckets="
+ maxBuckets
+ ",maxScale="
+ maxScale
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ void haveToString() {
.contains("ExplicitBucketHistogramAggregation");
assertThat(Aggregation.base2ExponentialBucketHistogram())
.asString()
.isEqualTo("ExponentialHistogramAggregation{maxBuckets=160,maxScale=20}");
.isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=160,maxScale=20}");
assertThat(Aggregation.base2ExponentialBucketHistogram(1, 0))
.asString()
.isEqualTo("ExponentialHistogramAggregation{maxBuckets=1,maxScale=0}");
.isEqualTo("Base2ExponentialHistogramAggregation{maxBuckets=1,maxScale=0}");
}

@Test
void histogramUsesExplicitBucket() {
// Note: This will change when exponential histograms are launched.
assertThat(Aggregation.explicitBucketHistogram())
.asString()
.contains("ExplicitBucketHistogram");
Expand Down
Loading

0 comments on commit feb0297

Please sign in to comment.