Provide a default Tracer and Meter for invalid instrumentation names

According to the spec, https://github.com/open-telemetry/opentelemetry-specification/blob/v0.6.0/specification/trace/api.md#tracerprovider-operations both a null and empty instrumentation name are invalid and a default implementation should be provided.

However, the spec is vague on what default means. See open-telemetry/opentelemetry-specification#586 (comment)

This change interprets "default" as meaning, "it should still work, but it's not ideal". Accordingly, a real Tracer or Meter implementation from the SDK is used. It meets the requirements of exporters and such by using a valid instrumentation name. It meets the requirements of application developers by producing valid tracing data. It also warns library and application developers by logging at the WARN level that an invalid value has been given.

The rational for this interpretation is that things should always work for the application developer. If an app developer doesn't provide a name, they should be chided, but they are instrumenting their own app, and likely don't care about the lack of name. If an app developer incorporates a library that doesn't specify the instrumentation name, the instrumentation should still be made available to application devs, but they should be able to turn if off (hence a valid name) and be made aware of something not being right (hence the logging) so they can report it to the library maintainers.

Fies open-telemetry#1879

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

@@ -19,7 +19,9 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.logging.Logger;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 /**
  * {@code Meter} provider implementation for {@link MeterProvider}.
@@ -29,6 +31,8 @@
  */
 public final class MeterSdkProvider implements MeterProvider {
 
+  private static final Logger logger = Logger.getLogger(MeterSdkProvider.class.getName());
+  private static final String defaultMeterName = "unknown";
   private final MeterSdkComponentRegistry registry;
   private final MetricProducer metricProducer;
 
@@ -41,11 +45,17 @@ private MeterSdkProvider(Clock clock, Resource resource) {
 
   @Override
   public MeterSdk get(String instrumentationName) {
-    return registry.get(instrumentationName);
+    return get(instrumentationName, null);
   }
 
   @Override
-  public MeterSdk get(String instrumentationName, String instrumentationVersion) {
+  public MeterSdk get(String instrumentationName, @Nullable String instrumentationVersion) {
+    // Per the spec, both null and empty are "invalid" and a "default" should be used. See commit
+    //   message for further details.
+    if (instrumentationName == null || instrumentationName.isEmpty()) {
+      logger.warning("Meter requested without instrumentation name.");
+      instrumentationName = defaultMeterName;
+    }
     return registry.get(instrumentationName, instrumentationVersion);
   }
 

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

@@ -101,4 +101,22 @@ void metricProducer_GetAllMetrics() {
                 Collections.singletonList(
                     LongPoint.create(testClock.now(), testClock.now(), Labels.empty(), 10))));
   }
+
+  @Test
+  void suppliesDefaultMeterForNullName() {
+    MeterSdk meter = meterProvider.get(null);
+    assertThat(meter.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+
+    meter = meterProvider.get(null, null);
+    assertThat(meter.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+  }
+
+  @Test
+  void suppliesDefaultMeterForEmptyName() {
+    MeterSdk meter = meterProvider.get("");
+    assertThat(meter.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+
+    meter = meterProvider.get("", "");
+    assertThat(meter.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+  }
 }

sdk/tracing/src/main/java/io/opentelemetry/sdk/trace/TracerSdkProvider.java

@@ -15,6 +15,7 @@
 import io.opentelemetry.sdk.internal.MillisClock;
 import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.trace.config.TraceConfig;
+import javax.annotation.Nullable;
 import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -28,6 +29,7 @@
  */
 public class TracerSdkProvider implements TracerProvider, TracerSdkManagement {
   private static final Logger logger = Logger.getLogger(TracerSdkProvider.class.getName());
+  private static final String defaultTracerName = "unknown";
   private final TracerSharedState sharedState;
   private final TracerSdkComponentRegistry tracerSdkComponentRegistry;
 
@@ -47,11 +49,17 @@ private TracerSdkProvider(Clock clock, IdGenerator idsGenerator, Resource resour
 
   @Override
   public Tracer get(String instrumentationName) {
-    return tracerSdkComponentRegistry.get(instrumentationName);
+    return get(instrumentationName, null);
   }
 
   @Override
-  public Tracer get(String instrumentationName, String instrumentationVersion) {
+  public Tracer get(String instrumentationName, @Nullable String instrumentationVersion) {
+    // Per the spec, both null and empty are "invalid" and a "default" should be used. See commit
+    //   message for further details.
+    if (instrumentationName == null || instrumentationName.isEmpty()) {
+      logger.warning("Tracer requested without instrumentation name.");
+      instrumentationName = defaultTracerName;
+    }
     return tracerSdkComponentRegistry.get(instrumentationName, instrumentationVersion);
   }
 

sdk/tracing/src/test/java/io/opentelemetry/sdk/trace/TracerSdkProviderTest.java

@@ -134,4 +134,22 @@ void returnNoopSpanAfterShutdown() {
     assertThat(span.getSpanContext().isValid()).isFalse();
     span.end();
   }
+
+  @Test
+  void suppliesDefaultTracerForNullName() {
+    TracerSdk tracer = (TracerSdk) tracerFactory.get(null);
+    assertThat(tracer.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+
+    tracer = (TracerSdk) tracerFactory.get(null, null);
+    assertThat(tracer.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+  }
+
+  @Test
+  void suppliesDefaultTracerForEmptyName() {
+    TracerSdk tracer = (TracerSdk) tracerFactory.get("");
+    assertThat(tracer.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+
+    tracer = (TracerSdk) tracerFactory.get("", "");
+    assertThat(tracer.getInstrumentationLibraryInfo().getName()).isEqualTo("unknown");
+  }
 }