Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a202a21
swap handle async
fandreuz May 12, 2025
c83804d
static
fandreuz May 12, 2025
f4a4093
atomicref
fandreuz May 12, 2025
7039b42
set meter config
fandreuz May 12, 2025
0bd9d86
null checks
fandreuz May 12, 2025
73a42da
generic drop agg
fandreuz May 12, 2025
3e4ef5c
setEnabled
fandreuz May 12, 2025
9136311
disable storages
fandreuz May 12, 2025
6de7e98
ops
fandreuz May 12, 2025
b44530c
move down
fandreuz May 12, 2025
d9e39f1
unit tests
fandreuz May 12, 2025
c6c8f30
fix stuff and order
fandreuz May 12, 2025
b42e68a
Merge branch 'main' into 7051-metric-config
fandreuz May 16, 2025
3b6f149
fix cmpl
fandreuz May 16, 2025
285d43c
optimize setEnabled
fandreuz May 21, 2025
e656bd9
optimize setEnabled
fandreuz May 21, 2025
21e188e
fix test name
fandreuz May 21, 2025
03d159d
nonnnull
fandreuz May 21, 2025
bd141f6
new test
fandreuz May 21, 2025
b00d726
fix isEnabled
fandreuz May 21, 2025
fb846dc
new tests
fandreuz May 21, 2025
30d81f5
new tests
fandreuz May 21, 2025
21d36d5
memory mode
fandreuz May 21, 2025
4196b17
cc
fandreuz May 21, 2025
3c10154
aggregator reset tests
fandreuz May 21, 2025
9b12918
more tests
fandreuz May 21, 2025
c8e0c85
Merge remote-tracking branch 'fandreuz/7051-metric-config' into 7051-…
fandreuz May 21, 2025
b21fa7b
ops
fandreuz May 21, 2025
e58c13c
nn
fandreuz May 22, 2025
d747d66
clear
fandreuz May 22, 2025
6ca257d
no order
fandreuz May 22, 2025
5f33bee
Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/interna…
fandreuz May 22, 2025
e5de344
Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/interna…
fandreuz May 23, 2025
7ab9b3a
spotless
fandreuz May 23, 2025
f3e3b50
Simplify dynamic meter config mechanics
jack-berg Jun 16, 2025
f46b35e
Fix tests
jack-berg Sep 16, 2025
fbfc03c
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
jack-berg Sep 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ final class SdkMeter implements Meter {
private final MeterProviderSharedState meterProviderSharedState;
private final InstrumentationScopeInfo instrumentationScopeInfo;
private final Map<RegisteredReader, MetricStorageRegistry> readerStorageRegistries;
private final boolean meterEnabled;

private boolean meterEnabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect this is going to need some sort of memory barrier around it. Either make it an AtomicBoolean, or provide some kind of synchronization around the update to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jack-berg should this field be marked volatile ?

Copy link
Member

Choose a reason for hiding this comment

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

In other similar situations we've punted on the volatile keyword with the argument that even the small performance impact of reading a volatile field vs a non-volatile field is too high a price to pay the dynamism we get from this feature, which will only be leveraged by a few users. I know that in theory not marking it volatile may mean that the changes are never seen by readers, but I'd like to see complaints of that in practice before paying the performance penalty.

In this particular instance, adding volatile isn't terribly consequential because the field is only read on collection. The read work happens down in DefaultSynchronousMetricStorage, which has an enabled field which is also marked non-volatile and which is read each and every time a measurement is recorded. So we could update SdkMeter#meterEnabled to be volatile, but I don't think we should make DefaultSynchronousMetricStorage#enabled volatile. So the question is, should we be mark SdkMeter#meterEnabled because we can, even though its not really consequential? Or leave it be for consistency with SdkTracer, SdkLogger, DefaultSynchronousMetricStorage#enabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough. I'm ok with it, if you're ok with it.


SdkMeter(
MeterProviderSharedState meterProviderSharedState,
Expand All @@ -103,28 +104,41 @@ final class SdkMeter implements Meter {
this.meterEnabled = meterConfig.isEnabled();
}

void updateMeterConfig(MeterConfig meterConfig) {
meterEnabled = meterConfig.isEnabled();

for (RegisteredReader registeredReader : readerStorageRegistries.keySet()) {
Collection<MetricStorage> storages =
Objects.requireNonNull(readerStorageRegistries.get(registeredReader)).getStorages();
for (MetricStorage storage : storages) {
storage.setEnabled(meterEnabled);
}
}
}

// Visible for testing
InstrumentationScopeInfo getInstrumentationScopeInfo() {
return instrumentationScopeInfo;
}

/** Collect all metrics for the meter. */
Collection<MetricData> collectAll(RegisteredReader registeredReader, long epochNanos) {
// Short circuit collection process if meter is disabled
if (!meterEnabled) {
return Collections.emptyList();
}
List<CallbackRegistration> currentRegisteredCallbacks;
synchronized (callbackLock) {
currentRegisteredCallbacks = new ArrayList<>(callbackRegistrations);
}
// Collections across all readers are sequential
synchronized (collectLock) {
for (CallbackRegistration callbackRegistration : currentRegisteredCallbacks) {
callbackRegistration.invokeCallback(
registeredReader, meterProviderSharedState.getStartEpochNanos(), epochNanos);
// Only invoke callbacks if meter is enabled
if (meterEnabled) {
for (CallbackRegistration callbackRegistration : currentRegisteredCallbacks) {
callbackRegistration.invokeCallback(
registeredReader, meterProviderSharedState.getStartEpochNanos(), epochNanos);
}
}

// Collect even if meter is disabled. Storage is responsible for managing state and returning
// empty metric if disabled.
Collection<MetricStorage> storages =
Objects.requireNonNull(readerStorageRegistries.get(registeredReader)).getStorages();
List<MetricData> result = new ArrayList<>(storages.size());
Expand Down Expand Up @@ -275,7 +289,8 @@ WriteableMetricStorage registerSynchronousMetricStorage(InstrumentDescriptor ins
reader,
registeredView,
instrument,
meterProviderSharedState.getExemplarFilter())));
meterProviderSharedState.getExemplarFilter(),
meterEnabled)));
}
}

Expand All @@ -301,7 +316,8 @@ SdkObservableMeasurement registerObservableMeasurement(
}
registeredStorages.add(
registry.register(
AsynchronousMetricStorage.create(reader, registeredView, instrumentDescriptor)));
AsynchronousMetricStorage.create(
reader, registeredView, instrumentDescriptor, meterEnabled)));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ public final class SdkMeterProvider implements MeterProvider, Closeable {
private final List<MetricProducer> metricProducers;
private final MeterProviderSharedState sharedState;
private final ComponentRegistry<SdkMeter> registry;
private final ScopeConfigurator<MeterConfig> meterConfigurator;
private final AtomicBoolean isClosed = new AtomicBoolean(false);

private ScopeConfigurator<MeterConfig> meterConfigurator;

/** Returns a new {@link SdkMeterProviderBuilder} for {@link SdkMeterProvider}. */
public static SdkMeterProviderBuilder builder() {
return new SdkMeterProviderBuilder();
Expand Down Expand Up @@ -105,6 +106,15 @@ private MeterConfig getMeterConfig(InstrumentationScopeInfo instrumentationScope
return meterConfig == null ? MeterConfig.defaultConfig() : meterConfig;
}

void setMeterConfigurator(ScopeConfigurator<MeterConfig> meterConfigurator) {
this.meterConfigurator = meterConfigurator;
this.registry
.getComponents()
.forEach(
sdkMeter ->
sdkMeter.updateMeterConfig(getMeterConfig(sdkMeter.getInstrumentationScopeInfo())));
}

@Override
public MeterBuilder meterBuilder(String instrumentationScopeName) {
if (registeredReaders.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ public static SdkMeterProviderBuilder setExemplarFilter(
return sdkMeterProviderBuilder;
}

/** Reflectively set the {@link ScopeConfigurator} to the {@link SdkMeterProvider}. */
public static void setMeterConfigurator(
SdkMeterProvider sdkMeterProvider, ScopeConfigurator<MeterConfig> scopeConfigurator) {
try {
Method method =
SdkMeterProvider.class.getDeclaredMethod("setMeterConfigurator", ScopeConfigurator.class);
method.setAccessible(true);
method.invoke(sdkMeterProvider, scopeConfigurator);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
throw new IllegalStateException("Error calling setMeterConfigurator on SdkMeterProvider", e);
}
}

/** Reflectively set the {@link ScopeConfigurator} to the {@link SdkMeterProviderBuilder}. */
public static SdkMeterProviderBuilder setMeterConfigurator(
SdkMeterProviderBuilder sdkMeterProviderBuilder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.opentelemetry.sdk.metrics.internal.aggregator.Aggregator;
import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorFactory;
import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorHandle;
import io.opentelemetry.sdk.metrics.internal.aggregator.EmptyMetricData;
import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
import io.opentelemetry.sdk.metrics.internal.descriptor.MetricDescriptor;
import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter;
Expand Down Expand Up @@ -86,12 +87,15 @@ public final class AsynchronousMetricStorage<T extends PointData, U extends Exem
private long startEpochNanos;
private long epochNanos;

private boolean enabled;

private AsynchronousMetricStorage(
RegisteredReader registeredReader,
MetricDescriptor metricDescriptor,
Aggregator<T, U> aggregator,
AttributesProcessor attributesProcessor,
int maxCardinality) {
int maxCardinality,
boolean enabled) {
this.registeredReader = registeredReader;
this.metricDescriptor = metricDescriptor;
this.aggregationTemporality =
Expand All @@ -102,6 +106,7 @@ private AsynchronousMetricStorage(
this.aggregator = aggregator;
this.attributesProcessor = attributesProcessor;
this.maxCardinality = maxCardinality - 1;
this.enabled = enabled;
this.reusablePointsPool = new ObjectPool<>(aggregator::createReusablePoint);
this.reusableHandlesPool = new ObjectPool<>(aggregator::createHandle);
this.handleBuilder = ignored -> reusableHandlesPool.borrowObject();
Expand All @@ -125,7 +130,8 @@ private AsynchronousMetricStorage(
AsynchronousMetricStorage<T, U> create(
RegisteredReader registeredReader,
RegisteredView registeredView,
InstrumentDescriptor instrumentDescriptor) {
InstrumentDescriptor instrumentDescriptor,
boolean enabled) {
View view = registeredView.getView();
MetricDescriptor metricDescriptor =
MetricDescriptor.create(view, registeredView.getViewSourceInfo(), instrumentDescriptor);
Expand All @@ -140,7 +146,8 @@ AsynchronousMetricStorage<T, U> create(
metricDescriptor,
aggregator,
registeredView.getViewAttributesProcessor(),
registeredView.getCardinalityLimit());
registeredView.getCardinalityLimit(),
enabled);
}

/** Record callback measurement from {@link ObservableLongMeasurement}. */
Expand Down Expand Up @@ -207,8 +214,10 @@ public MetricData collect(
aggregatorHandles.forEach(handleReleaser);
aggregatorHandles.clear();

return aggregator.toMetricData(
resource, instrumentationScopeInfo, metricDescriptor, result, aggregationTemporality);
return enabled
? aggregator.toMetricData(
resource, instrumentationScopeInfo, metricDescriptor, result, aggregationTemporality)
: EmptyMetricData.getInstance();
}

private Collection<T> collectWithDeltaAggregationTemporality() {
Expand Down Expand Up @@ -310,7 +319,7 @@ private Collection<T> collectWithCumulativeAggregationTemporality() {
}

@Override
public boolean isEmpty() {
return aggregator == Aggregator.drop();
public void setEnabled(boolean enabled) {
this.enabled = enabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,15 @@ public final class DefaultSynchronousMetricStorage<T extends PointData, U extend
private final ConcurrentLinkedQueue<AggregatorHandle<T, U>> aggregatorHandlePool =
new ConcurrentLinkedQueue<>();

private boolean enabled;

DefaultSynchronousMetricStorage(
RegisteredReader registeredReader,
MetricDescriptor metricDescriptor,
Aggregator<T, U> aggregator,
AttributesProcessor attributesProcessor,
int maxCardinality) {
int maxCardinality,
boolean enabled) {
this.registeredReader = registeredReader;
this.metricDescriptor = metricDescriptor;
this.aggregationTemporality =
Expand All @@ -90,6 +93,7 @@ public final class DefaultSynchronousMetricStorage<T extends PointData, U extend
this.attributesProcessor = attributesProcessor;
this.maxCardinality = maxCardinality - 1;
this.memoryMode = registeredReader.getReader().getMemoryMode();
this.enabled = enabled;
}

// Visible for testing
Expand All @@ -99,6 +103,9 @@ Queue<AggregatorHandle<T, U>> getAggregatorHandlePool() {

@Override
public void recordLong(long value, Attributes attributes, Context context) {
if (!enabled) {
return;
}
AggregatorHolder<T, U> aggregatorHolder = getHolderForRecord();
try {
AggregatorHandle<T, U> handle =
Expand All @@ -111,6 +118,9 @@ public void recordLong(long value, Attributes attributes, Context context) {

@Override
public void recordDouble(double value, Attributes attributes, Context context) {
if (!enabled) {
return;
}
if (Double.isNaN(value)) {
logger.log(
Level.FINE,
Expand All @@ -131,9 +141,14 @@ public void recordDouble(double value, Attributes attributes, Context context) {
}
}

@Override
public void setEnabled(boolean enabled) {
this.enabled = enabled;
}

@Override
public boolean isEnabled() {
return true;
return enabled;
}

/**
Expand Down Expand Up @@ -299,7 +314,7 @@ public MetricData collect(
previousCollectionAggregatorHandles = aggregatorHandles;
}

if (points.isEmpty()) {
if (points.isEmpty() || !enabled) {
return EmptyMetricData.getInstance();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,9 @@ public void recordDouble(double value, Attributes attributes, Context context) {
public boolean isEnabled() {
return false;
}

@Override
public void setEnabled(boolean enabled) {
// do nothing
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,5 @@ MetricData collect(
long startEpochNanos,
long epochNanos);

/**
* Determines whether this storage is an empty metric storage.
*
* <p>Uses the reference comparison since {@link EmptyMetricStorage} is singleton.
*
* @return true if is empty.
*/
default boolean isEmpty() {
return this == EmptyMetricStorage.INSTANCE;
}
void setEnabled(boolean enabled);
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ static <T extends PointData, U extends ExemplarData> SynchronousMetricStorage cr
RegisteredReader registeredReader,
RegisteredView registeredView,
InstrumentDescriptor instrumentDescriptor,
ExemplarFilter exemplarFilter) {
ExemplarFilter exemplarFilter,
boolean enabled) {
View view = registeredView.getView();
MetricDescriptor metricDescriptor =
MetricDescriptor.create(view, registeredView.getViewSourceInfo(), instrumentDescriptor);
Expand All @@ -57,6 +58,7 @@ static <T extends PointData, U extends ExemplarData> SynchronousMetricStorage cr
metricDescriptor,
aggregator,
registeredView.getViewAttributesProcessor(),
registeredView.getCardinalityLimit());
registeredView.getCardinalityLimit(),
enabled);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
import io.opentelemetry.context.Scope;
import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.internal.ScopeConfigurator;
import io.opentelemetry.sdk.metrics.data.MetricData;
import io.opentelemetry.sdk.metrics.export.MetricReader;
import io.opentelemetry.sdk.metrics.internal.MeterConfig;
import io.opentelemetry.sdk.metrics.internal.SdkMeterProviderUtil;
import io.opentelemetry.sdk.metrics.internal.view.ViewRegistry;
import io.opentelemetry.sdk.resources.Resource;
Expand All @@ -47,6 +49,7 @@
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand Down Expand Up @@ -1030,6 +1033,34 @@ void resetForTest() {
sum -> sum.isCumulative().hasPointsSatisfying(point -> point.hasValue(1))));
}

private static ScopeConfigurator<MeterConfig> flipConfigurator(boolean enabled) {
return scopeInfo -> enabled ? MeterConfig.disabled() : MeterConfig.enabled();
}

@Test
void propagatesEnablementToMeterDirectly() {
SdkMeterProvider meterProvider =
SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build();
SdkMeter meter = (SdkMeter) meterProvider.get("test");
boolean isEnabled = meter.isMeterEnabled();

meterProvider.setMeterConfigurator(flipConfigurator(isEnabled));

Assertions.assertThat(meter.isMeterEnabled()).isEqualTo(!isEnabled);
}

@Test
void propagatesEnablementToMeterByUtil() {
SdkMeterProvider sdkMeterProvider =
SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build();
SdkMeter sdkMeter = (SdkMeter) sdkMeterProvider.get("test");
boolean isEnabled = sdkMeter.isMeterEnabled();

SdkMeterProviderUtil.setMeterConfigurator(sdkMeterProvider, flipConfigurator(isEnabled));

Assertions.assertThat(sdkMeter.isMeterEnabled()).isEqualTo(!isEnabled);
}

private static void registerViewForAllTypes(
SdkMeterProviderBuilder meterProviderBuilder, Aggregation aggregation) {
for (InstrumentType instrumentType : InstrumentType.values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
import io.opentelemetry.sdk.metrics.internal.MeterConfig;
import io.opentelemetry.sdk.metrics.internal.state.MetricStorageRegistry;
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
import java.util.Locale;
Expand Down Expand Up @@ -481,4 +482,16 @@ void stringRepresentation() {
+ "attributes={}"
+ "}}");
}

@Test
void updateEnabled() {
SdkMeterProvider sdkMeterProvider =
SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build();
SdkMeter meter = (SdkMeter) sdkMeterProvider.get("test");

meter.updateMeterConfig(MeterConfig.disabled());
assertThat(meter.isMeterEnabled()).isFalse();
meter.updateMeterConfig(MeterConfig.enabled());
assertThat(meter.isMeterEnabled()).isTrue();
}
}
Loading
Loading