Align SdkMeterProvider with SdkTracerProvider (#3792)

* Align SdkMeterProvider with SdkTracerProvider

* Use noop meter provider when metric exporter is none

api/metrics/src/main/java/io/opentelemetry/api/metrics/MeterProvider.java

@@ -58,8 +58,8 @@ public interface MeterProvider {
    */
   MeterBuilder meterBuilder(String instrumentationName);
 
-  /** Returns a MeterProvider that does nothing. */
+  /** Returns a no-op {@link MeterProvider} which provides meters which do not record or emit. */
-  public static MeterProvider noop() {
+  static MeterProvider noop() {
     return NoopMeterProvider.getInstance();
   }
 }

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java

@@ -44,8 +44,6 @@ final class MetricExporterConfiguration {
             "opentelemetry-exporter-logging");
         configureLoggingMetrics(config, sdkMeterProviderBuilder);
         return;
-      case "none":
-        return;
       default:
         MetricExporter spiExporter = configureSpiExporter(name, config);
         if (spiExporter == null) {

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/OpenTelemetrySdkAutoConfiguration.java

@@ -6,6 +6,8 @@
 package io.opentelemetry.sdk.autoconfigure;
 
 import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.MeterProvider;
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
@@ -103,12 +105,13 @@ public final class OpenTelemetrySdkAutoConfiguration {
     }
 
     String exporterName = config.getString("otel.metrics.exporter");
-    if (exporterName == null) {
+    if (exporterName == null || exporterName.equals("none")) {
-      exporterName = "none";
+      // In the event no exporters are configured set a noop exporter
+      GlobalMeterProvider.set(MeterProvider.noop());
+      return;
     }
     MetricExporterConfiguration.configureExporter(exporterName, config, meterProviderBuilder);
 
-    // In the event no exporters are configured, this returns a stubbed SdkMeterProvider.
     SdkMeterProvider meterProvider = meterProviderBuilder.buildAndRegisterGlobal();
 
     // Make sure metrics shut down when JVM shuts down.

sdk-extensions/autoconfigure/src/testInitializeRegistersGlobal/java/io/opentelemetry/sdk/autoconfigure/OpenTelemetrySdkAutoConfigurationTest.java

@@ -9,9 +9,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.internal.NoopMeterProvider;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
-import io.opentelemetry.sdk.metrics.SdkMeterProvider;
-import io.opentelemetry.sdk.metrics.testing.InMemoryMetricReader;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -20,6 +19,7 @@ class OpenTelemetrySdkAutoConfigurationTest {
   @BeforeEach
   void setUp() {
     GlobalOpenTelemetry.resetForTest();
+    GlobalMeterProvider.set(null);
   }
 
   @Test
@@ -45,8 +45,7 @@ class OpenTelemetrySdkAutoConfigurationTest {
     // OTEL_METRICS_EXPORTER=none so the metrics SDK should be completely disabled.
     // This is a bit of an odd test, so we just ensure that we don't have the same impl class as if
     // we instantiated an SDK with a reader.
-    assertThat(GlobalMeterProvider.get())
+    OpenTelemetrySdkAutoConfiguration.initialize();
-        .doesNotHaveSameClassAs(
+    assertThat(GlobalMeterProvider.get()).isInstanceOf(NoopMeterProvider.class);
-            SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build());
   }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/DefaultSdkMeterProvider.java

@@ -1,160 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.sdk.metrics;
-
-import io.opentelemetry.api.OpenTelemetry;
-import io.opentelemetry.api.metrics.MeterBuilder;
-import io.opentelemetry.sdk.common.Clock;
-import io.opentelemetry.sdk.common.CompletableResultCode;
-import io.opentelemetry.sdk.internal.ComponentRegistry;
-import io.opentelemetry.sdk.metrics.data.MetricData;
-import io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter;
-import io.opentelemetry.sdk.metrics.export.MetricProducer;
-import io.opentelemetry.sdk.metrics.export.MetricReader;
-import io.opentelemetry.sdk.metrics.export.MetricReaderFactory;
-import io.opentelemetry.sdk.metrics.internal.export.CollectionHandle;
-import io.opentelemetry.sdk.metrics.internal.export.CollectionInfo;
-import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
-import io.opentelemetry.sdk.metrics.internal.view.ViewRegistry;
-import io.opentelemetry.sdk.resources.Resource;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.function.Supplier;
-import java.util.logging.Logger;
-import javax.annotation.Nullable;
-
-/**
- * Default implementation for {@link SdkMeterProvider}.
- *
- * <p>This class is not intended to be used in application code and it is used only by {@link
- * OpenTelemetry}.
- */
-final class DefaultSdkMeterProvider implements SdkMeterProvider {
-
-  private static final Logger LOGGER = Logger.getLogger(DefaultSdkMeterProvider.class.getName());
-  static final String DEFAULT_METER_NAME = "unknown";
-
-  private final ComponentRegistry<SdkMeter> registry;
-  private final MeterProviderSharedState sharedState;
-  private final Set<CollectionHandle> collectors;
-  private final Map<CollectionHandle, CollectionInfo> collectionInfoMap;
-  private final AtomicBoolean isClosed = new AtomicBoolean(false);
-  private final AtomicLong lastCollectionTimestamp;
-
-  // Minimum amount of time we allow between synchronous collections.
-  // This meant to reduce overhead when multiple exporters attempt to read metrics quickly.
-  // TODO: This should be configurable at the SDK level.
-  private static final long MINIMUM_COLLECTION_INTERVAL_NANOS = TimeUnit.MILLISECONDS.toNanos(100);
-
-  DefaultSdkMeterProvider(
-      List<MetricReaderFactory> readerFactories,
-      Clock clock,
-      Resource resource,
-      ViewRegistry viewRegistry,
-      ExemplarFilter exemplarSampler) {
-    this.sharedState =
-        MeterProviderSharedState.create(clock, resource, viewRegistry, exemplarSampler);
-    this.registry =
-        new ComponentRegistry<>(
-            instrumentationLibraryInfo -> new SdkMeter(sharedState, instrumentationLibraryInfo));
-    this.lastCollectionTimestamp =
-        new AtomicLong(clock.nanoTime() - MINIMUM_COLLECTION_INTERVAL_NANOS);
-
-    // Here we construct our own unique handle ids for this SDK.
-    // These are guaranteed to be unique per-reader for this SDK, and only this SDK.
-    // These are *only* mutated in our constructor, and safe to use concurrently after construction.
-    collectors = CollectionHandle.mutableSet();
-    collectionInfoMap = new HashMap<>();
-    Supplier<CollectionHandle> handleSupplier = CollectionHandle.createSupplier();
-    for (MetricReaderFactory readerFactory : readerFactories) {
-      CollectionHandle handle = handleSupplier.get();
-      // TODO: handle failure in creation or just crash?
-      MetricReader reader = readerFactory.apply(new LeasedMetricProducer(handle));
-      collectionInfoMap.put(handle, CollectionInfo.create(handle, collectors, reader));
-      collectors.add(handle);
-    }
-  }
-
-  @Override
-  public MeterBuilder meterBuilder(@Nullable String instrumentationName) {
-    if (instrumentationName == null || instrumentationName.isEmpty()) {
-      LOGGER.fine("Meter requested without instrumentation name.");
-      instrumentationName = DEFAULT_METER_NAME;
-    }
-    return new SdkMeterBuilder(registry, instrumentationName);
-  }
-
-  @Override
-  public CompletableResultCode forceFlush() {
-    List<CompletableResultCode> results = new ArrayList<>();
-    for (CollectionInfo collectionInfo : collectionInfoMap.values()) {
-      results.add(collectionInfo.getReader().shutdown());
-    }
-    return CompletableResultCode.ofAll(results);
-  }
-
-  @Override
-  public CompletableResultCode close() {
-    if (!isClosed.compareAndSet(false, true)) {
-      LOGGER.info("Multiple close calls");
-      return CompletableResultCode.ofSuccess();
-    }
-    List<CompletableResultCode> results = new ArrayList<>();
-    for (CollectionInfo info : collectionInfoMap.values()) {
-      results.add(info.getReader().shutdown());
-    }
-    return CompletableResultCode.ofAll(results);
-  }
-
-  @Override
-  public CompletableResultCode shutdown() {
-    return close();
-  }
-
-  /** Helper class to expose registered metric exports. */
-  private class LeasedMetricProducer implements MetricProducer {
-    private final CollectionHandle handle;
-
-    LeasedMetricProducer(CollectionHandle handle) {
-      this.handle = handle;
-    }
-
-    @Override
-    public Collection<MetricData> collectAllMetrics() {
-      Collection<SdkMeter> meters = registry.getComponents();
-      // Suppress too-frequent-collection.
-      long currentNanoTime = sharedState.getClock().nanoTime();
-      long pastNanoTime = lastCollectionTimestamp.get();
-      // It hasn't been long enough since the last collection.
-      boolean disableSynchronousCollection =
-          (currentNanoTime - pastNanoTime) < MINIMUM_COLLECTION_INTERVAL_NANOS;
-      // If we're not disabling metrics, write the current collection time.
-      // We don't care if this happens in more than one thread, suppression is optimistic, and the
-      // interval is small enough some jitter isn't important.
-      if (!disableSynchronousCollection) {
-        lastCollectionTimestamp.lazySet(currentNanoTime);
-      }
-
-      List<MetricData> result = new ArrayList<>(meters.size());
-      for (SdkMeter meter : meters) {
-        result.addAll(
-            meter.collectAll(
-                collectionInfoMap.get(handle),
-                sharedState.getClock().now(),
-                disableSynchronousCollection));
-      }
-      return Collections.unmodifiableCollection(result);
-    }
-  }
-}

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/NoopMeterProvider.java

@@ -0,0 +1,18 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.sdk.metrics;
+
+import io.opentelemetry.api.metrics.MeterBuilder;
+import io.opentelemetry.api.metrics.MeterProvider;
+
+/** Implementation of {@link MeterProvider} which does not record or emit metrics. */
+final class NoopMeterProvider implements MeterProvider {
+
+  @Override
+  public MeterBuilder meterBuilder(String instrumentationName) {
+    return MeterProvider.noop().meterBuilder(instrumentationName);
+  }
+}

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/NoopSdkMeterProvider.java

@@ -1,34 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.sdk.metrics;
-
-import io.opentelemetry.api.metrics.MeterBuilder;
-import io.opentelemetry.api.metrics.MeterProvider;
-import io.opentelemetry.sdk.common.CompletableResultCode;
-
-/** Implementation of SdkMeterProvider which does not collect metrics. */
-final class NoopSdkMeterProvider implements SdkMeterProvider {
-
-  @Override
-  public MeterBuilder meterBuilder(String instrumentationName) {
-    return MeterProvider.noop().meterBuilder(instrumentationName);
-  }
-
-  @Override
-  public CompletableResultCode forceFlush() {
-    return CompletableResultCode.ofSuccess();
-  }
-
-  @Override
-  public CompletableResultCode close() {
-    return CompletableResultCode.ofSuccess();
-  }
-
-  @Override
-  public CompletableResultCode shutdown() {
-    return close();
-  }
-}

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java

@@ -5,32 +5,167 @@
 
 package io.opentelemetry.sdk.metrics;
 
+import io.opentelemetry.api.metrics.MeterBuilder;
 import io.opentelemetry.api.metrics.MeterProvider;
+import io.opentelemetry.sdk.common.Clock;
 import io.opentelemetry.sdk.common.CompletableResultCode;
+import io.opentelemetry.sdk.internal.ComponentRegistry;
+import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter;
+import io.opentelemetry.sdk.metrics.export.MetricProducer;
+import io.opentelemetry.sdk.metrics.export.MetricReader;
+import io.opentelemetry.sdk.metrics.export.MetricReaderFactory;
+import io.opentelemetry.sdk.metrics.internal.export.CollectionHandle;
+import io.opentelemetry.sdk.metrics.internal.export.CollectionInfo;
+import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
+import io.opentelemetry.sdk.metrics.internal.view.ViewRegistry;
+import io.opentelemetry.sdk.resources.Resource;
+import java.io.Closeable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.function.Supplier;
+import java.util.logging.Logger;
+import javax.annotation.Nullable;
 
-/**
+/** SDK implementation for {@link MeterProvider}. */
- * {@code SdkMeterProvider} provides SDK extensions for {@link MeterProvider}.
+public final class SdkMeterProvider implements MeterProvider, Closeable {
- *
- * <p>This class is not intended to be used in application code and it is used only by {@link
- * io.opentelemetry.api.OpenTelemetry}.
- */
-public interface SdkMeterProvider extends MeterProvider {
 
-  /** Forces metric readers to immediately read metrics, if able. */
+  private static final Logger LOGGER = Logger.getLogger(SdkMeterProvider.class.getName());
-  CompletableResultCode forceFlush();
+  static final String DEFAULT_METER_NAME = "unknown";
 
-  /** Shuts down metric collection and all associated metric readers. */
+  private final ComponentRegistry<SdkMeter> registry;
-  CompletableResultCode close();
+  private final MeterProviderSharedState sharedState;
+  private final Map<CollectionHandle, CollectionInfo> collectionInfoMap;
+  private final AtomicBoolean isClosed = new AtomicBoolean(false);
+  private final AtomicLong lastCollectionTimestamp;
 
-  /** Shuts down metric collection and all associated metric readers. */
+  // Minimum amount of time we allow between synchronous collections.
-  CompletableResultCode shutdown();
+  // This meant to reduce overhead when multiple exporters attempt to read metrics quickly.
+  // TODO: This should be configurable at the SDK level.
+  private static final long MINIMUM_COLLECTION_INTERVAL_NANOS = TimeUnit.MILLISECONDS.toNanos(100);
 
   /**
    * Returns a new {@link SdkMeterProviderBuilder} for {@link SdkMeterProvider}.
    *
    * @return a new {@link SdkMeterProviderBuilder} for {@link SdkMeterProvider}.
    */
-  static SdkMeterProviderBuilder builder() {
+  public static SdkMeterProviderBuilder builder() {
     return new SdkMeterProviderBuilder();
   }
+
+  SdkMeterProvider(
+      List<MetricReaderFactory> readerFactories,
+      Clock clock,
+      Resource resource,
+      ViewRegistry viewRegistry,
+      ExemplarFilter exemplarSampler) {
+    this.sharedState =
+        MeterProviderSharedState.create(clock, resource, viewRegistry, exemplarSampler);
+    this.registry =
+        new ComponentRegistry<>(
+            instrumentationLibraryInfo -> new SdkMeter(sharedState, instrumentationLibraryInfo));
+    this.lastCollectionTimestamp =
+        new AtomicLong(clock.nanoTime() - MINIMUM_COLLECTION_INTERVAL_NANOS);
+
+    // Here we construct our own unique handle ids for this SDK.
+    // These are guaranteed to be unique per-reader for this SDK, and only this SDK.
+    // These are *only* mutated in our constructor, and safe to use concurrently after construction.
+    Set<CollectionHandle> collectors = CollectionHandle.mutableSet();
+    collectionInfoMap = new HashMap<>();
+    Supplier<CollectionHandle> handleSupplier = CollectionHandle.createSupplier();
+    for (MetricReaderFactory readerFactory : readerFactories) {
+      CollectionHandle handle = handleSupplier.get();
+      // TODO: handle failure in creation or just crash?
+      MetricReader reader = readerFactory.apply(new LeasedMetricProducer(handle));
+      collectionInfoMap.put(handle, CollectionInfo.create(handle, collectors, reader));
+      collectors.add(handle);
+    }
+  }
+
+  @Override
+  public MeterBuilder meterBuilder(@Nullable String instrumentationName) {
+    if (instrumentationName == null || instrumentationName.isEmpty()) {
+      LOGGER.fine("Meter requested without instrumentation name.");
+      instrumentationName = DEFAULT_METER_NAME;
+    }
+    return new SdkMeterBuilder(registry, instrumentationName);
+  }
+
+  /**
+   * Call {@link MetricReader#flush()} on all metric readers associated with this provider. The
+   * resulting {@link CompletableResultCode} completes when all complete.
+   */
+  public CompletableResultCode forceFlush() {
+    List<CompletableResultCode> results = new ArrayList<>();
+    for (CollectionInfo collectionInfo : collectionInfoMap.values()) {
+      results.add(collectionInfo.getReader().flush());
+    }
+    return CompletableResultCode.ofAll(results);
+  }
+
+  /**
+   * Shutdown the provider. Calls {@link MetricReader#shutdown()} on all metric readers associated
+   * with this provider. The resulting {@link CompletableResultCode} completes when all complete.
+   */
+  public CompletableResultCode shutdown() {
+    if (!isClosed.compareAndSet(false, true)) {
+      LOGGER.info("Multiple close calls");
+      return CompletableResultCode.ofSuccess();
+    }
+    List<CompletableResultCode> results = new ArrayList<>();
+    for (CollectionInfo info : collectionInfoMap.values()) {
+      results.add(info.getReader().shutdown());
+    }
+    return CompletableResultCode.ofAll(results);
+  }
+
+  /** Close the meter provider. Calls {@link #shutdown()} and blocks waiting for it to complete. */
+  @Override
+  public void close() {
+    shutdown().join(10, TimeUnit.SECONDS);
+  }
+
+  /** Helper class to expose registered metric exports. */
+  private class LeasedMetricProducer implements MetricProducer {
+    private final CollectionHandle handle;
+
+    LeasedMetricProducer(CollectionHandle handle) {
+      this.handle = handle;
+    }
+
+    @Override
+    public Collection<MetricData> collectAllMetrics() {
+      Collection<SdkMeter> meters = registry.getComponents();
+      // Suppress too-frequent-collection.
+      long currentNanoTime = sharedState.getClock().nanoTime();
+      long pastNanoTime = lastCollectionTimestamp.get();
+      // It hasn't been long enough since the last collection.
+      boolean disableSynchronousCollection =
+          (currentNanoTime - pastNanoTime) < MINIMUM_COLLECTION_INTERVAL_NANOS;
+      // If we're not disabling metrics, write the current collection time.
+      // We don't care if this happens in more than one thread, suppression is optimistic, and the
+      // interval is small enough some jitter isn't important.
+      if (!disableSynchronousCollection) {
+        lastCollectionTimestamp.lazySet(currentNanoTime);
+      }
+
+      List<MetricData> result = new ArrayList<>(meters.size());
+      for (SdkMeter meter : meters) {
+        result.addAll(
+            meter.collectAll(
+                collectionInfoMap.get(handle),
+                sharedState.getClock().now(),
+                disableSynchronousCollection));
+      }
+      return Collections.unmodifiableCollection(result);
+    }
+  }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProviderBuilder.java

@@ -134,11 +134,7 @@ public final class SdkMeterProviderBuilder {
    * @see GlobalMeterProvider
    */
   public SdkMeterProvider build() {
-    // If no exporters are configured, optimize by returning no-op implementation.
+    return new SdkMeterProvider(
-    if (metricReaders.isEmpty()) {
-      return new NoopSdkMeterProvider();
-    }
-    return new DefaultSdkMeterProvider(
         metricReaders, clock, resource, viewRegistryBuilder.build(), exemplarFilter);
   }
 }

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderBuilderTest.java

@@ -34,12 +34,4 @@ class SdkMeterProviderBuilderTest {
         .extracting("sharedState")
         .hasFieldOrPropertyWithValue("resource", Resource.getDefault());
   }
-
-  @Test
-  void stubsWithNoReaders() {
-    // We need a reader to have a resource.
-    SdkMeterProvider meterProvider = SdkMeterProvider.builder().build();
-
-    assertThat(meterProvider).isInstanceOf(NoopSdkMeterProvider.class);
-  }
 }

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterRegistryTest.java

@@ -137,21 +137,21 @@ class SdkMeterRegistryTest {
   void suppliesDefaultMeterForNullName() {
     SdkMeter meter = (SdkMeter) meterProvider.get(null);
     assertThat(meter.getInstrumentationLibraryInfo().getName())
-        .isEqualTo(DefaultSdkMeterProvider.DEFAULT_METER_NAME);
+        .isEqualTo(SdkMeterProvider.DEFAULT_METER_NAME);
 
     meter = (SdkMeter) meterProvider.meterBuilder(null).build();
     assertThat(meter.getInstrumentationLibraryInfo().getName())
-        .isEqualTo(DefaultSdkMeterProvider.DEFAULT_METER_NAME);
+        .isEqualTo(SdkMeterProvider.DEFAULT_METER_NAME);
   }
 
   @Test
   void suppliesDefaultMeterForEmptyName() {
     SdkMeter meter = (SdkMeter) meterProvider.get("");
     assertThat(meter.getInstrumentationLibraryInfo().getName())
-        .isEqualTo(DefaultSdkMeterProvider.DEFAULT_METER_NAME);
+        .isEqualTo(SdkMeterProvider.DEFAULT_METER_NAME);
 
     meter = (SdkMeter) meterProvider.meterBuilder("").build();
     assertThat(meter.getInstrumentationLibraryInfo().getName())
-        .isEqualTo(DefaultSdkMeterProvider.DEFAULT_METER_NAME);
+        .isEqualTo(SdkMeterProvider.DEFAULT_METER_NAME);
   }
 }