From c7861e8b29230062cb3ab8ba755eacb81455272f Mon Sep 17 00:00:00 2001 From: Anton Polyakov Date: Mon, 1 Mar 2021 18:43:31 -0800 Subject: [PATCH] introducing view (#2956) * introducing view * code review --- .../sdk/metrics/AbstractAccumulator.java | 1 + .../sdk/metrics/SdkMeterProvider.java | 11 ++--- .../sdk/metrics/ViewRegistry.java | 40 +++++++++---------- .../opentelemetry/sdk/metrics/view/View.java | 25 ++++++++++++ .../sdk/metrics/view/ViewBuilder.java | 24 +++++++++++ .../sdk/metrics/SdkMeterProviderTest.java | 6 ++- .../sdk/metrics/ViewRegistryTest.java | 28 ++++++++----- 7 files changed, 97 insertions(+), 38 deletions(-) create mode 100644 sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/View.java create mode 100644 sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/ViewBuilder.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java index 0d8e71535c3..22b2f05ecc9 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java @@ -25,6 +25,7 @@ static Aggregator getAggregator( return meterProviderSharedState .getViewRegistry() .findView(descriptor) + .getAggregatorFactory() .create( meterProviderSharedState.getResource(), meterSharedState.getInstrumentationLibraryInfo(), diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java index a925e76e7fa..73a75926f1f 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java @@ -9,10 +9,10 @@ import io.opentelemetry.api.metrics.MeterProvider; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.internal.ComponentRegistry; -import io.opentelemetry.sdk.metrics.aggregator.AggregatorFactory; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.export.MetricProducer; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.metrics.view.View; import io.opentelemetry.sdk.resources.Resource; import java.util.ArrayList; import java.util.Collection; @@ -95,13 +95,14 @@ public static SdkMeterProviderBuilder builder() { * .buildInstrument(); * * // create a specification of how you want the metrics aggregated: - * AggregationFactory aggregationFactory = AggregationFactory.minMaxSumCount(); + * AggregatorFactory aggregatorFactory = AggregatorFactory.minMaxSumCount(); * * //register the view with the MeterSdkProvider - * meterProvider.registerView(instrumentSelector, aggregationFactory); + * meterProvider.registerView(instrumentSelector, View.builder() + * .setAggregatorFactory(aggregatorFactory).build()); * } */ - public void registerView(InstrumentSelector selector, AggregatorFactory aggregatorFactory) { - sharedState.getViewRegistry().registerView(selector, aggregatorFactory); + public void registerView(InstrumentSelector selector, View view) { + sharedState.getViewRegistry().registerView(selector, view); } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java index ff18d057f37..78a0c3e88cb 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java @@ -9,6 +9,7 @@ import io.opentelemetry.sdk.metrics.common.InstrumentDescriptor; import io.opentelemetry.sdk.metrics.common.InstrumentType; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.metrics.view.View; import java.util.EnumMap; import java.util.LinkedHashMap; import java.util.Map; @@ -23,15 +24,17 @@ * never blocked. */ final class ViewRegistry { - private static final LinkedHashMap EMPTY_CONFIG = - new LinkedHashMap<>(); - static final AggregatorFactory CUMULATIVE_SUM = AggregatorFactory.sum(true); - static final AggregatorFactory SUMMARY = AggregatorFactory.minMaxSumCount(); - static final AggregatorFactory LAST_VALUE = AggregatorFactory.lastValue(); + private static final LinkedHashMap EMPTY_CONFIG = new LinkedHashMap<>(); + static final View CUMULATIVE_SUM = + View.builder().setAggregatorFactory(AggregatorFactory.sum(true)).build(); + static final View SUMMARY = + View.builder().setAggregatorFactory(AggregatorFactory.minMaxSumCount()).build(); + static final View LAST_VALUE = + View.builder().setAggregatorFactory(AggregatorFactory.lastValue()).build(); // The lock is used to ensure only one updated to the configuration happens at any moment. private final ReentrantLock lock = new ReentrantLock(); - private volatile EnumMap> configuration; + private volatile EnumMap> configuration; ViewRegistry() { this.configuration = new EnumMap<>(InstrumentType.class); @@ -43,16 +46,16 @@ final class ViewRegistry { configuration.put(InstrumentType.VALUE_OBSERVER, EMPTY_CONFIG); } - void registerView(InstrumentSelector selector, AggregatorFactory aggregatorFactory) { + void registerView(InstrumentSelector selector, View view) { lock.lock(); try { - EnumMap> newConfiguration = + EnumMap> newConfiguration = new EnumMap<>(configuration); newConfiguration.put( selector.getInstrumentType(), newLinkedHashMap( selector.getInstrumentNamePattern(), - aggregatorFactory, + view, newConfiguration.get(selector.getInstrumentType()))); configuration = newConfiguration; } finally { @@ -60,10 +63,9 @@ void registerView(InstrumentSelector selector, AggregatorFactory aggregatorFacto } } - AggregatorFactory findView(InstrumentDescriptor descriptor) { - LinkedHashMap configPerType = - configuration.get(descriptor.getType()); - for (Map.Entry entry : configPerType.entrySet()) { + View findView(InstrumentDescriptor descriptor) { + LinkedHashMap configPerType = configuration.get(descriptor.getType()); + for (Map.Entry entry : configPerType.entrySet()) { if (entry.getKey().matcher(descriptor.getName()).matches()) { return entry.getValue(); } @@ -72,7 +74,7 @@ AggregatorFactory findView(InstrumentDescriptor descriptor) { return getDefaultSpecification(descriptor); } - private static AggregatorFactory getDefaultSpecification(InstrumentDescriptor descriptor) { + private static View getDefaultSpecification(InstrumentDescriptor descriptor) { switch (descriptor.getType()) { case COUNTER: case UP_DOWN_COUNTER: @@ -87,12 +89,10 @@ private static AggregatorFactory getDefaultSpecification(InstrumentDescriptor de throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); } - private static LinkedHashMap newLinkedHashMap( - Pattern pattern, - AggregatorFactory aggregatorFactory, - LinkedHashMap parentConfiguration) { - LinkedHashMap result = new LinkedHashMap<>(); - result.put(pattern, aggregatorFactory); + private static LinkedHashMap newLinkedHashMap( + Pattern pattern, View view, LinkedHashMap parentConfiguration) { + LinkedHashMap result = new LinkedHashMap<>(); + result.put(pattern, view); result.putAll(parentConfiguration); return result; } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/View.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/View.java new file mode 100644 index 00000000000..7d20a27a0d4 --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/View.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics.view; + +import com.google.auto.value.AutoValue; +import io.opentelemetry.sdk.metrics.aggregator.AggregatorFactory; +import javax.annotation.concurrent.Immutable; + +/** TODO: javadoc. */ +@AutoValue +@Immutable +public abstract class View { + public abstract AggregatorFactory getAggregatorFactory(); + + public static ViewBuilder builder() { + return new ViewBuilder(); + } + + static View create(AggregatorFactory aggregatorFactory) { + return new AutoValue_View(aggregatorFactory); + } +} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/ViewBuilder.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/ViewBuilder.java new file mode 100644 index 00000000000..21d940b1182 --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/ViewBuilder.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics.view; + +import io.opentelemetry.sdk.metrics.aggregator.AggregatorFactory; +import java.util.Objects; + +public final class ViewBuilder { + private AggregatorFactory aggregatorFactory; + + ViewBuilder() {} + + public ViewBuilder setAggregatorFactory(AggregatorFactory aggregatorFactory) { + this.aggregatorFactory = Objects.requireNonNull(aggregatorFactory, "aggregatorFactory"); + return this; + } + + public View build() { + return View.create(this.aggregatorFactory); + } +} diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java index 4a7b178cf67..2b671cab467 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java @@ -32,6 +32,7 @@ import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.data.ValueAtPercentile; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.metrics.view.View; import io.opentelemetry.sdk.resources.Resource; import java.util.Arrays; import java.util.Collections; @@ -161,7 +162,7 @@ void collectAllSyncInstruments() { void collectAllSyncInstruments_OverwriteTemporality() { sdkMeterProvider.registerView( InstrumentSelector.builder().setInstrumentType(InstrumentType.COUNTER).build(), - AggregatorFactory.sum(false)); + View.builder().setAggregatorFactory(AggregatorFactory.sum(false)).build()); LongCounter longCounter = sdkMeter.longCounterBuilder("testLongCounter").build(); longCounter.add(10, Labels.empty()); @@ -671,7 +672,8 @@ private static void registerViewForAllTypes( SdkMeterProvider meterProvider, AggregatorFactory factory) { for (InstrumentType instrumentType : InstrumentType.values()) { meterProvider.registerView( - InstrumentSelector.builder().setInstrumentType(instrumentType).build(), factory); + InstrumentSelector.builder().setInstrumentType(instrumentType).build(), + View.builder().setAggregatorFactory(factory).build()); } } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java index 73f2195a57e..33e4da5b8b9 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java @@ -12,12 +12,14 @@ import io.opentelemetry.sdk.metrics.common.InstrumentType; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.metrics.view.View; import org.junit.jupiter.api.Test; class ViewRegistryTest { @Test void selection_onType() { AggregatorFactory factory = AggregatorFactory.lastValue(); + View view = View.builder().setAggregatorFactory(factory).build(); ViewRegistry viewRegistry = new ViewRegistry(); viewRegistry.registerView( @@ -25,12 +27,12 @@ void selection_onType() { .setInstrumentType(InstrumentType.COUNTER) .setInstrumentNameRegex(".*") .build(), - factory); + view); assertThat( viewRegistry.findView( InstrumentDescriptor.create( "", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) - .isEqualTo(factory); + .isEqualTo(view); // this one hasn't been configured, so it gets the default still. assertThat( viewRegistry.findView( @@ -42,6 +44,7 @@ void selection_onType() { @Test void selection_onName() { AggregatorFactory factory = AggregatorFactory.lastValue(); + View view = View.builder().setAggregatorFactory(factory).build(); ViewRegistry viewRegistry = new ViewRegistry(); viewRegistry.registerView( @@ -49,12 +52,12 @@ void selection_onName() { .setInstrumentType(InstrumentType.COUNTER) .setInstrumentNameRegex("overridden") .build(), - factory); + view); assertThat( viewRegistry.findView( InstrumentDescriptor.create( "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) - .isSameAs(factory); + .isSameAs(view); // this one hasn't been configured, so it gets the default still. assertThat( viewRegistry.findView( @@ -66,7 +69,9 @@ void selection_onName() { @Test void selection_LastAddedViewWins() { AggregatorFactory factory1 = AggregatorFactory.lastValue(); + View view1 = View.builder().setAggregatorFactory(factory1).build(); AggregatorFactory factory2 = AggregatorFactory.minMaxSumCount(); + View view2 = View.builder().setAggregatorFactory(factory2).build(); ViewRegistry viewRegistry = new ViewRegistry(); viewRegistry.registerView( @@ -74,29 +79,30 @@ void selection_LastAddedViewWins() { .setInstrumentType(InstrumentType.COUNTER) .setInstrumentNameRegex(".*") .build(), - factory1); + view1); viewRegistry.registerView( InstrumentSelector.builder() .setInstrumentType(InstrumentType.COUNTER) .setInstrumentNameRegex("overridden") .build(), - factory2); + view2); assertThat( viewRegistry.findView( InstrumentDescriptor.create( "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) - .isEqualTo(factory2); + .isEqualTo(view2); assertThat( viewRegistry.findView( InstrumentDescriptor.create( "default", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) - .isEqualTo(factory1); + .isEqualTo(view1); } @Test void selection_regex() { AggregatorFactory factory = AggregatorFactory.lastValue(); + View view = View.builder().setAggregatorFactory(factory).build(); ViewRegistry viewRegistry = new ViewRegistry(); viewRegistry.registerView( @@ -104,18 +110,18 @@ void selection_regex() { .setInstrumentNameRegex("overrid(es|den)") .setInstrumentType(InstrumentType.COUNTER) .build(), - factory); + view); assertThat( viewRegistry.findView( InstrumentDescriptor.create( "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) - .isEqualTo(factory); + .isEqualTo(view); assertThat( viewRegistry.findView( InstrumentDescriptor.create( "overrides", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) - .isEqualTo(factory); + .isEqualTo(view); // this one hasn't been configured, so it gets the default still.. assertThat( viewRegistry.findView(