From 33e845d2e8ff84b38898f9233574a17cb5227611 Mon Sep 17 00:00:00 2001 From: Bogdan Cristian Drutu Date: Wed, 12 Feb 2020 10:25:36 -0800 Subject: [PATCH] Bound registry work in progress Signed-off-by: Bogdan Cristian Drutu --- .../sdk/metrics/AbstractBoundInstrument.java | 11 +- .../sdk/metrics/AbstractInstrument.java | 11 ++ .../AbstractInstrumentWithBinding.java | 116 +++++++++++++++++ .../sdk/metrics/ActiveViewAggregator.java | 120 ++++++++++++++++++ .../sdk/metrics/AggregatorMap.java | 106 ++++++++++++++++ .../sdk/metrics/AggregatorView.java | 86 +++++++++++++ .../sdk/metrics/DoubleCounterSdk.java | 19 ++- .../sdk/metrics/DoubleMeasureSdk.java | 17 ++- .../sdk/metrics/LongCounterSdk.java | 19 ++- .../sdk/metrics/LongMeasureSdk.java | 17 ++- .../metrics/AbstractBoundInstrumentTest.java | 57 +++++++-- 11 files changed, 549 insertions(+), 30 deletions(-) create mode 100644 sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentWithBinding.java create mode 100644 sdk/src/main/java/io/opentelemetry/sdk/metrics/ActiveViewAggregator.java create mode 100644 sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorMap.java create mode 100644 sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorView.java diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrument.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrument.java index c079bafe0bc..13c421ead86 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrument.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrument.java @@ -17,6 +17,7 @@ package io.opentelemetry.sdk.metrics; import io.opentelemetry.metrics.InstrumentWithBinding.BoundInstrument; +import io.opentelemetry.metrics.LabelSet; import io.opentelemetry.sdk.metrics.aggregator.Aggregator; import java.util.concurrent.atomic.AtomicLong; @@ -32,11 +33,13 @@ abstract class AbstractBoundInstrument implements BoundInstrument { // Atomically counts the number of references (usages) while also keeping a state of // mapped/unmapped into a registry map. private final AtomicLong refCountMapped; + private final ActiveViewAggregator activeViewAggregator; private final Aggregator aggregator; - AbstractBoundInstrument(Aggregator aggregator) { - this.aggregator = aggregator; + AbstractBoundInstrument(ActiveViewAggregator activeViewAggregator) { this.refCountMapped = new AtomicLong(0); + this.activeViewAggregator = activeViewAggregator; + this.aggregator = activeViewAggregator.newAggregator(); } /** @@ -78,4 +81,8 @@ final void recordLong(long value) { final void recordDouble(double value) { aggregator.recordDouble(value); } + + final void collect(LabelSet labelSet) { + activeViewAggregator.collect(labelSet, aggregator); + } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrument.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrument.java index c1e8a4ff261..a3efaa3bfac 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrument.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrument.java @@ -32,6 +32,7 @@ abstract class AbstractInstrument implements Instrument { private final List labelKeys; private final MeterSharedState meterSharedState; private final InstrumentationLibraryInfo instrumentationLibraryInfo; + private final ActiveViewAggregator activeViewAggregator; // All arguments cannot be null because they are checked in the abstract builder classes. AbstractInstrument( @@ -51,6 +52,12 @@ abstract class AbstractInstrument implements Instrument { this.labelKeys = labelKeys; this.meterSharedState = meterSharedState; this.instrumentationLibraryInfo = instrumentationLibraryInfo; + activeViewAggregator = + new ActiveViewAggregator( + instrumentType, + instrumentValueType, + meterSharedState.getResource(), + instrumentationLibraryInfo); } final String getName() { @@ -81,6 +88,10 @@ final InstrumentationLibraryInfo getInstrumentationLibraryInfo() { return instrumentationLibraryInfo; } + final ActiveViewAggregator getActiveViewAggregator() { + return activeViewAggregator; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentWithBinding.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentWithBinding.java new file mode 100644 index 00000000000..8c91b036939 --- /dev/null +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentWithBinding.java @@ -0,0 +1,116 @@ +/* + * 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; + +import io.opentelemetry.metrics.LabelSet; +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.data.MetricData; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; + +abstract class AbstractInstrumentWithBinding + extends AbstractInstrument { + private final ConcurrentHashMap boundLabels; + private final ReentrantLock collectLock; + private long collectionStartTime; + + AbstractInstrumentWithBinding( + String name, + String description, + String unit, + Map constantLabels, + List labelKeys, + InstrumentType instrumentType, + InstrumentValueType instrumentValueType, + MeterSharedState meterSharedState, + InstrumentationLibraryInfo instrumentationLibraryInfo) { + super( + name, + description, + unit, + constantLabels, + labelKeys, + instrumentType, + instrumentValueType, + meterSharedState, + instrumentationLibraryInfo); + boundLabels = new ConcurrentHashMap<>(); + collectLock = new ReentrantLock(); + collectionStartTime = getMeterSharedState().getClock().now(); + } + + // Cannot make this "bind" because of a Java problem if we make this class also implement the + // InstrumentWithBinding then the subclass will fail to compile because of different "bind" + // signature. This is a good trade-off. + final B bindInternal(LabelSet labelSet) { + B binding = boundLabels.get(labelSet); + if (binding != null && binding.bind()) { + // At this moment it is guaranteed that the Bound is in the map and will not be removed. + return binding; + } + + // Missing entry or no longer mapped, try to add a new entry. + binding = newBinding(); + while (true) { + B oldBound = boundLabels.putIfAbsent(labelSet, binding); + if (oldBound != null) { + if (oldBound.bind()) { + // At this moment it is guaranteed that the Bound is in the map and will not be removed. + return oldBound; + } + // Try to remove the oldBound. This will race with the collect method, but only one will + // succeed. + boundLabels.remove(labelSet, oldBound); + continue; + } + return binding; + } + } + + /** + * Collects records from all the entries (labelSet, Bound) that changed since the last collect() + * call. + */ + final Collection collect() { + getActiveViewAggregator().startCollection(); + long previousCollectionStartTime = collectionStartTime; + collectionStartTime = getMeterSharedState().getClock().now(); + collectLock.lock(); + try { + for (Map.Entry entry : boundLabels.entrySet()) { + if (entry.getValue().tryUnmap()) { + // If able to unmap then remove the record from the current Map. This can race with the + // acquire but because we requested a specific value only one will succeed. + boundLabels.remove(entry.getKey(), entry.getValue()); + } + + entry.getValue().collect(entry.getKey()); + } + } finally { + collectLock.unlock(); + } + return getActiveViewAggregator() + .stopCollection(previousCollectionStartTime, collectionStartTime); + } + + abstract B newBinding(); +} diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/ActiveViewAggregator.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/ActiveViewAggregator.java new file mode 100644 index 00000000000..3ad2d8063e3 --- /dev/null +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/ActiveViewAggregator.java @@ -0,0 +1,120 @@ +/* + * 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; + +import io.opentelemetry.metrics.LabelSet; +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.aggregator.Aggregator; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor; +import io.opentelemetry.sdk.metrics.view.Aggregation; +import io.opentelemetry.sdk.resources.Resource; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import javax.annotation.Nullable; + +// The current implementation allows to change the active ViewAggregator only before any record +// or binding happens. This is good for the moment to support default aggregation for all the +// instruments but needs to support adding/removing views at any moment, as well as support for +// multiple views in the same time. +final class ActiveViewAggregator { + private final InstrumentType instrumentType; + private final InstrumentValueType instrumentValueType; + private final Resource resource; + private final InstrumentationLibraryInfo instrumentationLibraryInfo; + private AggregatorMap aggregatorMap; + private volatile AggregatorView currentAggregatorView; + + ActiveViewAggregator( + InstrumentType instrumentType, + InstrumentValueType instrumentValueType, + Resource resource, + InstrumentationLibraryInfo instrumentationLibraryInfo) { + this.instrumentType = instrumentType; + this.instrumentValueType = instrumentValueType; + this.resource = resource; + this.instrumentationLibraryInfo = instrumentationLibraryInfo; + currentAggregatorView = AggregatorView.getNoop(); + } + + // TODO: Change this to: void addViewAggregator(View view); + void addView( + String name, + String description, + String unit, + Map constantLabels, + Aggregation aggregation) { + // TODO: Add support to reduce labels. + this.currentAggregatorView = + AggregatorView.getAllLabels( + toDescriptor( + name, + description, + unit, + constantLabels, + aggregation, + instrumentType, + instrumentValueType), + resource, + instrumentationLibraryInfo, + aggregation.getAggregatorFactory(instrumentValueType)); + } + + // Caller needs to call these methods in the following order (while holding a lock): + // * startCollection(); + // * collect(); // May be called multiple times. + // * startCollection() + void startCollection() { + aggregatorMap = currentAggregatorView.newAggregatorMap(); + } + + void collect(LabelSet labelSet, Aggregator aggregator) { + aggregatorMap.collect(labelSet, aggregator); + } + + Collection stopCollection(long startEpochNanos, long epochNanos) { + @Nullable MetricData metricData = aggregatorMap.toMetricData(startEpochNanos, epochNanos); + return metricData == null + ? Collections.emptyList() + : Collections.singletonList(metricData); + } + + Aggregator newAggregator() { + return currentAggregatorView.getAggregator(); + } + + // This should change the signature when we add View to be: + // static Descriptor toDescriptor(View, InstrumentType, InstrumentValueType); + private static Descriptor toDescriptor( + String name, + String description, + String unit, + Map constantLabels, + Aggregation aggregation, + InstrumentType instrumentType, + InstrumentValueType instrumentValueType) { + return Descriptor.create( + name, + description, + unit, + aggregation.getDescriptorType(instrumentType, instrumentValueType), + constantLabels); + } +} diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorMap.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorMap.java new file mode 100644 index 00000000000..fbea5a517d5 --- /dev/null +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorMap.java @@ -0,0 +1,106 @@ +/* + * 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; + +import io.opentelemetry.metrics.LabelSet; +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.aggregator.Aggregator; +import io.opentelemetry.sdk.metrics.aggregator.AggregatorFactory; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor; +import io.opentelemetry.sdk.metrics.data.MetricData.Point; +import io.opentelemetry.sdk.resources.Resource; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; + +abstract class AggregatorMap { + static AggregatorMap getNoop() { + return Noop.INSTANCE; + } + + static AggregatorMap getAllLabels( + Descriptor descriptor, + Resource resource, + InstrumentationLibraryInfo instrumentationLibraryInfo, + AggregatorFactory aggregatorFactory) { + return new AllLabels(descriptor, resource, instrumentationLibraryInfo, aggregatorFactory); + } + + abstract void collect(LabelSet labelSet, Aggregator aggregator); + + @Nullable + abstract MetricData toMetricData(long startEpochNanos, long epochNanos); + + private static final class Noop extends AggregatorMap { + private static final AggregatorMap INSTANCE = new Noop(); + + @Override + void collect(LabelSet labelSet, Aggregator aggregator) { + // Noop. + } + + @Nullable + @Override + MetricData toMetricData(long startEpochNanos, long epochNanos) { + return null; + } + } + + private static final class AllLabels extends AggregatorMap { + private final Descriptor descriptor; + private final Resource resource; + private final InstrumentationLibraryInfo instrumentationLibraryInfo; + private final AggregatorFactory aggregatorFactory; + private final Map, Aggregator> aggregatorMap; + + private AllLabels( + Descriptor descriptor, + Resource resource, + InstrumentationLibraryInfo instrumentationLibraryInfo, + AggregatorFactory aggregatorFactory) { + this.descriptor = descriptor; + this.resource = resource; + this.instrumentationLibraryInfo = instrumentationLibraryInfo; + this.aggregatorFactory = aggregatorFactory; + this.aggregatorMap = new HashMap<>(); + } + + @Override + final void collect(LabelSet labelSet, Aggregator aggregator) { + // TODO: Add support to reduce labels. + Map labels = ((LabelSetSdk) labelSet).getLabels(); + Aggregator currentAggregator = aggregatorMap.get(labels); + if (currentAggregator == null) { + currentAggregator = aggregatorFactory.getAggregator(); + aggregatorMap.put(labels, currentAggregator); + } + currentAggregator.mergeToAndReset(aggregator); + } + + @Override + final MetricData toMetricData(long startEpochNanos, long epochNanos) { + List points = new ArrayList<>(aggregatorMap.size()); + for (Map.Entry, Aggregator> entry : aggregatorMap.entrySet()) { + points.add(entry.getValue().toPoint(startEpochNanos, epochNanos, entry.getKey())); + } + return MetricData.create(descriptor, resource, instrumentationLibraryInfo, points); + } + } +} diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorView.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorView.java new file mode 100644 index 00000000000..07c41a6dca8 --- /dev/null +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/AggregatorView.java @@ -0,0 +1,86 @@ +/* + * 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; + +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.aggregator.Aggregator; +import io.opentelemetry.sdk.metrics.aggregator.AggregatorFactory; +import io.opentelemetry.sdk.metrics.aggregator.NoopAggregator; +import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor; +import io.opentelemetry.sdk.resources.Resource; + +abstract class AggregatorView { + + static AggregatorView getNoop() { + return Noop.INSTANCE; + } + + static AggregatorView getAllLabels( + Descriptor descriptor, + Resource resource, + InstrumentationLibraryInfo instrumentationLibraryInfo, + AggregatorFactory aggregatorFactory) { + return new AllLabels(descriptor, resource, instrumentationLibraryInfo, aggregatorFactory); + } + + abstract Aggregator getAggregator(); + + abstract AggregatorMap newAggregatorMap(); + + private static final class Noop extends AggregatorView { + private static final AggregatorView INSTANCE = new Noop(); + + @Override + Aggregator getAggregator() { + return NoopAggregator.getFactory().getAggregator(); + } + + @Override + AggregatorMap newAggregatorMap() { + return AggregatorMap.getNoop(); + } + } + + private static final class AllLabels extends AggregatorView { + private final Descriptor descriptor; + private final Resource resource; + private final InstrumentationLibraryInfo instrumentationLibraryInfo; + private final AggregatorFactory aggregatorFactory; + + AllLabels( + Descriptor descriptor, + Resource resource, + InstrumentationLibraryInfo instrumentationLibraryInfo, + AggregatorFactory aggregatorFactory) { + this.descriptor = descriptor; + this.resource = resource; + this.instrumentationLibraryInfo = instrumentationLibraryInfo; + this.aggregatorFactory = aggregatorFactory; + } + + @Override + Aggregator getAggregator() { + return aggregatorFactory.getAggregator(); + } + + @Override + AggregatorMap newAggregatorMap() { + return AggregatorMap.getAllLabels( + descriptor, resource, instrumentationLibraryInfo, aggregatorFactory); + } + } +} diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleCounterSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleCounterSdk.java index 6d8d8276a80..8791468c16d 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleCounterSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleCounterSdk.java @@ -19,11 +19,14 @@ import io.opentelemetry.metrics.DoubleCounter; import io.opentelemetry.metrics.LabelSet; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.DoubleCounterSdk.BoundInstrument; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.Aggregations; import java.util.List; import java.util.Map; -final class DoubleCounterSdk extends AbstractInstrument implements DoubleCounter { +final class DoubleCounterSdk extends AbstractInstrumentWithBinding + implements DoubleCounter { private final boolean monotonic; @@ -47,6 +50,7 @@ private DoubleCounterSdk( sharedState, instrumentationLibraryInfo); this.monotonic = monotonic; + getActiveViewAggregator().addView(name, description, unit, constantLabels, Aggregations.sum()); } @Override @@ -58,7 +62,12 @@ public void add(double delta, LabelSet labelSet) { @Override public BoundInstrument bind(LabelSet labelSet) { - return new BoundInstrument(monotonic); + return bindInternal(labelSet); + } + + @Override + BoundInstrument newBinding() { + return new BoundInstrument(monotonic, getActiveViewAggregator()); } @Override @@ -89,8 +98,8 @@ static final class BoundInstrument extends AbstractBoundInstrument implements Bo private final boolean monotonic; - BoundInstrument(boolean monotonic) { - super(null); + BoundInstrument(boolean monotonic, ActiveViewAggregator activeViewAggregator) { + super(activeViewAggregator); this.monotonic = monotonic; } @@ -99,7 +108,7 @@ public void add(double delta) { if (monotonic && delta < 0) { throw new IllegalArgumentException("monotonic counters can only increase"); } - // TODO: pass through to an aggregator/accumulator + recordDouble(delta); } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleMeasureSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleMeasureSdk.java index 1a801797d70..50547cb1834 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleMeasureSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/DoubleMeasureSdk.java @@ -19,11 +19,13 @@ import io.opentelemetry.metrics.DoubleMeasure; import io.opentelemetry.metrics.LabelSet; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.DoubleMeasureSdk.BoundInstrument; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; import java.util.List; import java.util.Map; -final class DoubleMeasureSdk extends AbstractInstrument implements DoubleMeasure { +final class DoubleMeasureSdk extends AbstractInstrumentWithBinding + implements DoubleMeasure { private final boolean absolute; @@ -58,7 +60,12 @@ public void record(double value, LabelSet labelSet) { @Override public BoundInstrument bind(LabelSet labelSet) { - return new BoundInstrument(this.absolute); + return bindInternal(labelSet); + } + + @Override + BoundInstrument newBinding() { + return new BoundInstrument(this.absolute, getActiveViewAggregator()); } @Override @@ -89,8 +96,8 @@ static final class BoundInstrument extends AbstractBoundInstrument implements Bo private final boolean absolute; - BoundInstrument(boolean absolute) { - super(null); + BoundInstrument(boolean absolute, ActiveViewAggregator activeViewAggregator) { + super(activeViewAggregator); this.absolute = absolute; } @@ -99,7 +106,7 @@ public void record(double value) { if (this.absolute && value < 0) { throw new IllegalArgumentException("absolute measure can only record positive values"); } - // TODO: pass through to an aggregator/accumulator + recordDouble(value); } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongCounterSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongCounterSdk.java index 6668e5da354..2cda3412a4d 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongCounterSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongCounterSdk.java @@ -19,11 +19,14 @@ import io.opentelemetry.metrics.LabelSet; import io.opentelemetry.metrics.LongCounter; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.LongCounterSdk.BoundInstrument; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.view.Aggregations; import java.util.List; import java.util.Map; -final class LongCounterSdk extends AbstractInstrument implements LongCounter { +final class LongCounterSdk extends AbstractInstrumentWithBinding + implements LongCounter { private final boolean monotonic; @@ -47,6 +50,7 @@ private LongCounterSdk( sharedState, instrumentationLibraryInfo); this.monotonic = monotonic; + getActiveViewAggregator().addView(name, description, unit, constantLabels, Aggregations.sum()); } @Override @@ -58,7 +62,12 @@ public void add(long delta, LabelSet labelSet) { @Override public BoundInstrument bind(LabelSet labelSet) { - return new BoundInstrument(monotonic); + return bindInternal(labelSet); + } + + @Override + BoundInstrument newBinding() { + return new BoundInstrument(monotonic, getActiveViewAggregator()); } @Override @@ -89,8 +98,8 @@ static final class BoundInstrument extends AbstractBoundInstrument implements Bo private final boolean monotonic; - BoundInstrument(boolean monotonic) { - super(null); + BoundInstrument(boolean monotonic, ActiveViewAggregator activeViewAggregator) { + super(activeViewAggregator); this.monotonic = monotonic; } @@ -99,7 +108,7 @@ public void add(long delta) { if (monotonic && delta < 0) { throw new IllegalArgumentException("monotonic counters can only increase"); } - // TODO: pass through to an aggregator/accumulator + recordLong(delta); } } diff --git a/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongMeasureSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongMeasureSdk.java index 7a47b104bee..9f352901baa 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongMeasureSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/metrics/LongMeasureSdk.java @@ -19,11 +19,13 @@ import io.opentelemetry.metrics.LabelSet; import io.opentelemetry.metrics.LongMeasure; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.LongMeasureSdk.BoundInstrument; import io.opentelemetry.sdk.metrics.common.InstrumentValueType; import java.util.List; import java.util.Map; -final class LongMeasureSdk extends AbstractInstrument implements LongMeasure { +final class LongMeasureSdk extends AbstractInstrumentWithBinding + implements LongMeasure { private final boolean absolute; @@ -58,7 +60,12 @@ public void record(long value, LabelSet labelSet) { @Override public BoundInstrument bind(LabelSet labelSet) { - return new BoundInstrument(this.absolute); + return bindInternal(labelSet); + } + + @Override + BoundInstrument newBinding() { + return new BoundInstrument(this.absolute, getActiveViewAggregator()); } @Override @@ -89,8 +96,8 @@ static final class BoundInstrument extends AbstractBoundInstrument implements Bo private final boolean absolute; - BoundInstrument(boolean absolute) { - super(null); + BoundInstrument(boolean absolute, ActiveViewAggregator activeViewAggregator) { + super(activeViewAggregator); this.absolute = absolute; } @@ -99,7 +106,7 @@ public void record(long value) { if (this.absolute && value < 0) { throw new IllegalArgumentException("absolute measure can only record positive values"); } - // TODO: pass through to an aggregator/accumulator + recordLong(value); } } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrumentTest.java b/sdk/src/test/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrumentTest.java index be299bc3f36..f81782c6ab7 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrumentTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/metrics/AbstractBoundInstrumentTest.java @@ -18,7 +18,15 @@ import static com.google.common.truth.Truth.assertThat; +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.metrics.aggregator.Aggregator; +import io.opentelemetry.sdk.metrics.aggregator.AggregatorFactory; +import io.opentelemetry.sdk.metrics.common.InstrumentType; +import io.opentelemetry.sdk.metrics.common.InstrumentValueType; +import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor.Type; +import io.opentelemetry.sdk.metrics.view.Aggregation; +import io.opentelemetry.sdk.resources.Resource; +import java.util.Collections; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,15 +39,48 @@ @RunWith(JUnit4.class) public class AbstractBoundInstrumentTest { @Mock private Aggregator aggregator; + private final ActiveViewAggregator activeViewAggregator = + new ActiveViewAggregator( + InstrumentType.COUNTER_MONOTONIC, + InstrumentValueType.LONG, + Resource.getEmpty(), + InstrumentationLibraryInfo.EMPTY); @Before public void setUp() { MockitoAnnotations.initMocks(this); + activeViewAggregator.addView( + "mock_metric", + "Mock metric for testing", + "1", + Collections.emptyMap(), + new Aggregation() { + @Override + public AggregatorFactory getAggregatorFactory(InstrumentValueType instrumentValueType) { + return new AggregatorFactory() { + @Override + public Aggregator getAggregator() { + return aggregator; + } + }; + } + + @Override + public Type getDescriptorType( + InstrumentType instrumentType, InstrumentValueType instrumentValueType) { + return Type.MONOTONIC_LONG; + } + + @Override + public boolean availableForInstrument(InstrumentType instrumentType) { + return true; + } + }); } @Test public void bindMapped() { - TestBoundInstrument testBoundInstrument = new TestBoundInstrument(aggregator); + TestBoundInstrument testBoundInstrument = new TestBoundInstrument(activeViewAggregator); assertThat(testBoundInstrument.bind()).isTrue(); testBoundInstrument.unbind(); assertThat(testBoundInstrument.bind()).isTrue(); @@ -52,7 +93,7 @@ public void bindMapped() { @Test public void tryUnmap_BoundInstrument() { - TestBoundInstrument testBoundInstrument = new TestBoundInstrument(aggregator); + TestBoundInstrument testBoundInstrument = new TestBoundInstrument(activeViewAggregator); assertThat(testBoundInstrument.bind()).isTrue(); assertThat(testBoundInstrument.tryUnmap()).isFalse(); testBoundInstrument.unbind(); @@ -61,7 +102,7 @@ public void tryUnmap_BoundInstrument() { @Test public void tryUnmap_BoundInstrument_MultipleTimes() { - TestBoundInstrument testBoundInstrument = new TestBoundInstrument(aggregator); + TestBoundInstrument testBoundInstrument = new TestBoundInstrument(activeViewAggregator); assertThat(testBoundInstrument.bind()).isTrue(); assertThat(testBoundInstrument.bind()).isTrue(); assertThat(testBoundInstrument.bind()).isTrue(); @@ -79,7 +120,7 @@ public void tryUnmap_BoundInstrument_MultipleTimes() { @Test public void bind_ThenUnmap_ThenTryToBind() { - TestBoundInstrument testBoundInstrument = new TestBoundInstrument(aggregator); + TestBoundInstrument testBoundInstrument = new TestBoundInstrument(activeViewAggregator); assertThat(testBoundInstrument.bind()).isTrue(); testBoundInstrument.unbind(); assertThat(testBoundInstrument.tryUnmap()).isTrue(); @@ -89,7 +130,7 @@ public void bind_ThenUnmap_ThenTryToBind() { @Test public void recordDoubleValue() { - TestBoundInstrument testBoundInstrument = new TestBoundInstrument(aggregator); + TestBoundInstrument testBoundInstrument = new TestBoundInstrument(activeViewAggregator); Mockito.verifyZeroInteractions(aggregator); Mockito.doNothing().when(aggregator).recordDouble(Mockito.anyDouble()); testBoundInstrument.recordDouble(1.2); @@ -98,7 +139,7 @@ public void recordDoubleValue() { @Test public void recordLongValue() { - TestBoundInstrument testBoundInstrument = new TestBoundInstrument(aggregator); + TestBoundInstrument testBoundInstrument = new TestBoundInstrument(activeViewAggregator); Mockito.verifyZeroInteractions(aggregator); Mockito.doNothing().when(aggregator).recordLong(Mockito.anyLong()); testBoundInstrument.recordLong(13); @@ -106,8 +147,8 @@ public void recordLongValue() { } private static final class TestBoundInstrument extends AbstractBoundInstrument { - TestBoundInstrument(Aggregator aggregator) { - super(aggregator); + TestBoundInstrument(ActiveViewAggregator activeViewAggregator) { + super(activeViewAggregator); } } }