Add setLoggerConfigurator support to LoggerProvider
#7332
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7332 +/- ##
============================================
- Coverage 89.85% 89.85% -0.01%
- Complexity 6890 6895 +5
============================================
Files 786 786
Lines 20772 20789 +17
Branches 2025 2025
============================================
+ Hits 18665 18680 +15
- Misses 1464 1466 +2
Partials 643 643 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| @Override | ||
| public boolean isEnabled() { |
There was a problem hiding this comment.
What for? isEnabled is inherited from SdkLogger, am I missing something?
There was a problem hiding this comment.
Its a minor thing: ExtendedSdkLogger implemented ExtendedLogger, which is where the isEnabled() API is defined.
If we only define it in SdkLogger, we can't keep the @Override annotation.
There was a problem hiding this comment.
Got it, fixed in 4d3871c
@jack-berg can you clarify why that's more difficult than something like this? diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java
index c0f147607..71a907f7f 100644
--- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java
+++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java
@@ -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;
SdkMeter(
MeterProviderSharedState meterProviderSharedState,
@@ -103,6 +104,10 @@ final class SdkMeter implements Meter {
this.meterEnabled = meterConfig.isEnabled();
}
+ void updateMeterConfig(MeterConfig meterConfig) {
+ meterEnabled = meterConfig.isEnabled();
+ }
+
// Visible for testing
InstrumentationScopeInfo getInstrumentationScopeInfo() {
return instrumentationScopeInfo;
diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java
index 30ae0b1da..a143aa0dd 100644
--- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java
+++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java
@@ -52,9 +52,19 @@ 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;
+
+ void setMeterConfigurator(ScopeConfigurator<MeterConfig> meterConfigurator) {
+ this.meterConfigurator = meterConfigurator;
+ this.registry
+ .getComponents()
+ .forEach(
+ sdkMeter ->
+ sdkMeter.updateMeterConfig(getMeterConfig(sdkMeter.getInstrumentationScopeInfo())));
+ }
+
/** Returns a new {@link SdkMeterProviderBuilder} for {@link SdkMeterProvider}. */
public static SdkMeterProviderBuilder builder() {
return new SdkMeterProviderBuilder();
diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/SdkMeterProviderUtil.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/SdkMeterProviderUtil.java
index 9fc690366..c324900ab 100644
--- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/SdkMeterProviderUtil.java
+++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/SdkMeterProviderUtil.java
@@ -49,6 +49,19 @@ public final class SdkMeterProviderUtil {
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,I've seen the remark in #7051, but I'm missing why the implementation above would not be enough 😅 |
Logs and spans are pretty much stateless: tracer and logger record spans and logs, and those flow through to processors and exporters. So its straight forward to change these pipelines to enable / disable tracers and loggers. Spans and logs start and stop flowing and all is good. Metrics are a whole different beast because they are stateful. Consider the case where you have an instrument from a meter which is initially disabled, and later enabled. The implementation needs to initially drop measurements, and then when the meter is enabled, it needs to initialize the metric storage and start recording / aggregating measurements. |
Fixes (partially) #7051