change exception logging when running in maven

[brendenehlers]

Fix for: #6827

Added try/catch around call to OpenTelemetrySdk.close() to catch NoClassDefFoundException. If the code is running in a maven environment (determined by presence of maven.home system property), then the exception is logged out as a warning. Otherwise, the exception is re-thrown.

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 89.84%. Comparing base (58acb53) to head (bd57cf1).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...nfigure/AutoConfiguredOpenTelemetrySdkBuilder.java	60.00%	2 Missing and 2 partials ⚠️

❌ Your patch check has failed because the patch coverage (60.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7336      +/-   ##
============================================
- Coverage     89.85%   89.84%   -0.02%     
- Complexity     6890     6897       +7     
============================================
  Files           786      786              
  Lines         20772    20798      +26     
  Branches       2025     2027       +2     
============================================
+ Hits          18665    18686      +21     
- Misses         1464     1468       +4     
- Partials        643      644       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[trask]

thanks!

have you tested this with the repro in #6827 (comment)?

[brendenehlers]

@trask originally built it there, but did a sanity check with the final product and realized that the logging dependencies might not be on the classpath when that hook is called resulting in the same error originally seen but with a different class.

while it might not be the best solution, i opted for System.out since it's guaranteed to be there when the code is run, so at least the end user will get the feedback even tho it won't be as tightly tied into the logging ecosystem. i figured it's at the end of the lifecycle so it should be fine.

[brendenehlers]

@trask idk what changed with the test but the codecov check isn't picking up the coverage for the provided test. the code it's saying isn't covered must be hit otherwise the test would fail, so i'm wondering if i've misconfigured something from the original successful pipeline?

[trask]

I think the goal here is not to (overly) alarm people, but also let them know the flush on shutdown did not occur

@cyrille-leclerc wdyt?

Suggested change

	System.out.printf("%s Flush failed during shutdown: %s%n", Level.WARNING, e);
	System.out.println("%s Flush failed during shutdown, which is a known issue when running OpenTelemetry inside maven (for details see https://github.com/open-telemetry/opentelemetry-java/issues/6827)");

[cyrille-leclerc]

I'm sorry, I forgot why this problem could occur in Maven builds.

The Maven OTel extension disables the OTel SDK shutdown hook in favor of explicitly shutting down the OTel SDK as part of the Maven plugin lifecycle. Is it cause by some tests like JUnit tests that run in process of the Maven build and launch an OTel SDK? Or is it happening when the Maven process is started with the OTel auto instrumentation agent?

As @trask highlighted, if there is a NoClassDefFoundError then some spans including the root spns are likely to not be exported which is a critical problem.

By the way, if this problem happens with Maven, shouldn't it also happen with the OTel Gradle Plugin?
FYI The Gradle OTel plugin also implements an explicit shutdown of the otel SDK here and there.

[trask]

@cyrille-leclerc it seems to have something to do with maven's class loader, you can find a simple repro here that doesn't even instantiate OpenTelemetry SDK: #6827 (comment)

[laurit]

My guess would be that this isn't maven specific. Probably by the time the shutdown hook runs the class loader that contains the sdk classes has been shut down and won't be able to load any more classes. For example the URLClassLoader provides a close method that could be used to trigger this. Perhaps the easiest way to repro would be to run on jdk8 (app loader is URLClassLoader) and manually call close on the app loader. Tomcat class loader also has similar behavior. With maven exec:java the class loader is shut down in https://github.com/mojohaus/exec-maven-plugin/blob/master/src/main/java/org/codehaus/mojo/exec/ExecJavaMojo.java#L334 and the class loader is a URLClassLoader https://github.com/mojohaus/exec-maven-plugin/blob/42bc369e89e51b474b25f03475b7c40af314a4a2/src/main/java/org/codehaus/mojo/exec/URLClassLoaderBuilder.java#L119
I probably would drop all the maven specific checks and just catch the NoClassDefFoundError. There should be a comment explaining under what circumstances the NoClassDefFoundError occurs. Sometimes issues like this can be fixed by ensuring that all classes needed for shutdown are loaded before the shutdown hook is run. Here I don't think this is really viable.

[brendenehlers]

So just to check my understanding, instead of handling it specifically for Maven I should change it to just catch the NoClassDefFoundError and log out the above message that @trask suggested?