Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-detect service name based on the jar name #6817

Merged
merged 4 commits into from
Oct 23, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions instrumentation/resources/library/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
plugins {
id("otel.library-instrumentation")
id("otel.animalsniffer-conventions")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK SPI doesn't work on android anyway, so we can get rid of this plugin.

}

val mrJarVersions = listOf(11)
val mrJarVersions = listOf(9, 11)

dependencies {
implementation("io.opentelemetry:opentelemetry-sdk-common")
implementation("io.opentelemetry:opentelemetry-semconv")
implementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")

compileOnly("org.codehaus.mojo:animal-sniffer-annotations")

annotationProcessor("com.google.auto.service:auto-service")
compileOnly("com.google.auto.service:auto-service-annotations")
testImplementation("com.google.auto.service:auto-service-annotations")

testImplementation("org.junit.jupiter:junit-jupiter-api")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Stream;
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;

/** Factory for {@link Resource} retrieving Container ID information. */
public final class ContainerResource {
Expand All @@ -25,7 +24,6 @@ public final class ContainerResource {
private static final String UNIQUE_HOST_NAME_FILE_NAME = "/proc/self/cgroup";
private static final Resource INSTANCE = buildSingleton(UNIQUE_HOST_NAME_FILE_NAME);

@IgnoreJRERequirement
private static Resource buildSingleton(String uniqueHostNameFileName) {
// can't initialize this statically without running afoul of animalSniffer on paths
return buildResource(Paths.get(uniqueHostNameFileName));
Expand All @@ -52,7 +50,6 @@ public static Resource get() {
*
* @return containerId
*/
@IgnoreJRERequirement
private static Optional<String> extractContainerId(Path cgroupFilePath) {
if (!Files.exists(cgroupFilePath) || !Files.isReadable(cgroupFilePath)) {
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.resources;

import static java.util.logging.Level.FINE;

import com.google.auto.service.AutoService;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider;
import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.logging.Logger;
import javax.annotation.Nullable;

/** A {@link ResourceProvider} that will attempt to guess the application name from the jar name. */
@AutoService(ResourceProvider.class)
public final class JarServiceNameProvider implements ConditionalResourceProvider {

private static final Logger logger = Logger.getLogger(JarServiceNameProvider.class.getName());

private final Supplier<String[]> getProcessHandleArguments;
private final Function<String, String> getSystemProperty;
private final Predicate<Path> fileExists;

@SuppressWarnings("unused") // SPI
public JarServiceNameProvider() {
this(ProcessArguments::getProcessArguments, System::getProperty, Files::isRegularFile);
}

// visible for tests
JarServiceNameProvider(
Supplier<String[]> getProcessHandleArguments,
Function<String, String> getSystemProperty,
Predicate<Path> fileExists) {
this.getProcessHandleArguments = getProcessHandleArguments;
this.getSystemProperty = getSystemProperty;
this.fileExists = fileExists;
}

@Override
public Resource createResource(ConfigProperties config) {
Path jarPath = getJarPathFromProcessHandle();
if (jarPath == null) {
jarPath = getJarPathFromSunCommandLine();
}
if (jarPath == null) {
return Resource.empty();
}
String serviceName = getServiceName(jarPath);
logger.log(FINE, "Auto-detected service name from the jar file name: {0}", serviceName);
return Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, serviceName));
}

@Override
public boolean shouldApply(ConfigProperties config, Resource existing) {
String serviceName = config.getString("otel.service.name");
Map<String, String> resourceAttributes = config.getMap("otel.resource.attributes");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these needed b/c this resource provider can't run after the built-in resource provider?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- the EnvironmentResource always runs last, as it's not a ResourceProvider, it's just called after all of the providers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think this is changeable, to make otel.service.name and otel.resource.attributes be real resource detectors, so that they can be leveraged as part of shouldApply (cc @jack-berg)

(I think this is a perfectly good workaround tho, and hopefully it's not a common use case)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think this is changeable, to make otel.service.name and otel.resource.attributes be real resource detectors, so that they can be leveraged as part of shouldApply

This idea has definitely cross my mine. I've also considered taking the idea further and modeling all the exporter configuration as Configurable{Signal}ExporterProviders. I think it could result in more module / testable code, and potentially have some nice interactions with a future file based configuration scheme.

return serviceName == null
&& !resourceAttributes.containsKey(ResourceAttributes.SERVICE_NAME.getKey())
&& "unknown_service:java".equals(existing.getAttribute(ResourceAttributes.SERVICE_NAME));
}

@Nullable
private Path getJarPathFromProcessHandle() {
String[] javaArgs = getProcessHandleArguments.get();
for (int i = 0; i < javaArgs.length; ++i) {
if ("-jar".equals(javaArgs[i]) && (i < javaArgs.length - 1)) {
return Paths.get(javaArgs[i + 1]);
}
}
return null;
}

@Nullable
private Path getJarPathFromSunCommandLine() {
// the jar file is the first argument in the command line string
String programArguments = getSystemProperty.apply("sun.java.command");
if (programArguments == null) {
return null;
}

// Take the path until the first space. If the path doesn't exist extend it up to the next
// space. Repeat until a path that exists is found or input runs out.
int next = 0;
while (true) {
int nextSpace = programArguments.indexOf(' ', next);
if (nextSpace == -1) {
Path candidate = Paths.get(programArguments);
return fileExists.test(candidate) ? candidate : null;
}
Path candidate = Paths.get(programArguments.substring(0, nextSpace));
next = nextSpace + 1;
if (fileExists.test(candidate)) {
return candidate;
}
}
}

private static String getServiceName(Path jarPath) {
String jarName = jarPath.getFileName().toString();
int dotIndex = jarName.lastIndexOf(".");
return dotIndex == -1 ? jarName : jarName.substring(0, dotIndex);
}

@Override
public int order() {
// make it run later than the SpringBootServiceNameGuesser
return 1000;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.resources;

final class ProcessArguments {

static String[] getProcessArguments() {
return new String[0];
}

private ProcessArguments() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@
package io.opentelemetry.instrumentation.resources;

import java.lang.management.ManagementFactory;
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;

final class ProcessPid {

private ProcessPid() {}

@IgnoreJRERequirement
static long getPid() {
// While this is not strictly defined, almost all commonly used JVMs format this as
// pid@hostname.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import java.io.File;
import java.lang.management.ManagementFactory;
import java.lang.management.RuntimeMXBean;
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;

/** Factory of a {@link Resource} which provides information about the current running process. */
public final class ProcessResource {
Expand All @@ -38,7 +37,6 @@ static Resource buildResource() {
}
}

@IgnoreJRERequirement
private static Resource doBuildResource() {
AttributesBuilder attributes = Attributes.builder();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@
package io.opentelemetry.instrumentation.resources;

import java.lang.management.ManagementFactory;
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;

final class ProcessPid {

private ProcessPid() {}

@IgnoreJRERequirement
static long getPid() {
return ManagementFactory.getRuntimeMXBean().getPid();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.resources;

final class ProcessArguments {

static String[] getProcessArguments() {
return ProcessHandle.current().info().arguments().orElseGet(() -> new String[0]);
}
Comment on lines +10 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leveraging the module system here is sooooooo much nicer than the reflection in the original version. ❤️


private ProcessArguments() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.resources;

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static org.junit.jupiter.params.provider.Arguments.arguments;

import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
import java.nio.file.Path;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.ArgumentsProvider;
import org.junit.jupiter.params.provider.ArgumentsSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class JarServiceNameProviderTest {

@Mock ConfigProperties config;

@Test
void createResource_empty() {
JarServiceNameProvider serviceNameProvider =
new JarServiceNameProvider(
() -> new String[0], prop -> null, JarServiceNameProviderTest::failPath);

Resource resource = serviceNameProvider.createResource(config);

assertThat(resource.getAttributes()).isEmpty();
}

@Test
void createResource_noJarFileInArgs() {
String[] args = new String[] {"-Dtest=42", "-Xmx666m", "-jar"};
JarServiceNameProvider serviceNameProvider =
new JarServiceNameProvider(() -> args, prop -> null, JarServiceNameProviderTest::failPath);

Resource resource = serviceNameProvider.createResource(config);

assertThat(resource.getAttributes()).isEmpty();
}

@Test
void createResource_processHandleJar() {
String[] args =
new String[] {"-Dtest=42", "-Xmx666m", "-jar", "/path/to/app/my-service.jar", "abc", "def"};
JarServiceNameProvider serviceNameProvider =
new JarServiceNameProvider(() -> args, prop -> null, JarServiceNameProviderTest::failPath);

Resource resource = serviceNameProvider.createResource(config);

assertThat(resource.getAttributes())
.hasSize(1)
.containsEntry(ResourceAttributes.SERVICE_NAME, "my-service");
}

@Test
void createResource_processHandleJarWithoutExtension() {
String[] args = new String[] {"-Dtest=42", "-Xmx666m", "-jar", "/path/to/app/my-service"};
JarServiceNameProvider serviceNameProvider =
new JarServiceNameProvider(() -> args, prop -> null, JarServiceNameProviderTest::failPath);

Resource resource = serviceNameProvider.createResource(config);

assertThat(resource.getAttributes())
.hasSize(1)
.containsEntry(ResourceAttributes.SERVICE_NAME, "my-service");
}

@ParameterizedTest
@ArgumentsSource(SunCommandLineProvider.class)
void createResource_sunCommandLine(String commandLine, String jarPath) {
Function<String, String> getProperty =
key -> "sun.java.command".equals(key) ? commandLine : null;
Predicate<Path> fileExists = file -> jarPath.equals(file.toString());

JarServiceNameProvider serviceNameProvider =
new JarServiceNameProvider(() -> new String[0], getProperty, fileExists);

Resource resource = serviceNameProvider.createResource(config);

assertThat(resource.getAttributes())
.hasSize(1)
.containsEntry(ResourceAttributes.SERVICE_NAME, "my-service");
}

static final class SunCommandLineProvider implements ArgumentsProvider {

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
return Stream.of(
arguments("/path/to/my-service.jar", "/path/to/my-service.jar"),
arguments(
"/path to app/with spaces/my-service.jar 1 2 3",
"/path to app/with spaces/my-service.jar"),
arguments(
"/path to app/with spaces/my-service 1 2 3", "/path to app/with spaces/my-service"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a little surprising to me because it doesn't actually have .jar in any argument names. Since we're not looking for .jar, how are we able to confidently assume that the file is my-service and not my-service 1 2 3?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what the second arg is for - the service detector checks whether a particular path (a substring of the sun.java.command) is a regular file (not a directory; a leaf in the file system graph).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying this out, and these tests fail on Windows, because later on the Path gets normalized with \, it's ok to merge as-is though (I need to try again to get the github actions running on windows)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I completely didn't think of this. Great point, I've changed this test to use Path instead, hope that helps 🤞

}
}

private static boolean failPath(Path file) {
throw new AssertionError("Unexpected call to Files.isRegularFile()");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.instrumentation.spring.resources;

import static java.util.logging.Level.FINE;

import com.google.auto.service.AutoService;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider;
Expand Down Expand Up @@ -90,7 +92,7 @@ public Resource createResource(ConfigProperties config) {
.findFirst()
.map(
serviceName -> {
logger.log(Level.FINER, "Guessed Spring Boot service name: {0}", serviceName);
logger.log(FINE, "Auto-detected Spring Boot service name: {0}", serviceName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fancy phrasing is fancy. 🎩

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧐

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted the message to be very similar in all of these service.name auto-detectors -- thus this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like auto-detected > guessed 😄

EDIT: rename class SpringBootServiceNameDetector?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed both classes to ...Detector

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like auto-detected > guessed 😄

I mean, it's more official-sounding for sure. But let's be real, it's only a heuristic, and it's guessing. 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh "Heuristic" is another good fancy word 😂. Guess sounds like it's sorta random.

return Resource.builder().put(ResourceAttributes.SERVICE_NAME, serviceName).build();
})
.orElseGet(Resource::empty);
Expand Down