Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Prototype of a very basic Aggregation-configuration API in the SDK. #1412

Closed
wants to merge 13 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,9 @@ public void batch(Labels labelSet, Aggregator aggregator, boolean mappedAggregat
public List<MetricData> completeCollectionCycle() {
return batcher.completeCollectionCycle();
}

@Override
public boolean generatesDeltas() {
return batcher.generatesDeltas();
}
}
3 changes: 3 additions & 0 deletions sdk/src/main/java/io/opentelemetry/sdk/metrics/Batcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,7 @@ interface Batcher {
* @return the list of metrics batched in this Batcher.
*/
List<MetricData> completeCollectionCycle();

/** Does this batcher generate "delta" style metrics. The alternative is "cumulative". */
boolean generatesDeltas();
}
10 changes: 10 additions & 0 deletions sdk/src/main/java/io/opentelemetry/sdk/metrics/Batchers.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ public void batch(Labels labelSet, Aggregator aggregator, boolean mappedAggregat
public List<MetricData> completeCollectionCycle() {
return Collections.emptyList();
}

@Override
public boolean generatesDeltas() {
return false;
}
}

private static final class AllLabels implements Batcher {
Expand Down Expand Up @@ -158,6 +163,11 @@ public final List<MetricData> completeCollectionCycle() {
return Collections.singletonList(
MetricData.create(descriptor, resource, instrumentationLibraryInfo, points));
}

@Override
public boolean generatesDeltas() {
return delta;
}
}

private static Descriptor getDefaultMetricDescriptor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,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;
Expand All @@ -41,11 +43,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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering why we don't want ViewRegistry per MeterSdk instance level? What if I have Metric instruments with the same type and name but I want it have different custom aggregation & temporality on different instrumentationName?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SDK user won't in general know what named Meters are available, so I don't know how you would access the specific Meter in order to have a registry at that level. How were you thinking that an operator would know how to use this if the API were available only on the individual Meters?

this.metricProducer = new MetricProducerSdk(this.registry);
}

Expand Down Expand Up @@ -142,6 +145,34 @@ public MeterSdk newComponent(InstrumentationLibraryInfo instrumentationLibraryIn
}
}

/**
* Register a view with the given {@link InstrumentSelector}.
*
* <p>Example on how to register a view:
*
* <pre>{@code
* // get a handle to the MeterSdkProvider
* MeterSdkProvider meterProvider = OpenTelemetrySdk.getMeterProvider();
*
* // create a selector to select which instruments to customize:
* InstrumentSelector instrumentSelector = InstrumentSelector.newBuilder()
* .instrumentType(InstrumentType.COUNTER)
* .build();
*
* // create a specification of how you want the metrics aggregated:
* AggregationConfiguration viewSpecification =
* AggregationConfiguration.create(Aggregations.minMaxSumCount(), Temporality.DELTA);
*
* //register the view with the MeterSdkProvider
* meterProvider.registerView(instrumentSelector, viewSpecification);
* }</pre>
*
* @see AggregationConfiguration
*/
public void registerView(InstrumentSelector selector, AggregationConfiguration specification) {
viewRegistry.registerView(selector, specification);
}

private static final class MetricProducerSdk implements MetricProducer {
private final MeterSdkComponentRegistry registry;

Expand Down
102 changes: 75 additions & 27 deletions sdk/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,43 @@
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 java.util.Map;
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
* the {@link io.opentelemetry.sdk.metrics.MeterSdkProvider}.
*/
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 final Map<InstrumentSelector, AggregationConfiguration> configuration =
new ConcurrentHashMap<>();

/**
* Create a new {@link io.opentelemetry.sdk.metrics.Batcher} for use in metric recording
* aggregation.
Expand All @@ -47,39 +63,71 @@ Batcher createBatcher(
MeterSharedState meterSharedState,
InstrumentDescriptor descriptor) {

Aggregation aggregation = getRegisteredAggregation(descriptor);
AggregationConfiguration 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()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we'd usually order specification.temporality() == or use a switch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen it both ways...I don't think there's much of a standard, or even agreement on this. Back in the day, I'd use .equals() even for enums, since someone could change it from an enum and break code if you use ==. :)

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());
}

private static Aggregation getRegisteredAggregation(InstrumentDescriptor descriptor) {
// todo look up based on fields of the descriptor.
// todo: consider moving this method to its own class, for more targetted testing.
private AggregationConfiguration findBestMatch(InstrumentDescriptor descriptor) {

for (Map.Entry<InstrumentSelector, AggregationConfiguration> entry : configuration.entrySet()) {
InstrumentSelector registeredSelector = entry.getKey();

if (matchesOnType(descriptor, registeredSelector)
&& matchesOnName(descriptor, registeredSelector)) {
return entry.getValue();
jkwatson marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 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 Aggregations.sum();
case VALUE_RECORDER:
return CUMULATIVE_SUM;
// TODO: Revisit the batcher used here for value observers,
// currently this does not remove duplicate records in the same cycle.
case VALUE_OBSERVER:
return Aggregations.minMaxSumCount();
case VALUE_RECORDER:
return DELTA_SUMMARY;
case SUM_OBSERVER:
case UP_DOWN_SUM_OBSERVER:
return Aggregations.lastValue();
return CUMULATIVE_LAST_VALUE;
}
throw new IllegalArgumentException("Unknown descriptor type: " + descriptor.getType());
}

void registerView(InstrumentSelector selector, AggregationConfiguration specification) {
configuration.put(selector, specification);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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.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
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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 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();
}
}
Loading