Skip to content

Commit

Permalink
Implement metric identity specification (#4222)
Browse files Browse the repository at this point in the history
* Implement metric identity specification

* PR feedback

* Fix build
  • Loading branch information
jack-berg authored Mar 15, 2022
1 parent ef99593 commit ccfa2dc
Show file tree
Hide file tree
Showing 22 changed files with 1,375 additions and 290 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
import io.opentelemetry.sdk.metrics.common.InstrumentType;
import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil;
import io.opentelemetry.sdk.metrics.view.Aggregation;
import io.opentelemetry.sdk.metrics.view.InstrumentSelector;
import io.opentelemetry.sdk.metrics.view.InstrumentSelectorBuilder;
Expand Down Expand Up @@ -182,17 +183,10 @@ static View toView(ViewSpecification viewSpec) {

// Visible for testing
static Aggregation toAggregation(String aggregation) {
switch (aggregation) {
case "sum":
return Aggregation.sum();
case "last_value":
return Aggregation.lastValue();
case "drop":
return Aggregation.drop();
case "histogram":
return Aggregation.explicitBucketHistogram();
default:
throw new ConfigurationException("Unrecognized aggregation " + aggregation);
try {
return AggregationUtil.forName(aggregation);
} catch (IllegalArgumentException e) {
throw new ConfigurationException("Error creating aggregation", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,9 @@ void toView() {

@Test
void toAggregation() {
assertThat(ViewConfig.toAggregation("sum")).isEqualTo(Aggregation.sum());
assertThat(ViewConfig.toAggregation("last_value")).isEqualTo(Aggregation.lastValue());
assertThat(ViewConfig.toAggregation("histogram"))
.isEqualTo(Aggregation.explicitBucketHistogram());
assertThat(ViewConfig.toAggregation("drop")).isEqualTo(Aggregation.drop());
assertThatThrownBy(() -> ViewConfig.toAggregation("foo"))
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("Unrecognized aggregation foo");
.hasMessageContaining("Error creating aggregation");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.metrics.internal.descriptor;

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.sdk.metrics.common.InstrumentType;
import io.opentelemetry.sdk.metrics.common.InstrumentValueType;
import org.junit.jupiter.api.Test;

/**
* {@link InstrumentDescriptor#equals(Object)} must ignore {@link
* InstrumentDescriptor#getSourceInfo()}, which only returns a meaningful value when {@code
* otel.experimental.sdk.metrics.debug=true}.
*/
class InstrumentDescriptorTest {

@Test
void equals() {
InstrumentDescriptor descriptor =
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG);

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

// Validate getSourceInfo() is not equal for otherwise equal descriptors
assertThat(descriptor.getSourceInfo())
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG)
.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));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "foo", "unit", InstrumentType.COUNTER, InstrumentValueType.LONG));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "description", "foo", InstrumentType.COUNTER, InstrumentValueType.LONG));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name",
"description",
"unit",
InstrumentType.UP_DOWN_COUNTER,
InstrumentValueType.LONG));
assertThat(descriptor)
.isNotEqualTo(
InstrumentDescriptor.create(
"name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.DOUBLE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,16 @@ void testDuplicateExceptionMessage_pureInstruments() {
MetricDescriptor.create(
View.builder().build(),
InstrumentDescriptor.create(
"name", "description2", "unit2", InstrumentType.COUNTER, InstrumentValueType.LONG));
"name2",
"description2",
"unit2",
InstrumentType.COUNTER,
InstrumentValueType.LONG));
assertThat(DebugUtils.duplicateMetricErrorMessage(simple, simpleWithNewDescription))
.contains("Found duplicate metric definition: name")
.contains("- Unit [unit2] does not match [unit]")
.contains("- Description [description2] does not match [description]")
.contains("- InstrumentDescription [description2] does not match [description]")
.contains("- InstrumentName [name2] does not match [name]")
.contains("- InstrumentUnit [unit2] does not match [unit]")
.contains("- InstrumentType [COUNTER] does not match [OBSERVABLE_COUNTER]")
.contains("- InstrumentValueType [LONG] does not match [DOUBLE]")
.contains(simple.getSourceInstrument().getSourceInfo().multiLineDebugString())
Expand All @@ -60,10 +65,9 @@ void testDuplicateExceptionMessage_pureInstruments() {

@Test
void testDuplicateExceptionMessage_viewBasedConflict() {
View problemView = View.builder().setName("name2").build();
MetricDescriptor simple =
MetricDescriptor.create(
problemView,
View.builder().setName("name2").build(),
InstrumentDescriptor.create(
"name",
"description",
Expand All @@ -82,9 +86,9 @@ void testDuplicateExceptionMessage_viewBasedConflict() {
assertThat(DebugUtils.duplicateMetricErrorMessage(simple, simpleWithNewDescription))
.contains("Found duplicate metric definition: name2")
.contains(simple.getSourceInstrument().getSourceInfo().multiLineDebugString())
.contains("- Description [description2] does not match [description]")
.contains("- InstrumentDescription [description2] does not match [description]")
.contains("Conflicting view registered")
.contains(ImmutableView.getSourceInfo(problemView).multiLineDebugString())
.contains(ImmutableView.getSourceInfo(simple.getSourceView()).multiLineDebugString())
.contains("FROM instrument name")
.contains(
simpleWithNewDescription.getSourceInstrument().getSourceInfo().multiLineDebugString());
Expand Down Expand Up @@ -117,7 +121,7 @@ void testDuplicateExceptionMessage_viewBasedConflict2() {
.contains(ImmutableView.getSourceInfo(problemView).multiLineDebugString())
.contains("FROM instrument name2")
.contains(simple.getSourceInstrument().getSourceInfo().multiLineDebugString())
.contains("- Unit [unit] does not match [unit2]")
.contains("- InstrumentUnit [unit] does not match [unit2]")
.contains("Original instrument registered with same name but is incompatible.")
.contains(
simpleWithNewDescription.getSourceInstrument().getSourceInfo().multiLineDebugString());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.metrics.internal.aggregator;

import io.opentelemetry.sdk.metrics.view.Aggregation;
import java.util.HashMap;
import java.util.Map;

/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public class AggregationUtil {

private static final Map<String, Aggregation> aggregationByName;
private static final Map<Class<? extends Aggregation>, String> nameByAggregation;

private static final String AGGREGATION_DEFAULT = "default";
private static final String AGGREGATION_SUM = "sum";
private static final String AGGREGATION_LAST_VALUE = "last_value";
private static final String AGGREGATION_DROP = "drop";
private static final String AGGREGATION_EXPLICIT_BUCKET_HISTOGRAM = "explicit_bucket_histogram";

static {
aggregationByName = new HashMap<>();
aggregationByName.put(AGGREGATION_DEFAULT, Aggregation.defaultAggregation());
aggregationByName.put(AGGREGATION_SUM, Aggregation.sum());
aggregationByName.put(AGGREGATION_LAST_VALUE, Aggregation.lastValue());
aggregationByName.put(AGGREGATION_DROP, Aggregation.drop());
aggregationByName.put(
AGGREGATION_EXPLICIT_BUCKET_HISTOGRAM, Aggregation.explicitBucketHistogram());

nameByAggregation = new HashMap<>();
nameByAggregation.put(Aggregation.defaultAggregation().getClass(), AGGREGATION_DEFAULT);
nameByAggregation.put(Aggregation.sum().getClass(), AGGREGATION_SUM);
nameByAggregation.put(Aggregation.lastValue().getClass(), AGGREGATION_LAST_VALUE);
nameByAggregation.put(Aggregation.drop().getClass(), AGGREGATION_DROP);
nameByAggregation.put(
Aggregation.explicitBucketHistogram().getClass(), AGGREGATION_EXPLICIT_BUCKET_HISTOGRAM);
}

private AggregationUtil() {}

/**
* Return the aggregation for the human-readable {@code name}.
*
* <p>The inverse of {@link #aggregationName(Aggregation)}.
*
* @throws IllegalArgumentException if the name is not recognized
*/
public static Aggregation forName(String name) {
Aggregation aggregation = aggregationByName.get(name);
if (aggregation == null) {
throw new IllegalArgumentException("Unrecognized aggregation name " + name);
}
return aggregation;
}

/**
* Return the human-readable name of the {@code aggregation}.
*
* <p>The inverse of {@link #forName(String)}.
*
* @throws IllegalArgumentException if the aggregation is not recognized
*/
public static String aggregationName(Aggregation aggregation) {
String name = nameByAggregation.get(aggregation.getClass());
if (name == null) {
throw new IllegalStateException(
"Unrecognized aggregation " + aggregation.getClass().getName());
}
return name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public MetricData toMetricData(
instrumentationScopeInfo,
metricDescriptor.getName(),
metricDescriptor.getDescription(),
metricDescriptor.getUnit(),
metricDescriptor.getSourceInstrument().getUnit(),
ExponentialHistogramData.create(
temporality,
MetricDataUtils.toExponentialHistogramPointList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public MetricData toMetricData(
instrumentationScopeInfo,
metricDescriptor.getName(),
metricDescriptor.getDescription(),
metricDescriptor.getUnit(),
metricDescriptor.getSourceInstrument().getUnit(),
ImmutableHistogramData.create(
temporality,
MetricDataUtils.toDoubleHistogramPointList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public MetricData toMetricData(
instrumentationScopeInfo,
descriptor.getName(),
descriptor.getDescription(),
descriptor.getUnit(),
descriptor.getSourceInstrument().getUnit(),
ImmutableGaugeData.create(
MetricDataUtils.toDoublePointList(
accumulationByLabels,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public MetricData toMetricData(
instrumentationScopeInfo,
descriptor.getName(),
descriptor.getDescription(),
descriptor.getUnit(),
descriptor.getSourceInstrument().getUnit(),
ImmutableSumData.create(
isMonotonic(),
temporality,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public MetricData toMetricData(
instrumentationScopeInfo,
descriptor.getName(),
descriptor.getDescription(),
descriptor.getUnit(),
descriptor.getSourceInstrument().getUnit(),
ImmutableGaugeData.create(
MetricDataUtils.toLongPointList(
accumulationByLabels,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public MetricData toMetricData(
instrumentationScopeInfo,
descriptor.getName(),
descriptor.getDescription(),
descriptor.getUnit(),
descriptor.getSourceInstrument().getUnit(),
ImmutableSumData.create(
isMonotonic(),
temporality,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@
@AutoValue
@Immutable
public abstract class InstrumentDescriptor {

private final SourceInfo sourceInfo = SourceInfo.fromCurrentStack();

public static InstrumentDescriptor create(
String name,
String description,
String unit,
InstrumentType type,
InstrumentValueType valueType) {
return new AutoValue_InstrumentDescriptor(
name, description, unit, type, valueType, SourceInfo.fromCurrentStack());
return new AutoValue_InstrumentDescriptor(name, description, unit, type, valueType);
}

public abstract String getName();
Expand All @@ -41,8 +43,13 @@ public static InstrumentDescriptor create(

public abstract InstrumentValueType getValueType();

/** Debugging information for this instrument. */
public abstract SourceInfo getSourceInfo();
/**
* Debugging information for this instrument. Ignored from {@link #equals(Object)} and {@link
* #toString()}
*/
public final SourceInfo getSourceInfo() {
return sourceInfo;
}

@Memoized
@Override
Expand Down
Loading

0 comments on commit ccfa2dc

Please sign in to comment.