Fix reviews

Signed-off-by: Pavol Loffay <[email protected]>
open-telemetry · Jun 11, 2019 · 94a959a · 94a959a
1 parent 7fe173b
commit 94a959a
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 14 deletions.
diff --git a/api/src/main/java/io/opentelemetry/OpenTelemetry.java b/api/src/main/java/io/opentelemetry/OpenTelemetry.java
@@ -25,6 +25,8 @@
 import io.opentelemetry.trace.DefaultTracer;
 import io.opentelemetry.trace.Tracer;
 import io.opentelemetry.trace.spi.TracerProvider;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.ServiceLoader;
 import javax.annotation.Nullable;
 
@@ -90,26 +92,57 @@ public static Tagger getTagger() {
    * setting a system property with FQCN.
    *
    * @param providerClass a provider class
+   * @param classLoader class loader
    * @param <T> provider type
    * @return a provider or null if not found
    * @throws IllegalStateException if a specified provider is not found
    */
+  private static <T> T loadSpiAndCheckSpecified(
+      Class<T> providerClass, final ClassLoader classLoader) {
+    T provider = loadSpi(providerClass, classLoader);
+    String specifiedProvider = System.getProperty(providerClass.getName());
+    if (specifiedProvider != null
+        && !specifiedProvider.equals(provider != null ? provider.getClass().getName() : null)) {
+      throw new IllegalStateException(String.format("Tracer %s not found", specifiedProvider));
+    }
+    return provider;
+  }
+
+  /**
+   * Load provider class via {@link ServiceLoader}.
+   *
+   * @param providerClass a provider class
+   * @param classLoader class loader
+   * @param <T> provider type
+   */
   @Nullable
-  private static <T> T loadSpi(Class<T> providerClass, ClassLoader classLoader) {
-    if (classLoader.getParent() != null) {
-      return loadSpi(providerClass, classLoader.getParent());
+  private static <T> T loadSpi(Class<T> providerClass, final ClassLoader classLoader) {
+    if (classLoader == null) {
+      return null;
+    }
+
+    ClassLoader parentCl =
+        AccessController.doPrivileged(
+            new PrivilegedAction<ClassLoader>() {
+              @Override
+              public ClassLoader run() {
+                return classLoader.getParent();
+              }
+            });
+    // providers packaged in application runtimes have a higher priority
+    // than ones packaged in the application archives
+    T t = loadSpi(providerClass, parentCl);
+    if (t != null) {
+      return t;
     }
 
     String specifiedProvider = System.getProperty(providerClass.getName());
-    ServiceLoader<T> providers = ServiceLoader.load(providerClass);
+    ServiceLoader<T> providers = ServiceLoader.load(providerClass, classLoader);
     for (T provider : providers) {
       if (specifiedProvider == null || specifiedProvider.equals(provider.getClass().getName())) {
         return provider;
       }
     }
-    if (specifiedProvider != null) {
-      throw new IllegalStateException(String.format("Tracer %s not found", specifiedProvider));
-    }
     return null;
   }
 
@@ -126,8 +159,19 @@ private static OpenTelemetry getInstance() {
   }
 
   private OpenTelemetry() {
-    TracerProvider tracerProvider =
-        loadSpi(TracerProvider.class, Thread.currentThread().getContextClassLoader());
+    ClassLoader cl =
+        AccessController.doPrivileged(
+            new PrivilegedAction<ClassLoader>() {
+              @Override
+              public ClassLoader run() {
+                return Thread.currentThread().getContextClassLoader();
+              }
+            });
+    if (cl == null) {
+      cl = OpenTelemetry.class.getClassLoader();
+    }
+
+    TracerProvider tracerProvider = loadSpiAndCheckSpecified(TracerProvider.class, cl);
     this.tracerProvider =
         tracerProvider != null
             ? tracerProvider
@@ -137,8 +181,7 @@ public Tracer get() {
                 return DefaultTracer.getInstance();
               }
             };
-    MeterProvider meterProvider =
-        loadSpi(MeterProvider.class, Thread.currentThread().getContextClassLoader());
+    MeterProvider meterProvider = loadSpiAndCheckSpecified(MeterProvider.class, cl);
     this.meterProvider =
         meterProvider != null
             ? meterProvider
@@ -148,8 +191,7 @@ public Meter get() {
                 return DefaultMeter.getInstance();
               }
             };
-    TaggerProvider taggerProvider =
-        loadSpi(TaggerProvider.class, Thread.currentThread().getContextClassLoader());
+    TaggerProvider taggerProvider = loadSpiAndCheckSpecified(TaggerProvider.class, cl);
     this.taggerProvider =
         taggerProvider != null
             ? taggerProvider

diff --git a/api/src/test/java/io/opentelemetry/OpenTelemetryTest.java b/api/src/test/java/io/opentelemetry/OpenTelemetryTest.java
@@ -174,7 +174,7 @@ public void testTaggerNotFound() {
   }
 
   private static File createService(Class<?> service, Class<?>... impls) throws IOException {
-    URL location = Tracer.class.getProtectionDomain().getCodeSource().getLocation();
+    URL location = OpenTelemetryTest.class.getProtectionDomain().getCodeSource().getLocation();
     File file = new File(location.getPath() + "META-INF/services/" + service.getName());
     file.getParentFile().mkdirs();