From 631d00f9d92ecf3578190e2ded610f2977ea44eb Mon Sep 17 00:00:00 2001 From: jwatson Date: Tue, 14 Jul 2020 12:37:08 -0700 Subject: [PATCH 01/37] Create a very basic view API in the SDK. --- .../sdk/metrics/ActiveBatcher.java | 5 + .../io/opentelemetry/sdk/metrics/Batcher.java | 3 + .../opentelemetry/sdk/metrics/Batchers.java | 10 + .../sdk/metrics/MeterSdkProvider.java | 28 ++- .../sdk/metrics/ViewRegistry.java | 62 ++++-- .../sdk/metrics/view/InstrumentSelector.java | 31 +++ .../sdk/metrics/view/ViewRegistryTest.java | 176 ++++++++++++++++++ 7 files changed, 295 insertions(+), 20 deletions(-) create mode 100644 sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java create mode 100644 sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ActiveBatcher.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ActiveBatcher.java index af704b14429..c1ec632d198 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ActiveBatcher.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ActiveBatcher.java @@ -41,4 +41,9 @@ public void batch(Labels labelSet, Aggregator aggregator, boolean mappedAggregat public List completeCollectionCycle() { return batcher.completeCollectionCycle(); } + + @Override + public boolean generatesDeltas() { + return batcher.generatesDeltas(); + } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java index d1c3277ab4f..4f254535b6d 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java @@ -51,4 +51,7 @@ interface Batcher { * @return the list of metrics batched in this Batcher. */ List completeCollectionCycle(); + + /** Does this batcher generate "delta" style metrics. The alternative is "cumulative". */ + boolean generatesDeltas(); } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java index a5e0bc3ff08..180531b4a43 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java @@ -80,6 +80,11 @@ public void batch(Labels labelSet, Aggregator aggregator, boolean mappedAggregat public List completeCollectionCycle() { return Collections.emptyList(); } + + @Override + public boolean generatesDeltas() { + return false; + } } private static final class AllLabels implements Batcher { @@ -155,6 +160,11 @@ public final List completeCollectionCycle() { aggregation.getDescriptorType(descriptor.getType(), descriptor.getValueType()), points)); } + + @Override + public boolean generatesDeltas() { + return delta; + } } private Batchers() {} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java index dae53e29244..4d4a05afa33 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java @@ -31,11 +31,12 @@ public final class MeterSdkProvider implements MeterProvider { private final MeterSdkComponentRegistry registry; private final MetricProducer metricProducer; + private final ViewRegistry viewRegistry = new ViewRegistry(); private MeterSdkProvider(Clock clock, Resource resource) { this.registry = new MeterSdkComponentRegistry( - MeterProviderSharedState.create(clock, resource), new ViewRegistry()); + MeterProviderSharedState.create(clock, resource), viewRegistry); this.metricProducer = new MetricProducerSdk(this.registry); } @@ -135,6 +136,31 @@ public MeterSdk newComponent(InstrumentationLibraryInfo instrumentationLibraryIn } } + /** + * Register a view with the given {@link InstrumentSelector}. + *

+ * Example on how to register a view: + * + * 

{@code
+   * // get a handle to the MeterSdkProvider
+   * MeterSdkProvider meterProvider = OpenTelemetrySdk.getMeterProvider();
+   *
+   * // create a selector to select which instruments to customize:
+   * InstrumentSelector instrumentSelector = InstrumentSelector.create(InstrumentType.COUNTER);
+   *
+   * // create a specification of how you want the metrics aggregated:
+   * ViewSpecification viewSpecification = ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA);
+   *
+   * //register the view with the MeterSdkProvider
+   * meterProvider.registerView(instrumentSelector, viewSpecification);
+   *
+ * + * @see ViewSpecification + */ + public void registerView(InstrumentSelector selector, ViewSpecification specification) { + viewRegistry.registerView(selector, specification); + } + private static final class MetricProducerSdk implements MetricProducer { private final MeterSdkComponentRegistry registry; 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 b1fd0dd6726..4532baaa64d 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 @@ -7,6 +7,9 @@ import io.opentelemetry.sdk.metrics.view.Aggregation; import io.opentelemetry.sdk.metrics.view.Aggregations; +import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; // notes: // specify by pieces of the descriptor. @@ -27,6 +30,16 @@ */ class ViewRegistry { + public static final ViewSpecification CUMULATIVE_SUM = + ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + public static final ViewSpecification DELTA_SUMMARY = + ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA); + public static final ViewSpecification CUMULATIVE_LAST_VALUE = + ViewSpecification.create(Aggregations.lastValue(), Temporality.CUMULATIVE); + + private final Map configuration = + new ConcurrentHashMap<>(); + /** * Create a new {@link io.opentelemetry.sdk.metrics.Batcher} for use in metric recording * aggregation. @@ -36,28 +49,34 @@ Batcher createBatcher( MeterSharedState meterSharedState, InstrumentDescriptor descriptor) { - Aggregation aggregation = getRegisteredAggregation(descriptor); + ViewSpecification specification = findBestMatch(descriptor); - // todo: don't just use the defaults! - switch (descriptor.getType()) { - case COUNTER: - case UP_DOWN_COUNTER: - case SUM_OBSERVER: - case UP_DOWN_SUM_OBSERVER: - return Batchers.getCumulativeAllLabels( - descriptor, meterProviderSharedState, meterSharedState, aggregation); - case VALUE_RECORDER: - // TODO: Revisit the batcher used here for value observers, - // currently this does not remove duplicate records in the same cycle. - case VALUE_OBSERVER: - return Batchers.getDeltaAllLabels( - descriptor, meterProviderSharedState, meterSharedState, aggregation); + Aggregation aggregation = specification.aggregation(); + + if (Temporality.CUMULATIVE == specification.temporality()) { + return Batchers.getCumulativeAllLabels( + descriptor, meterProviderSharedState, meterSharedState, aggregation); + } else if (Temporality.DELTA == specification.temporality()) { + return Batchers.getDeltaAllLabels( + descriptor, meterProviderSharedState, meterSharedState, aggregation); } - throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); + throw new IllegalStateException("unsupported Temporality: " + specification.temporality()); + } + + // todo: consider moving this method to its own class, for more targetted testing. + private ViewSpecification findBestMatch(InstrumentDescriptor descriptor) { + // select based on InstrumentType: + for (Map.Entry entry : configuration.entrySet()) { + InstrumentSelector registeredSelector = entry.getKey(); + if (registeredSelector.instrumentType().equals(descriptor.getType())) { + return entry.getValue(); + } + } + // If none found, use the defaults: + return getDefaultSpecification(descriptor); } - private static Aggregation getRegisteredAggregation(InstrumentDescriptor descriptor) { - // todo look up based on fields of the descriptor. + private static ViewSpecification getDefaultSpecification(InstrumentDescriptor descriptor) { switch (descriptor.getType()) { case COUNTER: case UP_DOWN_COUNTER: @@ -67,8 +86,13 @@ private static Aggregation getRegisteredAggregation(InstrumentDescriptor descrip case VALUE_OBSERVER: case SUM_OBSERVER: case UP_DOWN_SUM_OBSERVER: - return Aggregations.lastValue(); + return CUMULATIVE_LAST_VALUE; } throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); } + + /** todo: javadoc me. */ + public void registerView(InstrumentSelector selector, ViewSpecification specification) { + configuration.put(selector, specification); + } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java new file mode 100644 index 00000000000..070129cb657 --- /dev/null +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -0,0 +1,31 @@ +/* + * Copyright 2020, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.sdk.metrics.view; + +import com.google.auto.value.AutoValue; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import javax.annotation.concurrent.Immutable; + +@AutoValue +@Immutable +public abstract class InstrumentSelector { + public static InstrumentSelector create(InstrumentType instrumentType) { + return new AutoValue_InstrumentSelector(instrumentType); + } + + public abstract InstrumentType instrumentType(); +} diff --git a/sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java b/sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java new file mode 100644 index 00000000000..0e5c8d488c3 --- /dev/null +++ b/sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java @@ -0,0 +1,176 @@ +/* + * Copyright 2020, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.sdk.metrics.view; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.when; + +import io.opentelemetry.common.Labels; +import io.opentelemetry.sdk.internal.TestClock; +import io.opentelemetry.sdk.metrics.Batcher; +import io.opentelemetry.sdk.metrics.InstrumentDescriptor; +import io.opentelemetry.sdk.metrics.MeterProviderSharedState; +import io.opentelemetry.sdk.metrics.MeterSharedState; +import io.opentelemetry.sdk.metrics.aggregator.DoubleLastValueAggregator; +import io.opentelemetry.sdk.metrics.aggregator.DoubleMinMaxSumCount; +import io.opentelemetry.sdk.metrics.aggregator.DoubleSumAggregator; +import io.opentelemetry.sdk.metrics.aggregator.LongLastValueAggregator; +import io.opentelemetry.sdk.metrics.aggregator.LongMinMaxSumCount; +import io.opentelemetry.sdk.metrics.aggregator.LongSumAggregator; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class ViewRegistryTest { + + @Mock private MeterSharedState meterSharedState; + @Mock private MeterProviderSharedState meterProviderSharedState; + + @Before + public void setUp() { + when(meterProviderSharedState.getClock()).thenReturn(TestClock.create()); + } + + @Test + public void defaultAggregations() { + ViewRegistry viewRegistry = new ViewRegistry(); + + verifyCorrect( + viewRegistry, + InstrumentType.VALUE_RECORDER, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ true, + DoubleMinMaxSumCount.class); + verifyCorrect( + viewRegistry, + InstrumentType.VALUE_RECORDER, + InstrumentValueType.LONG, + /* expectedDeltas=*/ true, + LongMinMaxSumCount.class); + + verifyCorrect( + viewRegistry, + InstrumentType.VALUE_OBSERVER, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ true, + DoubleMinMaxSumCount.class); + verifyCorrect( + viewRegistry, + InstrumentType.VALUE_OBSERVER, + InstrumentValueType.LONG, + /* expectedDeltas=*/ true, + LongMinMaxSumCount.class); + + verifyCorrect( + viewRegistry, + InstrumentType.COUNTER, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ false, + DoubleSumAggregator.class); + verifyCorrect( + viewRegistry, + InstrumentType.COUNTER, + InstrumentValueType.LONG, + /* expectedDeltas=*/ false, + LongSumAggregator.class); + + verifyCorrect( + viewRegistry, + InstrumentType.UP_DOWN_COUNTER, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ false, + DoubleSumAggregator.class); + verifyCorrect( + viewRegistry, + InstrumentType.UP_DOWN_COUNTER, + InstrumentValueType.LONG, + /* expectedDeltas=*/ false, + LongSumAggregator.class); + + verifyCorrect( + viewRegistry, + InstrumentType.SUM_OBSERVER, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ false, + DoubleLastValueAggregator.class); + verifyCorrect( + viewRegistry, + InstrumentType.SUM_OBSERVER, + InstrumentValueType.LONG, + /* expectedDeltas=*/ false, + LongLastValueAggregator.class); + + verifyCorrect( + viewRegistry, + InstrumentType.UP_DOWN_SUM_OBSERVER, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ false, + DoubleLastValueAggregator.class); + verifyCorrect( + viewRegistry, + InstrumentType.UP_DOWN_SUM_OBSERVER, + InstrumentValueType.LONG, + /* expectedDeltas=*/ false, + LongLastValueAggregator.class); + } + + @Test + public void selectByInstrumentType() { + ViewRegistry viewRegistry = new ViewRegistry(); + + InstrumentType instrumentType = InstrumentType.VALUE_RECORDER; + + InstrumentSelector selector = InstrumentSelector.create(instrumentType); + ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + viewRegistry.registerView(selector, view); + + verifyCorrect( + viewRegistry, + instrumentType, + InstrumentValueType.DOUBLE, + /* expectedDeltas=*/ false, + DoubleSumAggregator.class); + } + + private void verifyCorrect( + ViewRegistry viewRegistry, + InstrumentType instrumentType, + InstrumentValueType valueType, + boolean expectedDeltas, + Class expectedAggregator) { + Batcher batcher = + viewRegistry.createBatcher( + meterProviderSharedState, + meterSharedState, + createDescriptor(instrumentType, valueType)); + + assertThat(batcher.generatesDeltas()).isEqualTo(expectedDeltas); + assertThat(batcher.getAggregator()).isInstanceOf(expectedAggregator); + } + + private static InstrumentDescriptor createDescriptor( + InstrumentType instrumentType, InstrumentValueType instrumentValueType) { + return InstrumentDescriptor.create( + "foo", "foo desc", "ms", Labels.empty(), instrumentType, instrumentValueType); + } +} From 9bbbe44633d10e1fc000add4fc20aa188cbd02ed Mon Sep 17 00:00:00 2001 From: jwatson Date: Tue, 14 Jul 2020 12:48:17 -0700 Subject: [PATCH 02/37] fix formatting --- .../sdk/metrics/MeterSdkProvider.java | 9 ++-- .../sdk/metrics/view/ViewSpecification.java | 51 +++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java index 4d4a05afa33..fb7b5e60ff0 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java @@ -138,8 +138,8 @@ public MeterSdk newComponent(InstrumentationLibraryInfo instrumentationLibraryIn /** * Register a view with the given {@link InstrumentSelector}. - *

- * Example on how to register a view: + * + *

Example on how to register a view: * * 

{@code
    * // get a handle to the MeterSdkProvider
@@ -149,11 +149,12 @@ public MeterSdk newComponent(InstrumentationLibraryInfo instrumentationLibraryIn
    * InstrumentSelector instrumentSelector = InstrumentSelector.create(InstrumentType.COUNTER);
    *
    * // create a specification of how you want the metrics aggregated:
-   * ViewSpecification viewSpecification = ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA);
+   * ViewSpecification viewSpecification =
+   *   ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA);
    *
    * //register the view with the MeterSdkProvider
    * meterProvider.registerView(instrumentSelector, viewSpecification);
-   *
+ * } * * @see ViewSpecification */ diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java new file mode 100644 index 00000000000..9700506aec1 --- /dev/null +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java @@ -0,0 +1,51 @@ +/* + * Copyright 2020, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.sdk.metrics.view; + +import com.google.auto.value.AutoValue; +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; + +@AutoValue +@Immutable +public abstract class ViewSpecification { + + public static ViewSpecification create(Aggregation aggregation, Temporality temporality) { + return new AutoValue_ViewSpecification(aggregation, temporality); + } + + /** Which aggregation should be used for this View. */ + @Nullable + public abstract Aggregation aggregation(); + + /** What temporality should be used for this View (delta vs. cumulative). */ + @Nullable + public abstract Temporality temporality(); + + /** An enumeration which describes the time period over which metrics should be aggregated. */ + public enum Temporality { + /** + * DELTA means that metrics will be aggregated only over the most recent collection interval. + */ + DELTA, + /** + * CUMULATIVE means that metrics will be aggregated over the lifetime of the associated {@link + * io.opentelemetry.metrics.Instrument}. + */ + CUMULATIVE + } +} From 6063ff45f5ccacab7dc12ecb28a7e86a840d8f94 Mon Sep 17 00:00:00 2001 From: jwatson Date: Tue, 14 Jul 2020 13:12:21 -0700 Subject: [PATCH 03/37] move the ViewRegistry up one package, and clean up the visibility of other classes --- .../io/opentelemetry/sdk/metrics/ViewRegistry.java | 11 ++++++----- .../sdk/metrics/{view => }/ViewRegistryTest.java | 9 ++++----- 2 files changed, 10 insertions(+), 10 deletions(-) rename sdk/src/test/java/io/opentelemetry/sdk/metrics/{view => }/ViewRegistryTest.java (95%) 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 4532baaa64d..68326f523e3 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 @@ -7,6 +7,8 @@ import io.opentelemetry.sdk.metrics.view.Aggregation; import io.opentelemetry.sdk.metrics.view.Aggregations; +import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.metrics.view.ViewSpecification; import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -30,11 +32,11 @@ */ class ViewRegistry { - public static final ViewSpecification CUMULATIVE_SUM = + private static final ViewSpecification CUMULATIVE_SUM = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); - public static final ViewSpecification DELTA_SUMMARY = + private static final ViewSpecification DELTA_SUMMARY = ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA); - public static final ViewSpecification CUMULATIVE_LAST_VALUE = + private static final ViewSpecification CUMULATIVE_LAST_VALUE = ViewSpecification.create(Aggregations.lastValue(), Temporality.CUMULATIVE); private final Map configuration = @@ -91,8 +93,7 @@ private static ViewSpecification getDefaultSpecification(InstrumentDescriptor de throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); } - /** todo: javadoc me. */ - public void registerView(InstrumentSelector selector, ViewSpecification specification) { + void registerView(InstrumentSelector selector, ViewSpecification specification) { configuration.put(selector, specification); } } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java b/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java similarity index 95% rename from sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java rename to sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java index 0e5c8d488c3..a5901c0b12a 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/metrics/view/ViewRegistryTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java @@ -14,17 +14,13 @@ * limitations under the License. */ -package io.opentelemetry.sdk.metrics.view; +package io.opentelemetry.sdk.metrics; import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.when; import io.opentelemetry.common.Labels; import io.opentelemetry.sdk.internal.TestClock; -import io.opentelemetry.sdk.metrics.Batcher; -import io.opentelemetry.sdk.metrics.InstrumentDescriptor; -import io.opentelemetry.sdk.metrics.MeterProviderSharedState; -import io.opentelemetry.sdk.metrics.MeterSharedState; import io.opentelemetry.sdk.metrics.aggregator.DoubleLastValueAggregator; import io.opentelemetry.sdk.metrics.aggregator.DoubleMinMaxSumCount; import io.opentelemetry.sdk.metrics.aggregator.DoubleSumAggregator; @@ -33,6 +29,9 @@ import io.opentelemetry.sdk.metrics.aggregator.LongSumAggregator; import io.opentelemetry.sdk.metrics.common.InstrumentType; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.Aggregations; +import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.metrics.view.ViewSpecification; import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; import org.junit.Before; import org.junit.Test; From 8432091a84adf183c475b558bef598a506e002da Mon Sep 17 00:00:00 2001 From: jwatson Date: Tue, 14 Jul 2020 15:16:43 -0700 Subject: [PATCH 04/37] Support matching by instrument name --- .../sdk/metrics/MeterSdkProvider.java | 4 +- .../sdk/metrics/ViewRegistry.java | 36 ++++++-- .../sdk/metrics/view/InstrumentSelector.java | 27 +++++- .../sdk/metrics/ViewRegistryTest.java | 87 +++++++++++++++++-- 4 files changed, 137 insertions(+), 17 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java index fb7b5e60ff0..f1ab3219231 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java @@ -146,7 +146,9 @@ public MeterSdk newComponent(InstrumentationLibraryInfo instrumentationLibraryIn * MeterSdkProvider meterProvider = OpenTelemetrySdk.getMeterProvider(); * * // create a selector to select which instruments to customize: - * InstrumentSelector instrumentSelector = InstrumentSelector.create(InstrumentType.COUNTER); + * InstrumentSelector instrumentSelector = InstrumentSelector.newBuilder() + * .instrumentType(InstrumentType.COUNTER) + * .build(); * * // create a specification of how you want the metrics aggregated: * ViewSpecification viewSpecification = 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 68326f523e3..3b2c53f7f9f 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 @@ -11,20 +11,22 @@ import io.opentelemetry.sdk.metrics.view.ViewSpecification; import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; // notes: // specify by pieces of the descriptor. -// instrument type -// instrument value type -// instrument name (wildcards allowed?) +// instrument type √ +// instrument name (regex) √ +// instrument value type (?) // constant labels (?) // units (?) // what you can choose: -// aggregation +// aggregation √ +// delta vs. cumulative √ // all labels vs. a list of labels -// delta vs. cumulative /** * Central location for Views to be registered. Registration of a view should eventually be done via @@ -67,17 +69,37 @@ Batcher createBatcher( // todo: consider moving this method to its own class, for more targetted testing. private ViewSpecification findBestMatch(InstrumentDescriptor descriptor) { - // select based on InstrumentType: + for (Map.Entry entry : configuration.entrySet()) { InstrumentSelector registeredSelector = entry.getKey(); - if (registeredSelector.instrumentType().equals(descriptor.getType())) { + + if (matchesOnName(descriptor, registeredSelector) + && matchesOnType(descriptor, registeredSelector)) { return entry.getValue(); } } + // If none found, use the defaults: return getDefaultSpecification(descriptor); } + private static boolean matchesOnType( + InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { + if (registeredSelector.instrumentType() == null) { + return true; + } + return Objects.equals(registeredSelector.instrumentType(), descriptor.getType()); + } + + private static boolean matchesOnName( + InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { + Pattern pattern = registeredSelector.instrumentNamePattern(); + if (pattern == null) { + return true; + } + return pattern.matcher(descriptor.getName()).matches(); + } + private static ViewSpecification getDefaultSpecification(InstrumentDescriptor descriptor) { switch (descriptor.getType()) { case COUNTER: diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 070129cb657..5538f9574a7 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -17,15 +17,38 @@ package io.opentelemetry.sdk.metrics.view; import com.google.auto.value.AutoValue; +import com.google.auto.value.extension.memoized.Memoized; import io.opentelemetry.sdk.metrics.common.InstrumentType; +import java.util.regex.Pattern; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @AutoValue @Immutable public abstract class InstrumentSelector { - public static InstrumentSelector create(InstrumentType instrumentType) { - return new AutoValue_InstrumentSelector(instrumentType); + + public static Builder newBuilder() { + return new AutoValue_InstrumentSelector.Builder(); } + @Nullable public abstract InstrumentType instrumentType(); + + @Nullable + public abstract String instrumentNameRegex(); + + @Memoized + @Nullable + public Pattern instrumentNamePattern() { + return instrumentNameRegex() == null ? null : Pattern.compile(instrumentNameRegex()); + } + + @AutoValue.Builder + public interface Builder { + Builder instrumentType(InstrumentType instrumentType); + + Builder instrumentNameRegex(String regex); + + InstrumentSelector build(); + } } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java b/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java index a5901c0b12a..3855d462e39 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java @@ -139,7 +139,8 @@ public void selectByInstrumentType() { InstrumentType instrumentType = InstrumentType.VALUE_RECORDER; - InstrumentSelector selector = InstrumentSelector.create(instrumentType); + InstrumentSelector selector = + InstrumentSelector.newBuilder().instrumentType(instrumentType).build(); ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); viewRegistry.registerView(selector, view); @@ -151,25 +152,97 @@ public void selectByInstrumentType() { DoubleSumAggregator.class); } + @Test + public void selectByInstrumentName() { + ViewRegistry viewRegistry = new ViewRegistry(); + + InstrumentSelector selector = + InstrumentSelector.newBuilder().instrumentNameRegex("http.*duration").build(); + ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + + viewRegistry.registerView(selector, view); + + InstrumentType instrumentType = InstrumentType.VALUE_RECORDER; + // this one matches on name + verifyCorrect( + viewRegistry, + createDescriptor(instrumentType, InstrumentValueType.DOUBLE, "http.server.duration"), + /* expectedDeltas= */ false, + DoubleSumAggregator.class); + // this one does not match on name + verifyCorrect( + viewRegistry, + createDescriptor(instrumentType, InstrumentValueType.DOUBLE, "foo.bar.duration"), + /* expectedDeltas=*/ true, + DoubleMinMaxSumCount.class); + } + + @Test + public void selectByInstrumentNameAndType() { + ViewRegistry viewRegistry = new ViewRegistry(); + + InstrumentSelector selector = + InstrumentSelector.newBuilder() + .instrumentType(InstrumentType.VALUE_RECORDER) + .instrumentNameRegex("http.*duration") + .build(); + ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + + viewRegistry.registerView(selector, view); + + // this one matches on name + verifyCorrect( + viewRegistry, + createDescriptor( + InstrumentType.VALUE_RECORDER, InstrumentValueType.DOUBLE, "http.server.duration"), + /* expectedDeltas= */ false, + DoubleSumAggregator.class); + // this one does not match on name, but does on type, so should get the default + verifyCorrect( + viewRegistry, + createDescriptor( + InstrumentType.VALUE_RECORDER, InstrumentValueType.DOUBLE, "foo.bar.duration"), + /* expectedDeltas=*/ true, + DoubleMinMaxSumCount.class); + // this one does not match on type, but does on name, so should get the default + verifyCorrect( + viewRegistry, + createDescriptor( + InstrumentType.SUM_OBSERVER, InstrumentValueType.DOUBLE, "http.bar.duration"), + /* expectedDeltas=*/ false, + DoubleLastValueAggregator.class); + } + private void verifyCorrect( ViewRegistry viewRegistry, InstrumentType instrumentType, InstrumentValueType valueType, boolean expectedDeltas, Class expectedAggregator) { + verifyCorrect( + viewRegistry, + createDescriptor(instrumentType, valueType, "foo"), + expectedDeltas, + expectedAggregator); + } + + private void verifyCorrect( + ViewRegistry viewRegistry, + InstrumentDescriptor descriptor, + boolean expectedDeltas, + Class expectedAggregator) { Batcher batcher = - viewRegistry.createBatcher( - meterProviderSharedState, - meterSharedState, - createDescriptor(instrumentType, valueType)); + viewRegistry.createBatcher(meterProviderSharedState, meterSharedState, descriptor); assertThat(batcher.generatesDeltas()).isEqualTo(expectedDeltas); assertThat(batcher.getAggregator()).isInstanceOf(expectedAggregator); } private static InstrumentDescriptor createDescriptor( - InstrumentType instrumentType, InstrumentValueType instrumentValueType) { + InstrumentType instrumentType, + InstrumentValueType instrumentValueType, + String instrumentName) { return InstrumentDescriptor.create( - "foo", "foo desc", "ms", Labels.empty(), instrumentType, instrumentValueType); + instrumentName, "foo desc", "ms", Labels.empty(), instrumentType, instrumentValueType); } } From cbe903d8d64fed385357e7131c771fec13bddb1b Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 16 Jul 2020 10:25:01 -0700 Subject: [PATCH 05/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java Co-authored-by: Anuraag Agrawal --- .../main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3b2c53f7f9f..6f2cf2a8de2 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 @@ -88,7 +88,7 @@ private static boolean matchesOnType( if (registeredSelector.instrumentType() == null) { return true; } - return Objects.equals(registeredSelector.instrumentType(), descriptor.getType()); + return registeredSelector.instrumentType().equals(descriptor.getType()); } private static boolean matchesOnName( From d42a9deecca46999f24391726250a976b1663988 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 16 Jul 2020 10:25:15 -0700 Subject: [PATCH 06/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/ViewSpecification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java index 9700506aec1..4606154245d 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java @@ -28,7 +28,7 @@ public static ViewSpecification create(Aggregation aggregation, Temporality temp return new AutoValue_ViewSpecification(aggregation, temporality); } - /** Which aggregation should be used for this View. */ + /** Which {@link Aggregation} should be used for this View. */ @Nullable public abstract Aggregation aggregation(); From 7742cf93c28b935742ff7910f74ccc47a89b1cad Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 16 Jul 2020 10:25:28 -0700 Subject: [PATCH 07/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/ViewSpecification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java index 4606154245d..d135a597316 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java @@ -32,7 +32,7 @@ public static ViewSpecification create(Aggregation aggregation, Temporality temp @Nullable public abstract Aggregation aggregation(); - /** What temporality should be used for this View (delta vs. cumulative). */ + /** What {@link Temporality} should be used for this View (delta vs. cumulative). */ @Nullable public abstract Temporality temporality(); From ed2884372789210825c89ec316d529f31ffa650d Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 16 Jul 2020 10:26:06 -0700 Subject: [PATCH 08/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/ViewSpecification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java index d135a597316..cd78fa58bac 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java @@ -39,7 +39,7 @@ public static ViewSpecification create(Aggregation aggregation, Temporality temp /** An enumeration which describes the time period over which metrics should be aggregated. */ public enum Temporality { /** - * DELTA means that metrics will be aggregated only over the most recent collection interval. + * Metrics will be aggregated only over the most recent collection interval. */ DELTA, /** From ab50fc9296271e18a1044ab041456e9a5cda4179 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 16 Jul 2020 10:26:13 -0700 Subject: [PATCH 09/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/ViewSpecification.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java index cd78fa58bac..ed6f31e88f4 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java @@ -43,7 +43,7 @@ public enum Temporality { */ DELTA, /** - * CUMULATIVE means that metrics will be aggregated over the lifetime of the associated {@link + * Metrics will be aggregated over the lifetime of the associated {@link * io.opentelemetry.metrics.Instrument}. */ CUMULATIVE From 856ca821601430bd12addfd9dcab15a1f220ce57 Mon Sep 17 00:00:00 2001 From: jwatson Date: Thu, 16 Jul 2020 10:33:23 -0700 Subject: [PATCH 10/37] fix formatting issues from GH --- .../main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java | 1 - .../io/opentelemetry/sdk/metrics/view/ViewSpecification.java | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) 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 6f2cf2a8de2..c67b37295e8 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 @@ -11,7 +11,6 @@ import io.opentelemetry.sdk.metrics.view.ViewSpecification; import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; import java.util.Map; -import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java index ed6f31e88f4..b11a7fc1ee5 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java @@ -38,9 +38,7 @@ public static ViewSpecification create(Aggregation aggregation, Temporality temp /** An enumeration which describes the time period over which metrics should be aggregated. */ public enum Temporality { - /** - * Metrics will be aggregated only over the most recent collection interval. - */ + /** Metrics will be aggregated only over the most recent collection interval. */ DELTA, /** * Metrics will be aggregated over the lifetime of the associated {@link From eaa2b3d5e93ae22618976ffae04c0ebeb45c681f Mon Sep 17 00:00:00 2001 From: jwatson Date: Mon, 20 Jul 2020 14:47:26 -0700 Subject: [PATCH 11/37] small renaming to a big name --- .../sdk/metrics/MeterSdkProvider.java | 8 +++--- .../sdk/metrics/ViewRegistry.java | 28 +++++++++---------- ...ion.java => AggregationConfiguration.java} | 10 +++---- .../sdk/metrics/ViewRegistryTest.java | 13 +++++---- 4 files changed, 31 insertions(+), 28 deletions(-) rename sdk/src/main/java/io/opentelemetry/sdk/metrics/view/{ViewSpecification.java => AggregationConfiguration.java} (82%) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java index f1ab3219231..c0d4f3e1b8a 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java @@ -151,16 +151,16 @@ public MeterSdk newComponent(InstrumentationLibraryInfo instrumentationLibraryIn * .build(); * * // create a specification of how you want the metrics aggregated: - * ViewSpecification viewSpecification = - * ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA); + * AggregationConfiguration viewSpecification = + * AggregationConfiguration.create(Aggregations.minMaxSumCount(), Temporality.DELTA); * * //register the view with the MeterSdkProvider * meterProvider.registerView(instrumentSelector, viewSpecification); * } * - * @see ViewSpecification + * @see AggregationConfiguration */ - public void registerView(InstrumentSelector selector, ViewSpecification specification) { + public void registerView(InstrumentSelector selector, AggregationConfiguration specification) { viewRegistry.registerView(selector, specification); } 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 c67b37295e8..5ec0b85250d 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 @@ -6,10 +6,10 @@ package io.opentelemetry.sdk.metrics; import io.opentelemetry.sdk.metrics.view.Aggregation; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration.Temporality; import io.opentelemetry.sdk.metrics.view.Aggregations; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; -import io.opentelemetry.sdk.metrics.view.ViewSpecification; -import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; @@ -33,14 +33,14 @@ */ class ViewRegistry { - private static final ViewSpecification CUMULATIVE_SUM = - ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); - private static final ViewSpecification DELTA_SUMMARY = - ViewSpecification.create(Aggregations.minMaxSumCount(), Temporality.DELTA); - private static final ViewSpecification CUMULATIVE_LAST_VALUE = - ViewSpecification.create(Aggregations.lastValue(), Temporality.CUMULATIVE); + private static final AggregationConfiguration CUMULATIVE_SUM = + AggregationConfiguration.create(Aggregations.sum(), Temporality.CUMULATIVE); + private static final AggregationConfiguration DELTA_SUMMARY = + AggregationConfiguration.create(Aggregations.minMaxSumCount(), Temporality.DELTA); + private static final AggregationConfiguration CUMULATIVE_LAST_VALUE = + AggregationConfiguration.create(Aggregations.lastValue(), Temporality.CUMULATIVE); - private final Map configuration = + private final Map configuration = new ConcurrentHashMap<>(); /** @@ -52,7 +52,7 @@ Batcher createBatcher( MeterSharedState meterSharedState, InstrumentDescriptor descriptor) { - ViewSpecification specification = findBestMatch(descriptor); + AggregationConfiguration specification = findBestMatch(descriptor); Aggregation aggregation = specification.aggregation(); @@ -67,9 +67,9 @@ Batcher createBatcher( } // todo: consider moving this method to its own class, for more targetted testing. - private ViewSpecification findBestMatch(InstrumentDescriptor descriptor) { + private AggregationConfiguration findBestMatch(InstrumentDescriptor descriptor) { - for (Map.Entry entry : configuration.entrySet()) { + for (Map.Entry entry : configuration.entrySet()) { InstrumentSelector registeredSelector = entry.getKey(); if (matchesOnName(descriptor, registeredSelector) @@ -99,7 +99,7 @@ private static boolean matchesOnName( return pattern.matcher(descriptor.getName()).matches(); } - private static ViewSpecification getDefaultSpecification(InstrumentDescriptor descriptor) { + private static AggregationConfiguration getDefaultSpecification(InstrumentDescriptor descriptor) { switch (descriptor.getType()) { case COUNTER: case UP_DOWN_COUNTER: @@ -114,7 +114,7 @@ private static ViewSpecification getDefaultSpecification(InstrumentDescriptor de throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); } - void registerView(InstrumentSelector selector, ViewSpecification specification) { + void registerView(InstrumentSelector selector, AggregationConfiguration specification) { configuration.put(selector, specification); } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java similarity index 82% rename from sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java rename to sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index b11a7fc1ee5..c46df10037c 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/ViewSpecification.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -17,15 +17,16 @@ package io.opentelemetry.sdk.metrics.view; import com.google.auto.value.AutoValue; +import io.opentelemetry.metrics.Instrument; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @AutoValue @Immutable -public abstract class ViewSpecification { +public abstract class AggregationConfiguration { - public static ViewSpecification create(Aggregation aggregation, Temporality temporality) { - return new AutoValue_ViewSpecification(aggregation, temporality); + public static AggregationConfiguration create(Aggregation aggregation, Temporality temporality) { + return new AutoValue_AggregationConfiguration(aggregation, temporality); } /** Which {@link Aggregation} should be used for this View. */ @@ -41,8 +42,7 @@ public enum Temporality { /** Metrics will be aggregated only over the most recent collection interval. */ DELTA, /** - * Metrics will be aggregated over the lifetime of the associated {@link - * io.opentelemetry.metrics.Instrument}. + * Metrics will be aggregated over the lifetime of the associated {@link Instrument}. */ CUMULATIVE } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java b/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java index 3855d462e39..c08312cbeae 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java @@ -29,10 +29,10 @@ import io.opentelemetry.sdk.metrics.aggregator.LongSumAggregator; import io.opentelemetry.sdk.metrics.common.InstrumentType; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration.Temporality; import io.opentelemetry.sdk.metrics.view.Aggregations; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; -import io.opentelemetry.sdk.metrics.view.ViewSpecification; -import io.opentelemetry.sdk.metrics.view.ViewSpecification.Temporality; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -141,7 +141,8 @@ public void selectByInstrumentType() { InstrumentSelector selector = InstrumentSelector.newBuilder().instrumentType(instrumentType).build(); - ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + AggregationConfiguration view = + AggregationConfiguration.create(Aggregations.sum(), Temporality.CUMULATIVE); viewRegistry.registerView(selector, view); verifyCorrect( @@ -158,7 +159,8 @@ public void selectByInstrumentName() { InstrumentSelector selector = InstrumentSelector.newBuilder().instrumentNameRegex("http.*duration").build(); - ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + AggregationConfiguration view = + AggregationConfiguration.create(Aggregations.sum(), Temporality.CUMULATIVE); viewRegistry.registerView(selector, view); @@ -186,7 +188,8 @@ public void selectByInstrumentNameAndType() { .instrumentType(InstrumentType.VALUE_RECORDER) .instrumentNameRegex("http.*duration") .build(); - ViewSpecification view = ViewSpecification.create(Aggregations.sum(), Temporality.CUMULATIVE); + AggregationConfiguration view = + AggregationConfiguration.create(Aggregations.sum(), Temporality.CUMULATIVE); viewRegistry.registerView(selector, view); From 2d07c945dfda9d651a6a4e00e0c632fbde0a521b Mon Sep 17 00:00:00 2001 From: jwatson Date: Mon, 20 Jul 2020 14:47:49 -0700 Subject: [PATCH 12/37] small renaming to a big name --- .../sdk/metrics/view/AggregationConfiguration.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index c46df10037c..b1f35be59b9 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -41,9 +41,7 @@ public static AggregationConfiguration create(Aggregation aggregation, Temporali public enum Temporality { /** Metrics will be aggregated only over the most recent collection interval. */ DELTA, - /** - * Metrics will be aggregated over the lifetime of the associated {@link Instrument}. - */ + /** Metrics will be aggregated over the lifetime of the associated {@link Instrument}. */ CUMULATIVE } } From 230627d2dc00310edaf1b4e2283aa3bc15ee4f1e Mon Sep 17 00:00:00 2001 From: jwatson Date: Thu, 30 Jul 2020 10:14:00 -0700 Subject: [PATCH 13/37] re-order matching check and fix a merge issue --- .../java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java | 2 ++ .../main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java index c0d4f3e1b8a..dd2a5b6d8f6 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/MeterSdkProvider.java @@ -13,6 +13,8 @@ import io.opentelemetry.sdk.internal.MillisClock; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.export.MetricProducer; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; +import io.opentelemetry.sdk.metrics.view.InstrumentSelector; import io.opentelemetry.sdk.resources.Resource; import java.util.ArrayList; import java.util.Collection; 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 5ec0b85250d..d2e6f9aad80 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 @@ -72,8 +72,8 @@ private AggregationConfiguration findBestMatch(InstrumentDescriptor descriptor) for (Map.Entry entry : configuration.entrySet()) { InstrumentSelector registeredSelector = entry.getKey(); - if (matchesOnName(descriptor, registeredSelector) - && matchesOnType(descriptor, registeredSelector)) { + if (matchesOnType(descriptor, registeredSelector) + && matchesOnName(descriptor, registeredSelector)) { return entry.getValue(); } } From 36d6887c729f0fa745fdfc11532d96223a365968 Mon Sep 17 00:00:00 2001 From: jkwatson Date: Fri, 6 Nov 2020 08:57:17 -0800 Subject: [PATCH 14/37] Update from upstream changes. --- .../sdk/metrics/ViewRegistry.java | 6 +-- .../view/AggregationConfiguration.java | 36 +++++++++++++++ .../sdk/metrics/view/InstrumentSelector.java | 44 +++++++++++++++++++ 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java create mode 100644 sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java 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 d2e6f9aad80..cea10568fdf 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 @@ -66,7 +66,7 @@ Batcher createBatcher( throw new IllegalStateException("unsupported Temporality: " + specification.temporality()); } - // todo: consider moving this method to its own class, for more targetted testing. + // todo: consider moving this method to its own class, for more targeted testing. private AggregationConfiguration findBestMatch(InstrumentDescriptor descriptor) { for (Map.Entry entry : configuration.entrySet()) { @@ -103,9 +103,9 @@ private static AggregationConfiguration getDefaultSpecification(InstrumentDescri switch (descriptor.getType()) { case COUNTER: case UP_DOWN_COUNTER: - return Aggregations.sum(); + return CUMULATIVE_SUM; case VALUE_RECORDER: - return Aggregations.minMaxSumCount(); + return DELTA_SUMMARY; case VALUE_OBSERVER: case SUM_OBSERVER: case UP_DOWN_SUM_OBSERVER: diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java new file mode 100644 index 00000000000..f53dec0ccdd --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -0,0 +1,36 @@ +/* + * 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.api.metrics.Instrument; +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; + +@AutoValue +@Immutable +public abstract class AggregationConfiguration { + + public static AggregationConfiguration create(Aggregation aggregation, Temporality temporality) { + return new AutoValue_AggregationConfiguration(aggregation, temporality); + } + + /** Which {@link Aggregation} should be used for this View. */ + @Nullable + public abstract Aggregation aggregation(); + + /** What {@link Temporality} should be used for this View (delta vs. cumulative). */ + @Nullable + public abstract Temporality temporality(); + + /** An enumeration which describes the time period over which metrics should be aggregated. */ + public enum Temporality { + /** Metrics will be aggregated only over the most recent collection interval. */ + DELTA, + /** Metrics will be aggregated over the lifetime of the associated {@link Instrument}. */ + CUMULATIVE + } +} \ No newline at end of file diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java new file mode 100644 index 00000000000..12fc048ebcf --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package io.opentelemetry.sdk.metrics.view; + +import com.google.auto.value.AutoValue; +import com.google.auto.value.extension.memoized.Memoized; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import java.util.regex.Pattern; +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; + +@AutoValue +@Immutable +public abstract class InstrumentSelector { + + public static Builder newBuilder() { + return new AutoValue_InstrumentSelector.Builder(); + } + + @Nullable + public abstract InstrumentType instrumentType(); + + @Nullable + public abstract String instrumentNameRegex(); + + @Memoized + @Nullable + public Pattern instrumentNamePattern() { + return instrumentNameRegex() == null ? null : Pattern.compile(instrumentNameRegex()); + } + + @AutoValue.Builder + public interface Builder { + Builder instrumentType(InstrumentType instrumentType); + + Builder instrumentNameRegex(String regex); + + InstrumentSelector build(); + } +} \ No newline at end of file From b1bbf16f8d7407e1a4c08c4f21013cb8fbfd3a2d Mon Sep 17 00:00:00 2001 From: jkwatson Date: Fri, 6 Nov 2020 08:59:37 -0800 Subject: [PATCH 15/37] Update from upstream changes. --- .../sdk/metrics/view/AggregationConfiguration.java | 2 +- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index f53dec0ccdd..cc3d763f3fd 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -33,4 +33,4 @@ public enum Temporality { /** Metrics will be aggregated over the lifetime of the associated {@link Instrument}. */ CUMULATIVE } -} \ No newline at end of file +} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 12fc048ebcf..ca3a9a0fe8e 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ - package io.opentelemetry.sdk.metrics.view; import com.google.auto.value.AutoValue; @@ -41,4 +40,4 @@ public interface Builder { InstrumentSelector build(); } -} \ No newline at end of file +} From b7f2967926fffa12a13ffceadcd8eccade37ea20 Mon Sep 17 00:00:00 2001 From: jkwatson Date: Fri, 6 Nov 2020 09:40:30 -0800 Subject: [PATCH 16/37] Adjust defaults based on the latest behavior --- .../main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java | 3 +++ 1 file changed, 3 insertions(+) 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 cea10568fdf..62682f9950c 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 @@ -39,6 +39,8 @@ class ViewRegistry { AggregationConfiguration.create(Aggregations.minMaxSumCount(), Temporality.DELTA); private static final AggregationConfiguration CUMULATIVE_LAST_VALUE = AggregationConfiguration.create(Aggregations.lastValue(), Temporality.CUMULATIVE); + private static final AggregationConfiguration DELTA_LAST_VALUE = + AggregationConfiguration.create(Aggregations.lastValue(), Temporality.DELTA); private final Map configuration = new ConcurrentHashMap<>(); @@ -107,6 +109,7 @@ private static AggregationConfiguration getDefaultSpecification(InstrumentDescri case VALUE_RECORDER: return DELTA_SUMMARY; case VALUE_OBSERVER: + return DELTA_LAST_VALUE; case SUM_OBSERVER: case UP_DOWN_SUM_OBSERVER: return CUMULATIVE_LAST_VALUE; From 3a3c72f1206e6090dda691145d76f6d49992d284 Mon Sep 17 00:00:00 2001 From: jkwatson Date: Wed, 11 Nov 2020 10:58:47 -0800 Subject: [PATCH 17/37] refactor before writing tests --- .../sdk/metrics/AggregationChooser.java | 83 +++++++++++++++++++ .../sdk/metrics/ViewRegistry.java | 76 +++-------------- .../view/AggregationConfiguration.java | 3 - 3 files changed, 94 insertions(+), 68 deletions(-) create mode 100644 sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java new file mode 100644 index 00000000000..975e195fa10 --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java @@ -0,0 +1,83 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics; + +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; +import io.opentelemetry.sdk.metrics.view.Aggregations; +import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; + +class AggregationChooser { + private static final AggregationConfiguration CUMULATIVE_SUM = + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.CUMULATIVE); + private static final AggregationConfiguration DELTA_SUMMARY = + AggregationConfiguration.create( + Aggregations.minMaxSumCount(), AggregationConfiguration.Temporality.DELTA); + private static final AggregationConfiguration CUMULATIVE_LAST_VALUE = + AggregationConfiguration.create( + Aggregations.lastValue(), AggregationConfiguration.Temporality.CUMULATIVE); + private static final AggregationConfiguration DELTA_LAST_VALUE = + AggregationConfiguration.create( + Aggregations.lastValue(), AggregationConfiguration.Temporality.DELTA); + + private final Map configuration = + new ConcurrentHashMap<>(); + + private static boolean matchesOnType( + InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { + if (registeredSelector.instrumentType() == null) { + return true; + } + return registeredSelector.instrumentType().equals(descriptor.getType()); + } + + AggregationConfiguration chooseAggregation(InstrumentDescriptor descriptor) { + + for (Map.Entry entry : configuration.entrySet()) { + InstrumentSelector registeredSelector = entry.getKey(); + + if (matchesOnType(descriptor, registeredSelector) + && matchesOnName(descriptor, registeredSelector)) { + return entry.getValue(); + } + } + + // If none found, use the defaults: + return getDefaultSpecification(descriptor); + } + + private static boolean matchesOnName( + InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { + Pattern pattern = registeredSelector.instrumentNamePattern(); + if (pattern == null) { + return true; + } + return pattern.matcher(descriptor.getName()).matches(); + } + + private static AggregationConfiguration getDefaultSpecification(InstrumentDescriptor descriptor) { + switch (descriptor.getType()) { + case COUNTER: + case UP_DOWN_COUNTER: + return CUMULATIVE_SUM; + case VALUE_RECORDER: + return DELTA_SUMMARY; + case VALUE_OBSERVER: + return DELTA_LAST_VALUE; + case SUM_OBSERVER: + case UP_DOWN_SUM_OBSERVER: + return CUMULATIVE_LAST_VALUE; + } + throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); + } + + void addView(InstrumentSelector selector, AggregationConfiguration specification) { + configuration.put(selector, specification); + } +} 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 62682f9950c..0c4eb66dd63 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 @@ -8,11 +8,7 @@ import io.opentelemetry.sdk.metrics.view.Aggregation; import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; import io.opentelemetry.sdk.metrics.view.AggregationConfiguration.Temporality; -import io.opentelemetry.sdk.metrics.view.Aggregations; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.regex.Pattern; // notes: // specify by pieces of the descriptor. @@ -33,17 +29,16 @@ */ class ViewRegistry { - private static final AggregationConfiguration CUMULATIVE_SUM = - AggregationConfiguration.create(Aggregations.sum(), Temporality.CUMULATIVE); - private static final AggregationConfiguration DELTA_SUMMARY = - AggregationConfiguration.create(Aggregations.minMaxSumCount(), Temporality.DELTA); - private static final AggregationConfiguration CUMULATIVE_LAST_VALUE = - AggregationConfiguration.create(Aggregations.lastValue(), Temporality.CUMULATIVE); - private static final AggregationConfiguration DELTA_LAST_VALUE = - AggregationConfiguration.create(Aggregations.lastValue(), Temporality.DELTA); + private final AggregationChooser aggregationChooser; - private final Map configuration = - new ConcurrentHashMap<>(); + ViewRegistry() { + this(new AggregationChooser()); + } + + // VisibleForTesting + ViewRegistry(AggregationChooser aggregationChooser) { + this.aggregationChooser = aggregationChooser; + } /** * Create a new {@link io.opentelemetry.sdk.metrics.Batcher} for use in metric recording @@ -54,7 +49,7 @@ Batcher createBatcher( MeterSharedState meterSharedState, InstrumentDescriptor descriptor) { - AggregationConfiguration specification = findBestMatch(descriptor); + AggregationConfiguration specification = aggregationChooser.chooseAggregation(descriptor); Aggregation aggregation = specification.aggregation(); @@ -68,56 +63,7 @@ Batcher createBatcher( throw new IllegalStateException("unsupported Temporality: " + specification.temporality()); } - // todo: consider moving this method to its own class, for more targeted testing. - private AggregationConfiguration findBestMatch(InstrumentDescriptor descriptor) { - - for (Map.Entry entry : configuration.entrySet()) { - InstrumentSelector registeredSelector = entry.getKey(); - - if (matchesOnType(descriptor, registeredSelector) - && matchesOnName(descriptor, registeredSelector)) { - return entry.getValue(); - } - } - - // If none found, use the defaults: - return getDefaultSpecification(descriptor); - } - - private static boolean matchesOnType( - InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { - if (registeredSelector.instrumentType() == null) { - return true; - } - return registeredSelector.instrumentType().equals(descriptor.getType()); - } - - private static boolean matchesOnName( - InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { - Pattern pattern = registeredSelector.instrumentNamePattern(); - if (pattern == null) { - return true; - } - return pattern.matcher(descriptor.getName()).matches(); - } - - private static AggregationConfiguration getDefaultSpecification(InstrumentDescriptor descriptor) { - switch (descriptor.getType()) { - case COUNTER: - case UP_DOWN_COUNTER: - return CUMULATIVE_SUM; - case VALUE_RECORDER: - return DELTA_SUMMARY; - case VALUE_OBSERVER: - return DELTA_LAST_VALUE; - case SUM_OBSERVER: - case UP_DOWN_SUM_OBSERVER: - return CUMULATIVE_LAST_VALUE; - } - throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType()); - } - void registerView(InstrumentSelector selector, AggregationConfiguration specification) { - configuration.put(selector, specification); + aggregationChooser.addView(selector, specification); } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index cc3d763f3fd..37bd952209e 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -7,7 +7,6 @@ import com.google.auto.value.AutoValue; import io.opentelemetry.api.metrics.Instrument; -import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @AutoValue @@ -19,11 +18,9 @@ public static AggregationConfiguration create(Aggregation aggregation, Temporali } /** Which {@link Aggregation} should be used for this View. */ - @Nullable public abstract Aggregation aggregation(); /** What {@link Temporality} should be used for this View (delta vs. cumulative). */ - @Nullable public abstract Temporality temporality(); /** An enumeration which describes the time period over which metrics should be aggregated. */ From 96785319061fca577e46a026025eecb9e145bcec Mon Sep 17 00:00:00 2001 From: jkwatson Date: Wed, 11 Nov 2020 16:09:56 -0800 Subject: [PATCH 18/37] tests for the AggregationChooser and a bugfix they uncovered --- .../sdk/metrics/AggregationChooser.java | 47 +++-- .../sdk/metrics/view/Aggregations.java | 25 +++ .../sdk/metrics/view/InstrumentSelector.java | 11 +- .../sdk/metrics/AggregationChooserTest.java | 180 ++++++++++++++++++ 4 files changed, 248 insertions(+), 15 deletions(-) create mode 100644 sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationChooserTest.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java index 975e195fa10..de2d7e0a7c7 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java @@ -8,6 +8,8 @@ import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; import io.opentelemetry.sdk.metrics.view.Aggregations; import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; @@ -29,34 +31,53 @@ class AggregationChooser { private final Map configuration = new ConcurrentHashMap<>(); - private static boolean matchesOnType( - InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { - if (registeredSelector.instrumentType() == null) { - return true; - } - return registeredSelector.instrumentType().equals(descriptor.getType()); - } - AggregationConfiguration chooseAggregation(InstrumentDescriptor descriptor) { - + List> possibleMatches = + new ArrayList<>(); for (Map.Entry entry : configuration.entrySet()) { InstrumentSelector registeredSelector = entry.getKey(); - + // if it matches everything, return it right away... if (matchesOnType(descriptor, registeredSelector) && matchesOnName(descriptor, registeredSelector)) { return entry.getValue(); } + // otherwise throw it into a bucket of possible matches if it matches one of the criteria + if (matchesOne(descriptor, registeredSelector)) { + possibleMatches.add(entry); + } + } + + if (possibleMatches.isEmpty()) { + return getDefaultSpecification(descriptor); } - // If none found, use the defaults: - return getDefaultSpecification(descriptor); + // If no exact matches found, pick the first one that matches something: + return possibleMatches.get(0).getValue(); + } + + private boolean matchesOne(InstrumentDescriptor descriptor, InstrumentSelector selector) { + if (selector.hasNameRegex() && !matchesOnName(descriptor, selector)) { + return false; + } + if (selector.hasType() && !matchesOnType(descriptor, selector)) { + return false; + } + return true; + } + + private static boolean matchesOnType( + InstrumentDescriptor descriptor, InstrumentSelector selector) { + if (selector.instrumentType() == null) { + return false; + } + return selector.instrumentType().equals(descriptor.getType()); } private static boolean matchesOnName( InstrumentDescriptor descriptor, InstrumentSelector registeredSelector) { Pattern pattern = registeredSelector.instrumentNamePattern(); if (pattern == null) { - return true; + return false; } return pattern.matcher(descriptor.getName()).matches(); } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/Aggregations.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/Aggregations.java index 9b27d9d5f6b..4bfb48916aa 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/Aggregations.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/Aggregations.java @@ -99,6 +99,11 @@ public boolean availableForInstrument(InstrumentType instrumentType) { return instrumentType == InstrumentType.VALUE_OBSERVER || instrumentType == InstrumentType.VALUE_RECORDER; } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } @Immutable @@ -142,6 +147,11 @@ public boolean availableForInstrument(InstrumentType instrumentType) { // Available for all instruments. return true; } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } @Immutable @@ -170,6 +180,11 @@ public boolean availableForInstrument(InstrumentType instrumentType) { // Available for all instruments. return true; } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } @Immutable @@ -201,6 +216,11 @@ public String getUnit(String initialUnit) { public boolean availableForInstrument(InstrumentType instrumentType) { throw new UnsupportedOperationException("Implement this"); } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } @Immutable @@ -248,6 +268,11 @@ public boolean availableForInstrument(InstrumentType instrumentType) { return instrumentType == InstrumentType.SUM_OBSERVER || instrumentType == InstrumentType.UP_DOWN_SUM_OBSERVER; } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } private Aggregations() {} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index ca3a9a0fe8e..37bf0b4ec12 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -15,7 +15,6 @@ @AutoValue @Immutable public abstract class InstrumentSelector { - public static Builder newBuilder() { return new AutoValue_InstrumentSelector.Builder(); } @@ -26,12 +25,20 @@ public static Builder newBuilder() { @Nullable public abstract String instrumentNameRegex(); - @Memoized @Nullable + @Memoized public Pattern instrumentNamePattern() { return instrumentNameRegex() == null ? null : Pattern.compile(instrumentNameRegex()); } + public boolean hasType() { + return instrumentType() != null; + } + + public boolean hasNameRegex() { + return instrumentNameRegex() != null; + } + @AutoValue.Builder public interface Builder { Builder instrumentType(InstrumentType instrumentType); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationChooserTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationChooserTest.java new file mode 100644 index 00000000000..b51406135a3 --- /dev/null +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AggregationChooserTest.java @@ -0,0 +1,180 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; +import io.opentelemetry.sdk.metrics.view.Aggregations; +import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import org.junit.jupiter.api.Test; + +class AggregationChooserTest { + + @Test + void selection_onType() { + AggregationConfiguration configuration = + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.DELTA); + + AggregationChooser aggregationChooser = new AggregationChooser(); + aggregationChooser.addView( + InstrumentSelector.newBuilder().instrumentType(InstrumentType.COUNTER).build(), + configuration); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo(configuration); + // this one hasn't been configured, so it gets the default still.. + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.CUMULATIVE)); + } + + @Test + void selection_onName() { + AggregationConfiguration configuration = + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.DELTA); + + AggregationChooser aggregationChooser = new AggregationChooser(); + aggregationChooser.addView( + InstrumentSelector.newBuilder().instrumentNameRegex("overridden").build(), configuration); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo(configuration); + // this one hasn't been configured, so it gets the default still.. + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "default", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.CUMULATIVE)); + } + + @Test + void selection_moreSpecificWins() { + AggregationConfiguration configuration1 = + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.DELTA); + AggregationConfiguration configuration2 = + AggregationConfiguration.create( + Aggregations.count(), AggregationConfiguration.Temporality.DELTA); + + AggregationChooser aggregationChooser = new AggregationChooser(); + aggregationChooser.addView( + InstrumentSelector.newBuilder() + .instrumentNameRegex("overridden") + .instrumentType(InstrumentType.COUNTER) + .build(), + configuration2); + aggregationChooser.addView( + InstrumentSelector.newBuilder().instrumentType(InstrumentType.COUNTER).build(), + configuration1); + + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo(configuration2); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "default", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo(configuration1); + } + + @Test + void selection_regex() { + AggregationConfiguration configuration1 = + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.DELTA); + + AggregationChooser aggregationChooser = new AggregationChooser(); + aggregationChooser.addView( + InstrumentSelector.newBuilder() + .instrumentNameRegex("overrid(es|den)") + .instrumentType(InstrumentType.COUNTER) + .build(), + configuration1); + + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo(configuration1); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "overrides", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo(configuration1); + // this one hasn't been configured, so it gets the default still.. + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "default", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.CUMULATIVE)); + } + + @Test + void defaults() { + AggregationChooser aggregationChooser = new AggregationChooser(); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.CUMULATIVE)); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.sum(), AggregationConfiguration.Temporality.CUMULATIVE)); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.VALUE_RECORDER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.minMaxSumCount(), AggregationConfiguration.Temporality.DELTA)); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.SUM_OBSERVER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.lastValue(), AggregationConfiguration.Temporality.CUMULATIVE)); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.VALUE_OBSERVER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.lastValue(), AggregationConfiguration.Temporality.DELTA)); + assertThat( + aggregationChooser.chooseAggregation( + InstrumentDescriptor.create( + "", "", "", InstrumentType.UP_DOWN_SUM_OBSERVER, InstrumentValueType.LONG))) + .isEqualTo( + AggregationConfiguration.create( + Aggregations.lastValue(), AggregationConfiguration.Temporality.CUMULATIVE)); + } +} From 77f5c7fb42b8e2802a3e2c0382aefda452a3d083 Mon Sep 17 00:00:00 2001 From: jkwatson Date: Thu, 12 Nov 2020 10:44:14 -0800 Subject: [PATCH 19/37] tests for the ViewRegistry --- .../sdk/metrics/AggregationChooser.java | 2 +- .../opentelemetry/sdk/metrics/Batchers.java | 60 +++++++++++ .../sdk/metrics/ViewRegistry.java | 8 +- .../sdk/metrics/ViewRegistryTest.java | 102 ++++++++++++++++++ 4 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java index de2d7e0a7c7..98540f80107 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java @@ -55,7 +55,7 @@ && matchesOnName(descriptor, registeredSelector)) { return possibleMatches.get(0).getValue(); } - private boolean matchesOne(InstrumentDescriptor descriptor, InstrumentSelector selector) { + private static boolean matchesOne(InstrumentDescriptor descriptor, InstrumentSelector selector) { if (selector.hasNameRegex() && !matchesOnName(descriptor, selector)) { return false; } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java index 180531b4a43..ce62008d687 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java @@ -165,6 +165,66 @@ public final List completeCollectionCycle() { public boolean generatesDeltas() { return delta; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + AllLabels allLabels = (AllLabels) o; + + if (startEpochNanos != allLabels.startEpochNanos) { + return false; + } + if (delta != allLabels.delta) { + return false; + } + if (descriptor != null ? !descriptor.equals(allLabels.descriptor) + : allLabels.descriptor != null) { + return false; + } + if (aggregation != null ? !aggregation.equals(allLabels.aggregation) + : allLabels.aggregation != null) { + return false; + } + if (resource != null ? !resource.equals(allLabels.resource) : allLabels.resource != null) { + return false; + } + if (instrumentationLibraryInfo != null ? !instrumentationLibraryInfo + .equals(allLabels.instrumentationLibraryInfo) + : allLabels.instrumentationLibraryInfo != null) { + return false; + } + if (clock != null ? !clock.equals(allLabels.clock) : allLabels.clock != null) { + return false; + } + if (aggregatorFactory != null ? !aggregatorFactory.equals(allLabels.aggregatorFactory) + : allLabels.aggregatorFactory != null) { + return false; + } + return aggregatorMap != null ? aggregatorMap.equals(allLabels.aggregatorMap) + : allLabels.aggregatorMap == null; + } + + @Override + public int hashCode() { + int result = descriptor != null ? descriptor.hashCode() : 0; + result = 31 * result + (aggregation != null ? aggregation.hashCode() : 0); + result = 31 * result + (resource != null ? resource.hashCode() : 0); + result = + 31 * result + (instrumentationLibraryInfo != null ? instrumentationLibraryInfo.hashCode() + : 0); + result = 31 * result + (clock != null ? clock.hashCode() : 0); + result = 31 * result + (aggregatorFactory != null ? aggregatorFactory.hashCode() : 0); + result = 31 * result + (aggregatorMap != null ? aggregatorMap.hashCode() : 0); + result = 31 * result + (int) (startEpochNanos ^ (startEpochNanos >>> 32)); + result = 31 * result + (delta ? 1 : 0); + return result; + } } private Batchers() {} 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 0c4eb66dd63..d44112ea585 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 @@ -40,6 +40,10 @@ class ViewRegistry { this.aggregationChooser = aggregationChooser; } + void registerView(InstrumentSelector selector, AggregationConfiguration specification) { + aggregationChooser.addView(selector, specification); + } + /** * Create a new {@link io.opentelemetry.sdk.metrics.Batcher} for use in metric recording * aggregation. @@ -62,8 +66,4 @@ Batcher createBatcher( } throw new IllegalStateException("unsupported Temporality: " + specification.temporality()); } - - void registerView(InstrumentSelector selector, AggregationConfiguration specification) { - aggregationChooser.addView(selector, specification); - } } 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 new file mode 100644 index 00000000000..f206b3915a4 --- /dev/null +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ViewRegistryTest.java @@ -0,0 +1,102 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.internal.TestClock; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.AggregationConfiguration; +import io.opentelemetry.sdk.metrics.view.Aggregations; +import io.opentelemetry.sdk.metrics.view.InstrumentSelector; +import io.opentelemetry.sdk.resources.Resource; +import org.junit.jupiter.api.Test; + +class ViewRegistryTest { + + @Test + void registerView() { + AggregationChooser chooser = mock(AggregationChooser.class); + + ViewRegistry viewRegistry = new ViewRegistry(chooser); + InstrumentSelector selector = + InstrumentSelector.newBuilder().instrumentType(InstrumentType.COUNTER).build(); + AggregationConfiguration specification = + AggregationConfiguration.create( + Aggregations.count(), AggregationConfiguration.Temporality.CUMULATIVE); + + viewRegistry.registerView(selector, specification); + + verify(chooser).addView(selector, specification); + } + + @Test + void createBatcher_cumulative() { + AggregationChooser chooser = mock(AggregationChooser.class); + + ViewRegistry viewRegistry = new ViewRegistry(chooser); + + InstrumentDescriptor descriptor = InstrumentDescriptor.create("name", "description", "unit", + InstrumentType.COUNTER, InstrumentValueType.DOUBLE); + MeterProviderSharedState providerSharedState = MeterProviderSharedState.create( + TestClock.create(), + Resource.getEmpty()); + MeterSharedState meterSharedState = MeterSharedState + .create(InstrumentationLibraryInfo.create("test", "1.0")); + + AggregationConfiguration specification = + AggregationConfiguration.create( + Aggregations.count(), AggregationConfiguration.Temporality.CUMULATIVE); + Batcher expectedBatcher = Batchers + .getCumulativeAllLabels(descriptor, providerSharedState, meterSharedState, + Aggregations.count()); + + when(chooser.chooseAggregation(descriptor)).thenReturn(specification); + + Batcher result = viewRegistry.createBatcher(providerSharedState, meterSharedState, descriptor); + + assertThat(result.generatesDeltas()).isFalse(); + assertThat(result).isEqualTo(expectedBatcher); + + assertThat(result).isNotNull(); + } + + @Test + void createBatcher_delta() { + AggregationChooser chooser = mock(AggregationChooser.class); + + ViewRegistry viewRegistry = new ViewRegistry(chooser); + + InstrumentDescriptor descriptor = InstrumentDescriptor.create("name", "description", "unit", + InstrumentType.COUNTER, InstrumentValueType.DOUBLE); + MeterProviderSharedState providerSharedState = MeterProviderSharedState.create( + TestClock.create(), + Resource.getEmpty()); + MeterSharedState meterSharedState = MeterSharedState + .create(InstrumentationLibraryInfo.create("test", "1.0")); + + AggregationConfiguration specification = + AggregationConfiguration.create( + Aggregations.count(), AggregationConfiguration.Temporality.DELTA); + Batcher expectedBatcher = Batchers + .getDeltaAllLabels(descriptor, providerSharedState, meterSharedState, + Aggregations.count()); + + when(chooser.chooseAggregation(descriptor)).thenReturn(specification); + + Batcher result = viewRegistry.createBatcher(providerSharedState, meterSharedState, descriptor); + + assertThat(result.generatesDeltas()).isTrue(); + assertThat(result).isEqualTo(expectedBatcher); + + assertThat(result).isNotNull(); + } +} From c85e464ad48c9f85ce608c0fcc7f12de7e857586 Mon Sep 17 00:00:00 2001 From: jkwatson Date: Thu, 12 Nov 2020 10:51:05 -0800 Subject: [PATCH 20/37] Javadoc for the AggregationConfiguration --- .../opentelemetry/sdk/metrics/Batchers.java | 20 ++++++---- .../view/AggregationConfiguration.java | 7 ++++ .../sdk/metrics/ViewRegistryTest.java | 40 +++++++++---------- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java index ce62008d687..dc95c2a04b5 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java @@ -183,30 +183,34 @@ public boolean equals(Object o) { if (delta != allLabels.delta) { return false; } - if (descriptor != null ? !descriptor.equals(allLabels.descriptor) + if (descriptor != null + ? !descriptor.equals(allLabels.descriptor) : allLabels.descriptor != null) { return false; } - if (aggregation != null ? !aggregation.equals(allLabels.aggregation) + if (aggregation != null + ? !aggregation.equals(allLabels.aggregation) : allLabels.aggregation != null) { return false; } if (resource != null ? !resource.equals(allLabels.resource) : allLabels.resource != null) { return false; } - if (instrumentationLibraryInfo != null ? !instrumentationLibraryInfo - .equals(allLabels.instrumentationLibraryInfo) + if (instrumentationLibraryInfo != null + ? !instrumentationLibraryInfo.equals(allLabels.instrumentationLibraryInfo) : allLabels.instrumentationLibraryInfo != null) { return false; } if (clock != null ? !clock.equals(allLabels.clock) : allLabels.clock != null) { return false; } - if (aggregatorFactory != null ? !aggregatorFactory.equals(allLabels.aggregatorFactory) + if (aggregatorFactory != null + ? !aggregatorFactory.equals(allLabels.aggregatorFactory) : allLabels.aggregatorFactory != null) { return false; } - return aggregatorMap != null ? aggregatorMap.equals(allLabels.aggregatorMap) + return aggregatorMap != null + ? aggregatorMap.equals(allLabels.aggregatorMap) : allLabels.aggregatorMap == null; } @@ -216,8 +220,8 @@ public int hashCode() { result = 31 * result + (aggregation != null ? aggregation.hashCode() : 0); result = 31 * result + (resource != null ? resource.hashCode() : 0); result = - 31 * result + (instrumentationLibraryInfo != null ? instrumentationLibraryInfo.hashCode() - : 0); + 31 * result + + (instrumentationLibraryInfo != null ? instrumentationLibraryInfo.hashCode() : 0); result = 31 * result + (clock != null ? clock.hashCode() : 0); result = 31 * result + (aggregatorFactory != null ? aggregatorFactory.hashCode() : 0); result = 31 * result + (aggregatorMap != null ? aggregatorMap.hashCode() : 0); diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index 37bd952209e..8f4647b3c54 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -9,6 +9,13 @@ import io.opentelemetry.api.metrics.Instrument; import javax.annotation.concurrent.Immutable; +/** + * An AggregationConfiguration describes how an aggregation should be performed. It includes both an + * {@link Aggregation} which implements what shape of aggregation is created (i.e. histogram, sum, + * minMaxSumCount, etc), and a {@link AggregationConfiguration.Temporality} which describes whether + * aggregations should be reset with every collection interval, or continue to accumulate across + * collection intervals. + */ @AutoValue @Immutable public abstract class AggregationConfiguration { 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 f206b3915a4..a649df8ab7c 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 @@ -44,20 +44,20 @@ void createBatcher_cumulative() { ViewRegistry viewRegistry = new ViewRegistry(chooser); - InstrumentDescriptor descriptor = InstrumentDescriptor.create("name", "description", "unit", - InstrumentType.COUNTER, InstrumentValueType.DOUBLE); - MeterProviderSharedState providerSharedState = MeterProviderSharedState.create( - TestClock.create(), - Resource.getEmpty()); - MeterSharedState meterSharedState = MeterSharedState - .create(InstrumentationLibraryInfo.create("test", "1.0")); + InstrumentDescriptor descriptor = + InstrumentDescriptor.create( + "name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.DOUBLE); + MeterProviderSharedState providerSharedState = + MeterProviderSharedState.create(TestClock.create(), Resource.getEmpty()); + MeterSharedState meterSharedState = + MeterSharedState.create(InstrumentationLibraryInfo.create("test", "1.0")); AggregationConfiguration specification = AggregationConfiguration.create( Aggregations.count(), AggregationConfiguration.Temporality.CUMULATIVE); - Batcher expectedBatcher = Batchers - .getCumulativeAllLabels(descriptor, providerSharedState, meterSharedState, - Aggregations.count()); + Batcher expectedBatcher = + Batchers.getCumulativeAllLabels( + descriptor, providerSharedState, meterSharedState, Aggregations.count()); when(chooser.chooseAggregation(descriptor)).thenReturn(specification); @@ -75,20 +75,20 @@ void createBatcher_delta() { ViewRegistry viewRegistry = new ViewRegistry(chooser); - InstrumentDescriptor descriptor = InstrumentDescriptor.create("name", "description", "unit", - InstrumentType.COUNTER, InstrumentValueType.DOUBLE); - MeterProviderSharedState providerSharedState = MeterProviderSharedState.create( - TestClock.create(), - Resource.getEmpty()); - MeterSharedState meterSharedState = MeterSharedState - .create(InstrumentationLibraryInfo.create("test", "1.0")); + InstrumentDescriptor descriptor = + InstrumentDescriptor.create( + "name", "description", "unit", InstrumentType.COUNTER, InstrumentValueType.DOUBLE); + MeterProviderSharedState providerSharedState = + MeterProviderSharedState.create(TestClock.create(), Resource.getEmpty()); + MeterSharedState meterSharedState = + MeterSharedState.create(InstrumentationLibraryInfo.create("test", "1.0")); AggregationConfiguration specification = AggregationConfiguration.create( Aggregations.count(), AggregationConfiguration.Temporality.DELTA); - Batcher expectedBatcher = Batchers - .getDeltaAllLabels(descriptor, providerSharedState, meterSharedState, - Aggregations.count()); + Batcher expectedBatcher = + Batchers.getDeltaAllLabels( + descriptor, providerSharedState, meterSharedState, Aggregations.count()); when(chooser.chooseAggregation(descriptor)).thenReturn(specification); From 778a8a318a0d2aaf2f80e5c2301a6948f6abf4e9 Mon Sep 17 00:00:00 2001 From: jkwatson Date: Thu, 12 Nov 2020 11:08:01 -0800 Subject: [PATCH 21/37] Add more javadoc. --- .../sdk/metrics/AggregationChooser.java | 4 +-- .../view/AggregationConfiguration.java | 1 + .../sdk/metrics/view/InstrumentSelector.java | 27 +++++++++++++++++-- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java index 98540f80107..74ad5cf324d 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AggregationChooser.java @@ -56,10 +56,10 @@ && matchesOnName(descriptor, registeredSelector)) { } private static boolean matchesOne(InstrumentDescriptor descriptor, InstrumentSelector selector) { - if (selector.hasNameRegex() && !matchesOnName(descriptor, selector)) { + if (selector.hasInstrumentNameRegex() && !matchesOnName(descriptor, selector)) { return false; } - if (selector.hasType() && !matchesOnType(descriptor, selector)) { + if (selector.hasInstrumentType() && !matchesOnType(descriptor, selector)) { return false; } return true; diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index 8f4647b3c54..4b6328ac37e 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -20,6 +20,7 @@ @Immutable public abstract class AggregationConfiguration { + /** Create a new configuration with the provided options. */ public static AggregationConfiguration create(Aggregation aggregation, Temporality temporality) { return new AutoValue_AggregationConfiguration(aggregation, temporality); } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 37bf0b4ec12..32b6b97ec53 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -12,6 +12,12 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; +/** + * Provides means for selecting one ore more {@link io.opentelemetry.api.metrics.Instrument}s. Used + * for configuring a aggregations for the specified instruments. + * + *

There are two options for selecting instruments: by instrument name and by instrument type. + */ @AutoValue @Immutable public abstract class InstrumentSelector { @@ -19,32 +25,49 @@ public static Builder newBuilder() { return new AutoValue_InstrumentSelector.Builder(); } + /** + * What {@link InstrumentType} should be selected. If null, then this specifier will not be used. + */ @Nullable public abstract InstrumentType instrumentType(); + /** + * What instrument names should be selected. This is a regex. If null, then this specifier will + * not be used. + */ @Nullable public abstract String instrumentNameRegex(); + /** + * The {@link Pattern} generated by the provided {@link #instrumentNameRegex()}, or null if none + * was specified. + */ @Nullable @Memoized public Pattern instrumentNamePattern() { return instrumentNameRegex() == null ? null : Pattern.compile(instrumentNameRegex()); } - public boolean hasType() { + /** Whether the InstrumentType been specified. */ + public boolean hasInstrumentType() { return instrumentType() != null; } - public boolean hasNameRegex() { + /** Whether the instrument name regex been specified. */ + public boolean hasInstrumentNameRegex() { return instrumentNameRegex() != null; } + /** Builder for {@link InstrumentSelector} instances. */ @AutoValue.Builder public interface Builder { + /** Provide a specifier for {@link InstrumentType}. */ Builder instrumentType(InstrumentType instrumentType); + /** Provide a specifier for selecting Instruments by name. */ Builder instrumentNameRegex(String regex); + /** Create an InstrumentSelector instance. */ InstrumentSelector build(); } } From 567c16e1c3cdc7f32a814e7b1f5d9e1482245cd6 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:41:06 -0800 Subject: [PATCH 22/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java Co-authored-by: Anuraag Agrawal --- .../src/main/java/io/opentelemetry/sdk/metrics/Batcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java index 4f254535b6d..6ebca0771d3 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java @@ -52,6 +52,6 @@ interface Batcher { */ List completeCollectionCycle(); - /** Does this batcher generate "delta" style metrics. The alternative is "cumulative". */ + /** Returns whether this batcher generate "delta" style metrics. The alternative is "cumulative". */ boolean generatesDeltas(); } From 79a29fda734a8321d2a294d8214223f10a13d4fc Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:41:15 -0800 Subject: [PATCH 23/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java Co-authored-by: Anuraag Agrawal --- .../sdk/metrics/view/AggregationConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index 4b6328ac37e..cb8ad344a6a 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -20,7 +20,7 @@ @Immutable public abstract class AggregationConfiguration { - /** Create a new configuration with the provided options. */ + /** Returns a new configuration with the provided options. */ public static AggregationConfiguration create(Aggregation aggregation, Temporality temporality) { return new AutoValue_AggregationConfiguration(aggregation, temporality); } From 25d546478f4f1edd7c8b673d68cd524f98b489e0 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:41:26 -0800 Subject: [PATCH 24/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java Co-authored-by: Anuraag Agrawal --- .../sdk/metrics/view/AggregationConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index cb8ad344a6a..d61fa60992d 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -28,7 +28,7 @@ public static AggregationConfiguration create(Aggregation aggregation, Temporali /** Which {@link Aggregation} should be used for this View. */ public abstract Aggregation aggregation(); - /** What {@link Temporality} should be used for this View (delta vs. cumulative). */ + /** Returns the {@link Temporality} that should be used for this View (delta vs. cumulative). */ public abstract Temporality temporality(); /** An enumeration which describes the time period over which metrics should be aggregated. */ From 55537f01beb9e627d7dc48b2e5e4cfd7a41844f3 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:41:35 -0800 Subject: [PATCH 25/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 32b6b97ec53..838b8c80eaa 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -64,7 +64,7 @@ public interface Builder { /** Provide a specifier for {@link InstrumentType}. */ Builder instrumentType(InstrumentType instrumentType); - /** Provide a specifier for selecting Instruments by name. */ + /** Sets a specifier for selecting Instruments by name. */ Builder instrumentNameRegex(String regex); /** Create an InstrumentSelector instance. */ From 02a83241f2ad995d702b7c4dffe78646b87a6fbf Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:41:47 -0800 Subject: [PATCH 26/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 838b8c80eaa..576ad9ba770 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -67,7 +67,7 @@ public interface Builder { /** Sets a specifier for selecting Instruments by name. */ Builder instrumentNameRegex(String regex); - /** Create an InstrumentSelector instance. */ + /** Returns an InstrumentSelector instance with the content of this builder. */ InstrumentSelector build(); } } From c7e4b59be6730eebbc068902436d494fb84d1b1b Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:41:58 -0800 Subject: [PATCH 27/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java Co-authored-by: Anuraag Agrawal --- .../sdk/metrics/view/AggregationConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index b1f35be59b9..9d1e213cb1b 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -29,7 +29,7 @@ public static AggregationConfiguration create(Aggregation aggregation, Temporali return new AutoValue_AggregationConfiguration(aggregation, temporality); } - /** Which {@link Aggregation} should be used for this View. */ + /** Returns the {@link Aggregation} that should be used for this View. */ @Nullable public abstract Aggregation aggregation(); From fd42ee340fc971fda17b8bd2e7fcd78a05cc49ff Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:42:10 -0800 Subject: [PATCH 28/37] Update sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java Co-authored-by: Anuraag Agrawal --- .../sdk/metrics/view/AggregationConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index 9d1e213cb1b..b75043bfb81 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -33,7 +33,7 @@ public static AggregationConfiguration create(Aggregation aggregation, Temporali @Nullable public abstract Aggregation aggregation(); - /** What {@link Temporality} should be used for this View (delta vs. cumulative). */ + /** Returns the {@link Temporality} that should be used for this View (delta vs. cumulative). */ @Nullable public abstract Temporality temporality(); From 28b763619ef4321bc47328ef9bba3bbe2414f8d8 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:42:27 -0800 Subject: [PATCH 29/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java Co-authored-by: Anuraag Agrawal --- .../sdk/metrics/view/AggregationConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java index d61fa60992d..e67a6bdc8b5 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/AggregationConfiguration.java @@ -25,7 +25,7 @@ public static AggregationConfiguration create(Aggregation aggregation, Temporali return new AutoValue_AggregationConfiguration(aggregation, temporality); } - /** Which {@link Aggregation} should be used for this View. */ + /** Returns the {@link Aggregation} that should be used for this View. */ public abstract Aggregation aggregation(); /** Returns the {@link Temporality} that should be used for this View (delta vs. cumulative). */ From 444c3b9ddf35920ee98c1a656486fe54a8f150a0 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:42:43 -0800 Subject: [PATCH 30/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 576ad9ba770..58745c1ee57 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -26,7 +26,7 @@ public static Builder newBuilder() { } /** - * What {@link InstrumentType} should be selected. If null, then this specifier will not be used. + * Returns {@link InstrumentType} that should be selected. If null, then this specifier will not be used. */ @Nullable public abstract InstrumentType instrumentType(); From 5e94bc6b0a45c3b2c1d198b5fa4feb7778df2c80 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:42:57 -0800 Subject: [PATCH 31/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 58745c1ee57..302bb2fcd2f 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -32,7 +32,7 @@ public static Builder newBuilder() { public abstract InstrumentType instrumentType(); /** - * What instrument names should be selected. This is a regex. If null, then this specifier will + * Returns which instrument names should be selected. This is a regex. If null, then this specifier will * not be used. */ @Nullable From 41180b2d040688e1be9756f8d1a6e9e7d51f7fd0 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:43:12 -0800 Subject: [PATCH 32/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 302bb2fcd2f..d9ef5b10a51 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -39,7 +39,7 @@ public static Builder newBuilder() { public abstract String instrumentNameRegex(); /** - * The {@link Pattern} generated by the provided {@link #instrumentNameRegex()}, or null if none + * Returns the {@link Pattern} generated by the provided {@link #instrumentNameRegex()}, or null if none * was specified. */ @Nullable From bd60c114277c2a4068e9aea2aa82a4bda1f70de6 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:43:47 -0800 Subject: [PATCH 33/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index d9ef5b10a51..e5cfb11147c 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -48,7 +48,7 @@ public Pattern instrumentNamePattern() { return instrumentNameRegex() == null ? null : Pattern.compile(instrumentNameRegex()); } - /** Whether the InstrumentType been specified. */ + /** Returns whether the InstrumentType been specified. */ public boolean hasInstrumentType() { return instrumentType() != null; } From d99012d9a6aaa41484ba7d5cda320d1572eaabc6 Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:44:06 -0800 Subject: [PATCH 34/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index e5cfb11147c..1bd0c6f5623 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -53,7 +53,7 @@ public boolean hasInstrumentType() { return instrumentType() != null; } - /** Whether the instrument name regex been specified. */ + /** Returns whether the instrument name regex been specified. */ public boolean hasInstrumentNameRegex() { return instrumentNameRegex() != null; } From 5d783b324286cd972a8229b25b0cd6b017e9de2c Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 16:44:43 -0800 Subject: [PATCH 35/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java Co-authored-by: Anuraag Agrawal --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index 1bd0c6f5623..cf9d9c39d62 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -61,7 +61,7 @@ public boolean hasInstrumentNameRegex() { /** Builder for {@link InstrumentSelector} instances. */ @AutoValue.Builder public interface Builder { - /** Provide a specifier for {@link InstrumentType}. */ + /** Sets a specifier for {@link InstrumentType}. */ Builder instrumentType(InstrumentType instrumentType); /** Sets a specifier for selecting Instruments by name. */ From 816c35cfa9217f02ede1e2630f1c30358dffcd2f Mon Sep 17 00:00:00 2001 From: jkwatson Date: Thu, 12 Nov 2020 16:53:39 -0800 Subject: [PATCH 36/37] fix formatting issues --- .../java/io/opentelemetry/sdk/metrics/Batcher.java | 4 +++- .../sdk/metrics/view/InstrumentSelector.java | 11 ++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java index 6ebca0771d3..99ecae4dada 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java @@ -52,6 +52,8 @@ interface Batcher { */ List completeCollectionCycle(); - /** Returns whether this batcher generate "delta" style metrics. The alternative is "cumulative". */ + /** + * Returns whether this batcher generate "delta" style metrics. The alternative is "cumulative". + */ boolean generatesDeltas(); } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index cf9d9c39d62..e257f24fce2 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -26,21 +26,22 @@ public static Builder newBuilder() { } /** - * Returns {@link InstrumentType} that should be selected. If null, then this specifier will not be used. + * Returns {@link InstrumentType} that should be selected. If null, then this specifier will not + * be used. */ @Nullable public abstract InstrumentType instrumentType(); /** - * Returns which instrument names should be selected. This is a regex. If null, then this specifier will - * not be used. + * Returns which instrument names should be selected. This is a regex. If null, then this + * specifier will not be used. */ @Nullable public abstract String instrumentNameRegex(); /** - * Returns the {@link Pattern} generated by the provided {@link #instrumentNameRegex()}, or null if none - * was specified. + * Returns the {@link Pattern} generated by the provided {@link #instrumentNameRegex()}, or null + * if none was specified. */ @Nullable @Memoized From 2f5862ca496d187709abdb371cbab55d41cab43a Mon Sep 17 00:00:00 2001 From: John Watson Date: Thu, 12 Nov 2020 18:16:36 -0800 Subject: [PATCH 37/37] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java --- .../io/opentelemetry/sdk/metrics/view/InstrumentSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java index e257f24fce2..fa58d52da83 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java @@ -14,7 +14,7 @@ /** * Provides means for selecting one ore more {@link io.opentelemetry.api.metrics.Instrument}s. Used - * for configuring a aggregations for the specified instruments. + * for configuring aggregations for the specified instruments. * *

There are two options for selecting instruments: by instrument name and by instrument type. */