Skip to content

Commit

Permalink
Prototype histogram advice API (i.e. Hints) (#5217)
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-berg authored Apr 11, 2023
1 parent 0a34867 commit 2870209
Show file tree
Hide file tree
Showing 34 changed files with 703 additions and 102 deletions.
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
Comparing source compatibility of against
No changes.
*** MODIFIED CLASS: PUBLIC io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader create(io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector, io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector)
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.metrics.Aggregation getDefaultAggregation(io.opentelemetry.sdk.metrics.InstrumentType)
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.extension.incubator.metrics;

import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
import java.util.function.Consumer;

/** Extended {@link DoubleHistogramBuilder} with experimental APIs. */
public interface ExtendedDoubleHistogramBuilder extends DoubleHistogramBuilder {

/** Specify advice for histogram implementations. */
default DoubleHistogramBuilder setAdvice(Consumer<HistogramAdviceConfigurer> adviceConsumer) {
return this;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.extension.incubator.metrics;

import io.opentelemetry.api.metrics.LongHistogramBuilder;
import java.util.function.Consumer;

/** Extended {@link LongHistogramBuilder} with experimental APIs. */
public interface ExtendedLongHistogramBuilder extends LongHistogramBuilder {

/** Specify advice for histogram implementations. */
default LongHistogramBuilder setAdvice(Consumer<HistogramAdviceConfigurer> adviceConsumer) {
return this;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.extension.incubator.metrics;

import io.opentelemetry.api.metrics.DoubleHistogram;
import io.opentelemetry.api.metrics.LongHistogram;
import java.util.List;

/** Configure advice for implementations of {@link LongHistogram} and {@link DoubleHistogram}. */
public interface HistogramAdviceConfigurer {

/** Specify recommended set of explicit bucket boundaries for this histogram. */
HistogramAdviceConfigurer setExplicitBucketBoundaries(List<Double> bucketBoundaries);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;

@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
class OpenTelemetrySdkTest {

@RegisterExtension
Expand Down
1 change: 1 addition & 0 deletions sdk/metrics/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ otelJava.moduleName.set("io.opentelemetry.sdk.metrics")
dependencies {
api(project(":api:all"))
api(project(":sdk:common"))
implementation(project(":extensions:incubator"))

compileOnly("org.codehaus.mojo:animal-sniffer-annotations")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,44 +22,80 @@ class InstrumentDescriptorTest {
void equals() {
InstrumentDescriptor descriptor =
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG);
"name",
"description",
"unit",
InstrumentType.COUNTER,
InstrumentValueType.LONG,
Advice.empty());

assertThat(descriptor)
.isEqualTo(
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG));
"name",
"description",
"unit",
InstrumentType.COUNTER,
InstrumentValueType.LONG,
Advice.empty()));

// Validate getSourceInfo() is not equal for otherwise equal descriptors
assertThat(descriptor.getSourceInfo())
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG)
"name",
"description",
"unit",
InstrumentType.COUNTER,
InstrumentValueType.LONG,
Advice.empty())
.getSourceInfo());

// Validate that name, description, unit, type, and value type are considered in equals
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"foo", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG));
"foo",
"description",
"unit",
InstrumentType.COUNTER,
InstrumentValueType.LONG,
Advice.empty()));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "foo", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG));
"name",
"foo",
"unit",
InstrumentType.COUNTER,
InstrumentValueType.LONG,
Advice.empty()));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "description", "foo", InstrumentType.COUNTER, InstrumentValueType.LONG));
"name",
"description",
"foo",
InstrumentType.COUNTER,
InstrumentValueType.LONG,
Advice.empty()));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name",
"description",
"unit",
InstrumentType.UP_DOWN_COUNTER,
InstrumentValueType.LONG));
InstrumentValueType.LONG,
Advice.empty()));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.DOUBLE));
"name",
"description",
"unit",
InstrumentType.COUNTER,
InstrumentValueType.DOUBLE,
Advice.empty()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.opentelemetry.sdk.metrics.InstrumentValueType;
import io.opentelemetry.sdk.metrics.View;
import io.opentelemetry.sdk.metrics.internal.debug.SourceInfo;
import io.opentelemetry.sdk.metrics.internal.descriptor.Advice;
import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
import io.opentelemetry.sdk.metrics.internal.descriptor.MetricDescriptor;
import io.opentelemetry.sdk.metrics.internal.state.DebugUtils;
Expand All @@ -23,10 +24,10 @@ class SourceInfoTest {

@Test
void sourceInfoFindsStackTrace() {
assertThat(info.shortDebugString()).isEqualTo("SourceInfoTest.java:22");
assertThat(info.shortDebugString()).isEqualTo("SourceInfoTest.java:23");
assertThat(info.multiLineDebugString())
.startsWith(
"\tat io.opentelemetry.testing.SourceInfoTest.<clinit>(SourceInfoTest.java:22)\n");
"\tat io.opentelemetry.testing.SourceInfoTest.<clinit>(SourceInfoTest.java:23)\n");
}

@Test
Expand All @@ -40,7 +41,8 @@ void testDuplicateExceptionMessage_pureInstruments() {
"description",
"unit",
InstrumentType.OBSERVABLE_COUNTER,
InstrumentValueType.DOUBLE));
InstrumentValueType.DOUBLE,
Advice.empty()));
MetricDescriptor simpleWithNewDescription =
MetricDescriptor.create(
View.builder().build(),
Expand All @@ -50,7 +52,8 @@ void testDuplicateExceptionMessage_pureInstruments() {
"description2",
"unit2",
InstrumentType.COUNTER,
InstrumentValueType.LONG));
InstrumentValueType.LONG,
Advice.empty()));
assertThat(DebugUtils.duplicateMetricErrorMessage(simple, simpleWithNewDescription))
.contains("Found duplicate metric definition: name")
.contains("- InstrumentDescription [description2] does not match [description]")
Expand All @@ -76,7 +79,8 @@ void testDuplicateExceptionMessage_viewBasedConflict() {
"description",
"unit",
InstrumentType.HISTOGRAM,
InstrumentValueType.DOUBLE));
InstrumentValueType.DOUBLE,
Advice.empty()));
MetricDescriptor simpleWithNewDescription =
MetricDescriptor.create(
View.builder().build(),
Expand All @@ -86,7 +90,8 @@ void testDuplicateExceptionMessage_viewBasedConflict() {
"description2",
"unit",
InstrumentType.HISTOGRAM,
InstrumentValueType.DOUBLE));
InstrumentValueType.DOUBLE,
Advice.empty()));
assertThat(DebugUtils.duplicateMetricErrorMessage(simple, simpleWithNewDescription))
.contains("Found duplicate metric definition: name2")
.contains(simple.getSourceInstrument().getSourceInfo().multiLineDebugString())
Expand All @@ -111,7 +116,8 @@ void testDuplicateExceptionMessage_viewBasedConflict2() {
"description",
"unit2",
InstrumentType.HISTOGRAM,
InstrumentValueType.DOUBLE));
InstrumentValueType.DOUBLE,
Advice.empty()));
MetricDescriptor simpleWithNewDescription =
MetricDescriptor.create(
problemView,
Expand All @@ -121,7 +127,8 @@ void testDuplicateExceptionMessage_viewBasedConflict2() {
"description",
"unit",
InstrumentType.HISTOGRAM,
InstrumentValueType.DOUBLE));
InstrumentValueType.DOUBLE,
Advice.empty()));
assertThat(DebugUtils.duplicateMetricErrorMessage(simple, simpleWithNewDescription))
.contains("Found duplicate metric definition: name")
.contains("VIEW defined")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
import io.opentelemetry.sdk.metrics.internal.descriptor.Advice;
import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
import io.opentelemetry.sdk.metrics.internal.state.CallbackRegistration;
import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
Expand All @@ -27,6 +28,7 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
private final InstrumentValueType valueType;
private String description;
private String unit;
private Advice advice;

protected final MeterSharedState meterSharedState;
protected final String instrumentName;
Expand All @@ -39,13 +41,34 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
String name,
String description,
String unit) {
this(
meterProviderSharedState,
meterSharedState,
type,
valueType,
name,
description,
unit,
Advice.empty());
}

AbstractInstrumentBuilder(
MeterProviderSharedState meterProviderSharedState,
MeterSharedState meterSharedState,
InstrumentType type,
InstrumentValueType valueType,
String name,
String description,
String unit,
Advice advice) {
this.type = type;
this.valueType = valueType;
this.instrumentName = name;
this.description = description;
this.unit = unit;
this.meterProviderSharedState = meterProviderSharedState;
this.meterSharedState = meterSharedState;
this.advice = advice;
}

protected abstract BuilderT getThis();
Expand All @@ -62,13 +85,17 @@ public BuilderT setDescription(String description) {

protected <T> T swapBuilder(SwapBuilder<T> swapper) {
return swapper.newBuilder(
meterProviderSharedState, meterSharedState, instrumentName, description, unit);
meterProviderSharedState, meterSharedState, instrumentName, description, unit, advice);
}

protected void setAdvice(Advice advice) {
this.advice = advice;
}

final <I extends AbstractInstrument> I buildSynchronousInstrument(
BiFunction<InstrumentDescriptor, WriteableMetricStorage, I> instrumentFactory) {
InstrumentDescriptor descriptor =
InstrumentDescriptor.create(instrumentName, description, unit, type, valueType);
InstrumentDescriptor.create(instrumentName, description, unit, type, valueType, advice);
WriteableMetricStorage storage =
meterSharedState.registerSynchronousMetricStorage(descriptor, meterProviderSharedState);
return instrumentFactory.apply(descriptor, storage);
Expand Down Expand Up @@ -96,15 +123,15 @@ final SdkObservableInstrument registerLongAsynchronousInstrument(

final SdkObservableMeasurement buildObservableMeasurement(InstrumentType type) {
InstrumentDescriptor descriptor =
InstrumentDescriptor.create(instrumentName, description, unit, type, valueType);
InstrumentDescriptor.create(instrumentName, description, unit, type, valueType, advice);
return meterSharedState.registerObservableMeasurement(descriptor);
}

@Override
public String toString() {
return this.getClass().getSimpleName()
+ "{descriptor="
+ InstrumentDescriptor.create(instrumentName, description, unit, type, valueType)
+ InstrumentDescriptor.create(instrumentName, description, unit, type, valueType, advice)
+ "}";
}

Expand All @@ -115,6 +142,7 @@ T newBuilder(
MeterSharedState meterSharedState,
String name,
String description,
String unit);
String unit,
Advice advice);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
import io.opentelemetry.context.Context;
import io.opentelemetry.sdk.internal.ThrottlingLogger;
import io.opentelemetry.sdk.metrics.internal.descriptor.Advice;
import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
import io.opentelemetry.sdk.metrics.internal.state.MeterSharedState;
Expand Down Expand Up @@ -62,15 +63,17 @@ static final class SdkDoubleCounterBuilder
MeterSharedState sharedState,
String name,
String description,
String unit) {
String unit,
Advice advice) {
super(
meterProviderSharedState,
sharedState,
InstrumentType.COUNTER,
InstrumentValueType.DOUBLE,
name,
description,
unit);
unit,
advice);
}

@Override
Expand Down
Loading

0 comments on commit 2870209

Please sign in to comment.