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

Add Spring Boot service version finder / ResourceProvider #9480

Merged
merged 6 commits into from
Sep 21, 2023

Conversation

Hayanesh
Copy link
Contributor

@Hayanesh Hayanesh commented Sep 17, 2023

No description provided.

@Hayanesh Hayanesh requested a review from a team September 17, 2023 11:06
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 17, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@laurit
Copy link
Contributor

laurit commented Sep 21, 2023

Could you also add a smoke test for this similarly to

then: "service name is autodetected"
def serviceName = findResourceAttribute(traces, "service.name")
.map { it.stringValue }
.findAny()
serviceName.isPresent()
serviceName.get() == "otel-spring-test-app"
You'll also have to update the tag in
"ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-spring-boot:jdk$jdk-20230321.4484174638"
to 20230920.6251727205 (if I didn't mess anything up that version should contain the build-info.properties)

InputStream openClasspathResource(String filename, String location) {
String path = location + "/" + filename;
return classLoader.getResourceAsStream(path);
}
Copy link
Member

Choose a reason for hiding this comment

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

Now that the SystemHelper is used in two classes, WDYT about making it a top-level class?

}

// Exists for testing
public SpringBootServiceVersionDetector(SystemHelper system) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public SpringBootServiceVersionDetector(SystemHelper system) {
SpringBootServiceVersionDetector(SystemHelper system) {

}

private Optional<String> getServiceVersionFromBuildInfo() {
try (InputStream in = system.openClasspathResource("build-info.properties", "META-INF")) {
Copy link
Member

Choose a reason for hiding this comment

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

I think putting "META-INF" first makes more sense, since it's a directory that contains the build info file

Suggested change
try (InputStream in = system.openClasspathResource("build-info.properties", "META-INF")) {
try (InputStream in = system.openClasspathResource("META-INF", "build-info.properties")) {

Copy link
Member

@mateuszrzeszutek mateuszrzeszutek left a comment

Choose a reason for hiding this comment

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

Thanks @Hayanesh !

@mateuszrzeszutek mateuszrzeszutek merged commit e73118b into open-telemetry:main Sep 21, 2023
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discover service.version attribute from Spring Boot's build-info.properties file
3 participants