From d08a1a9c5ce20ad4d06a15d25ef9d4389e0b22c1 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Tue, 23 Jun 2020 22:35:12 +0300 Subject: [PATCH] When loading exporter factories via SPI from an external jar, look only inside that jar, and not any parent classloader. (#569) --- .../auto/tooling/ExporterClassLoader.java | 4 +- .../tooling/ExporterClassLoaderTest.groovy | 144 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 agent-tooling/src/test/groovy/io/opentelemetry/auto/tooling/ExporterClassLoaderTest.groovy diff --git a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/ExporterClassLoader.java b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/ExporterClassLoader.java index 16a76f52b8..2dad4519dc 100644 --- a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/ExporterClassLoader.java +++ b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/ExporterClassLoader.java @@ -68,7 +68,9 @@ public class ExporterClassLoader extends URLClassLoader { // A small hack to prevent other exporters from being loaded by this classloader if they // should happen to appear on the classpath. if (name.equals( - "META-INF/services/io.opentelemetry.sdk.contrib.auto.config.SpanExporterFactory")) { + "META-INF/services/io.opentelemetry.sdk.contrib.auto.config.SpanExporterFactory") + || name.equals( + "META-INF/services/io.opentelemetry.sdk.contrib.auto.config.MetricExporterFactory")) { return findResources(name); } return super.getResources(name); diff --git a/agent-tooling/src/test/groovy/io/opentelemetry/auto/tooling/ExporterClassLoaderTest.groovy b/agent-tooling/src/test/groovy/io/opentelemetry/auto/tooling/ExporterClassLoaderTest.groovy new file mode 100644 index 0000000000..92cd1ba9f6 --- /dev/null +++ b/agent-tooling/src/test/groovy/io/opentelemetry/auto/tooling/ExporterClassLoaderTest.groovy @@ -0,0 +1,144 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.opentelemetry.auto.tooling + + +import io.opentelemetry.sdk.contrib.auto.config.Config +import io.opentelemetry.sdk.contrib.auto.config.MetricExporterFactory +import io.opentelemetry.sdk.contrib.auto.config.SpanExporterFactory +import io.opentelemetry.sdk.metrics.export.MetricExporter +import io.opentelemetry.sdk.trace.export.SpanExporter +import java.nio.charset.StandardCharsets +import java.util.jar.JarEntry +import java.util.jar.JarOutputStream +import spock.lang.Specification + +class ExporterClassLoaderTest extends Specification { + + // Verifies https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/542 + def "does not look in parent classloader for metric exporters"() { + setup: + def parentClassloader = new URLClassLoader([createJarWithClasses(MetricExporterFactoryParent)] as URL[]) + def childClassloader = new ExporterClassLoader(createJarWithClasses(MetricExporterFactoryChild), parentClassloader) + + when: + ServiceLoader serviceLoader = ServiceLoader.load(MetricExporterFactory, childClassloader) + + then: + serviceLoader.size() == 1 + } + + def "does not look in parent classloader for span exporters"() { + setup: + def parentClassloader = new URLClassLoader([createJarWithClasses(SpanExporterFactoryParent)] as URL[]) + def childClassloader = new ExporterClassLoader(createJarWithClasses(SpanExporterFactoryChild), parentClassloader) + + when: + ServiceLoader serviceLoader = ServiceLoader.load(SpanExporterFactory, childClassloader) + + then: + serviceLoader.size() == 1 + } + + static class MetricExporterFactoryParent implements MetricExporterFactory { + + @Override + MetricExporter fromConfig(Config config) { + return null + } + } + + static class MetricExporterFactoryChild implements MetricExporterFactory { + + @Override + MetricExporter fromConfig(Config config) { + return null + } + } + + static class SpanExporterFactoryParent implements SpanExporterFactory { + + @Override + SpanExporter fromConfig(Config config) { + return null + } + } + + static class SpanExporterFactoryChild implements SpanExporterFactory { + + @Override + SpanExporter fromConfig(Config config) { + return null + } + } + + static URL createJarWithClasses(final Class... classes) + throws IOException { + final File tmpJar = File.createTempFile(UUID.randomUUID().toString() + "-", ".jar") + tmpJar.deleteOnExit() + + final JarOutputStream target = new JarOutputStream(new FileOutputStream(tmpJar)) + for (final Class clazz : classes) { + addToJar(clazz, clazz.getInterfaces()[0], target) + } + target.close() + + return tmpJar.toURI().toURL() + } + + //This is mostly copy-pasted from IntegrationTestUtils, but we need to save service files as well + private static void addToJar(final Class clazz, final Class serviceInterface, final JarOutputStream jarOutputStream) + throws IOException { + String resourceName = getResourceName(clazz.getName()) + + ClassLoader loader = clazz.getClassLoader() + if (null == loader) { + // bootstrap resources can be fetched through the system loader + loader = ClassLoader.getSystemClassLoader() + } + + InputStream inputStream = null + try { + final JarEntry entry = new JarEntry(resourceName) + jarOutputStream.putNextEntry(entry) + inputStream = loader.getResourceAsStream(resourceName) + + final byte[] buffer = new byte[1024] + while (true) { + final int count = inputStream.read(buffer) + if (count == -1) { + break + } + jarOutputStream.write(buffer, 0, count) + } + jarOutputStream.closeEntry() + + final JarEntry serviceEntry = new JarEntry("META-INF/services/" + serviceInterface.getName()) + jarOutputStream.putNextEntry(serviceEntry) + jarOutputStream.write(clazz.getName().getBytes(StandardCharsets.UTF_8)) + jarOutputStream.closeEntry() + } finally { + if (inputStream != null) { + inputStream.close() + } + } + } + + /** com.foo.Bar -> com/foo/Bar.class */ + private static String getResourceName(final String className) { + return className.replace('.', '/') + ".class" + } +}