From aeb5ea0c56c7339927aea028c6d91bc9677fba4b Mon Sep 17 00:00:00 2001 From: Anton Polyakov Date: Tue, 16 Feb 2021 17:27:06 -0800 Subject: [PATCH] code review: jaadocs, factory method signature --- .../sdk/metrics/AbstractAccumulator.java | 9 +++++++-- .../metrics/SynchronousInstrumentAccumulator.java | 2 +- .../metrics/processor/BaggageLabelsProcessor.java | 6 +++++- .../sdk/metrics/processor/LabelsProcessor.java | 11 ++++++++--- .../metrics/processor/LabelsProcessorFactory.java | 14 +++++++++++--- .../SynchronousInstrumentAccumulatorTest.java | 5 ++++- 6 files changed, 36 insertions(+), 11 deletions(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java index 3b4f91a87d5..b3543c6b80a 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractAccumulator.java @@ -34,11 +34,16 @@ static Aggregator getAggregator( } static LabelsProcessor getLabelsProcessor( - MeterProviderSharedState meterProviderSharedState, InstrumentDescriptor descriptor) { + MeterProviderSharedState meterProviderSharedState, + MeterSharedState meterSharedState, + InstrumentDescriptor descriptor) { return meterProviderSharedState .getViewRegistry() .findView(descriptor) .getLabelsProcessorFactory() - .create(); + .create( + meterProviderSharedState.getResource(), + meterSharedState.getInstrumentationLibraryInfo(), + descriptor); } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulator.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulator.java index 3cc47509869..90b08637bdc 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulator.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulator.java @@ -34,7 +34,7 @@ static SynchronousInstrumentAccumulator create( return new SynchronousInstrumentAccumulator<>( aggregator, new InstrumentProcessor<>(aggregator, meterProviderSharedState.getStartEpochNanos()), - getLabelsProcessor(meterProviderSharedState, descriptor)); + getLabelsProcessor(meterProviderSharedState, meterSharedState, descriptor)); } SynchronousInstrumentAccumulator( diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/BaggageLabelsProcessor.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/BaggageLabelsProcessor.java index e63d94d0e48..c5d213b47a9 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/BaggageLabelsProcessor.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/BaggageLabelsProcessor.java @@ -9,7 +9,11 @@ import io.opentelemetry.api.metrics.common.LabelsBuilder; import io.opentelemetry.context.Context; -public class BaggageLabelsProcessor implements LabelsProcessor { +/** + * A label processor which extracts labels from {@link io.opentelemetry.api.baggage.Baggage}. + * Delegates actual extraction implementation to {@link BaggageMetricsLabelsExtractor} + */ +public final class BaggageLabelsProcessor implements LabelsProcessor { private final BaggageMetricsLabelsExtractor baggageMetricsLabelsExtractor; public BaggageLabelsProcessor(BaggageMetricsLabelsExtractor baggageMetricsLabelsExtractor) { diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessor.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessor.java index b06888ad789..934c4d9d98e 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessor.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessor.java @@ -10,9 +10,14 @@ public interface LabelsProcessor { /** - * Called when bind() method is called. Allows to manipulate labels which this instrument is bound - * to. Particular use case includes enriching lables and/or adding more labels depending on the - * Context + * Called when bound synchronous instrument is created or metrics are recorded for non-bound + * synchronous instrument. Allows to manipulate labels which this instrument is bound to in case + * of binding operation or labels used for recording values in case of non-bound synchronous + * instrument. Particular use case includes enriching labels and/or adding more labels depending + * on the Context + * + *

Please note, this is an experimental API. In case of bound instruments, it will be only + * invoked upon instrument binding and not when measurements are recorded. * * @param ctx context of the operation * @param labels immutable labels. When processors are chained output labels of the previous one diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessorFactory.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessorFactory.java index 126075653ba..0e814acc024 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessorFactory.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/processor/LabelsProcessorFactory.java @@ -5,13 +5,18 @@ package io.opentelemetry.sdk.metrics.processor; +import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; +import io.opentelemetry.sdk.metrics.common.InstrumentDescriptor; +import io.opentelemetry.sdk.resources.Resource; + public interface LabelsProcessorFactory { static LabelsProcessorFactory noop() { - return NoopLabelsProcessor::new; + return (resource, instrumentationLibraryInfo, descriptor) -> new NoopLabelsProcessor(); } static LabelsProcessorFactory baggageExtractor(BaggageMetricsLabelsExtractor labelsExtractor) { - return () -> new BaggageLabelsProcessor(labelsExtractor); + return (resource, instrumentationLibraryInfo, descriptor) -> + new BaggageLabelsProcessor(labelsExtractor); } /** @@ -19,5 +24,8 @@ static LabelsProcessorFactory baggageExtractor(BaggageMetricsLabelsExtractor lab * * @return new {@link LabelsProcessorFactory} */ - LabelsProcessor create(); + LabelsProcessor create( + Resource resource, + InstrumentationLibraryInfo instrumentationLibraryInfo, + InstrumentDescriptor descriptor); } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulatorTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulatorTest.java index 34650bd3540..e0151bf4b1f 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulatorTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SynchronousInstrumentAccumulatorTest.java @@ -30,7 +30,10 @@ public class SynchronousInstrumentAccumulatorTest { AggregatorFactory.lastValue() .create( Resource.getEmpty(), InstrumentationLibraryInfo.create("test", "1.0"), DESCRIPTOR); - private final LabelsProcessor labelsProcessor = LabelsProcessorFactory.noop().create(); + private final LabelsProcessor labelsProcessor = + LabelsProcessorFactory.noop() + .create( + Resource.getEmpty(), InstrumentationLibraryInfo.create("test", "1.0"), DESCRIPTOR); @Test void sameAggregator_ForSameLabelSet() {