Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototype histogram advice API (i.e. Hints) #5217

Merged
merged 8 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -9,6 +9,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 @@ -33,6 +34,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 @@ -45,13 +47,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 @@ -74,13 +97,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 @@ -108,15 +135,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 Down Expand Up @@ -155,6 +182,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