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

spring boot starter: add service.version detection, improve service.name detection #10457

Merged
merged 7 commits into from
Feb 21, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ dependencies {
compileOnly(project(":instrumentation-annotations"))

compileOnly(project(":instrumentation:resources:library"))
compileOnly(project(":instrumentation:spring:spring-boot-resources:library"))
annotationProcessor("com.google.auto.service:auto-service")
compileOnly("com.google.auto.service:auto-service-annotations")

Expand All @@ -61,6 +62,7 @@ dependencies {
testImplementation("io.opentelemetry:opentelemetry-sdk")
testImplementation("io.opentelemetry:opentelemetry-sdk-testing")
testImplementation(project(":instrumentation:resources:library"))
testImplementation(project(":instrumentation:spring:spring-boot-resources:library"))
testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
testImplementation("io.opentelemetry:opentelemetry-extension-annotations")
testImplementation("io.opentelemetry:opentelemetry-extension-trace-propagators")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import io.opentelemetry.instrumentation.resources.ProcessRuntimeResource;
import io.opentelemetry.instrumentation.resources.ProcessRuntimeResourceProvider;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import io.opentelemetry.instrumentation.spring.resources.SpringBootServiceNameDetector;
import io.opentelemetry.instrumentation.spring.resources.SpringBootServiceVersionDetector;
import io.opentelemetry.sdk.autoconfigure.internal.EnvironmentResourceProvider;
import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
Expand Down Expand Up @@ -46,6 +48,18 @@ public ResourceProvider otelDistroVersionResourceProvider() {
return new DistroVersionResourceProvider();
}

@Bean
@ConditionalOnClass(SpringBootServiceNameDetector.class)
public ResourceProvider otelSpringBootServiceNameResourceProvider() {
return new SpringBootServiceNameDetector();
}

@Bean
@ConditionalOnClass(SpringBootServiceVersionDetector.class)
public ResourceProvider otelSpringBootServiceVersionResourceProvider() {
return new SpringBootServiceVersionDetector();
}

@Bean
@ConditionalOnClass(OsResource.class)
public ResourceProvider otelOsResourceProvider() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
* <li>Check for --spring.application.name program argument (not jvm arg) via ProcessHandle
* <li>Check for --spring.application.name program argument via sun.java.command system property
* </ul>
*
* <p>Note: should not be used inside a spring application, where the spring.application.name is
* already available.
Comment on lines +48 to +50
Copy link
Member

Choose a reason for hiding this comment

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

I didn't understand this note?

Copy link
Member Author

Choose a reason for hiding this comment

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

In spring, there's a resource provider that uses the always correct, pre-computed value from the spring environment.

Copy link
Member

Choose a reason for hiding this comment

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

sorry, still not following

Copy link
Member Author

Choose a reason for hiding this comment

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

the spring boot starter uses SpringResourceProvider, which doesn't need to parse any application.yaml, etc. file, because when you run inside a sprig application, you can just ask the environment.

In this case, the lookup is here:

Copy link
Member

Choose a reason for hiding this comment

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

so SpringBootServiceNameDetector should only be used by the Java agent?

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, exactly.

The provider will actually notice if a name found using shouldApply - so it's just a note about the intended usage.

Copy link
Member

Choose a reason for hiding this comment

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

could SpringBootServiceNameDetector be moved to a (new) spring-boot-resources/javaagent to make this extra clear?

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've done this move in #10621 - because this

PR moves SystemHelper to the common resource project (for other reasons).
Hope this makes sense.

*/
@AutoService(ResourceProvider.class)
public class SpringBootServiceNameDetector implements ConditionalResourceProvider {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ dependencies {
api("org.springframework.boot:spring-boot-starter-aop:$springBootVersion")
api(project(":instrumentation:spring:spring-boot-autoconfigure"))
implementation(project(":instrumentation:resources:library"))
implementation(project(":instrumentation:spring:spring-boot-resources:library"))
api("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
api("io.opentelemetry:opentelemetry-api")
api("io.opentelemetry:opentelemetry-exporter-logging")
Expand Down
Loading