-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix autoconfigure flaky tests #47528
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses flaky test failures by gracefully handling temporary directory creation failures in the OpenTelemetry autoconfigure tests. When a temporary directory cannot be created (which can occur intermittently in CI environments), tests now return Optional.empty() instead of throwing an exception.
Key Changes
- Modified
createOpenTelemetrySdk()inTestUtilsto returnOptional<OpenTelemetrySdk>instead ofOpenTelemetrySdkdirectly - Added exception handling to catch
ConfigurationExceptioncaused by directory creation failures - Introduced a public constant
UNABLE_TO_CREATE_DIRECTORYinTempDirsfor consistent error message matching - Updated all test methods calling
createOpenTelemetrySdk()to handle the Optional return type
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
TestUtils.java |
Changed return type to Optional<OpenTelemetrySdk> and added exception handling for temporary directory creation failures |
TempDirs.java |
Introduced public constant for error message to enable test code to detect directory creation failures |
EventHubsExporterIntegrationTest.java |
Updated two test methods to handle Optional return and skip when directory creation fails |
AzureMonitorStatsbeatTest.java |
Updated two test methods to handle Optional return and skip when directory creation fails |
AzureMonitorExportersEndToEndTest.java |
Updated five test methods to handle Optional return and skip when directory creation fails |
AppConfigurationExporterIntegrationTest.java |
Updated two test methods to handle Optional return and skip when directory creation fails |
.../test/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/utils/TestUtils.java
Show resolved
Hide resolved
...st/java/com/azure/monitor/opentelemetry/autoconfigure/AzureMonitorExportersEndToEndTest.java
Show resolved
Hide resolved
...st/java/com/azure/monitor/opentelemetry/autoconfigure/AzureMonitorExportersEndToEndTest.java
Show resolved
Hide resolved
...st/java/com/azure/monitor/opentelemetry/autoconfigure/AzureMonitorExportersEndToEndTest.java
Show resolved
Hide resolved
...st/java/com/azure/monitor/opentelemetry/autoconfigure/AzureMonitorExportersEndToEndTest.java
Show resolved
Hide resolved
...c/main/java/com/azure/monitor/opentelemetry/autoconfigure/implementation/utils/TempDirs.java
Show resolved
Hide resolved
...st/java/com/azure/monitor/opentelemetry/autoconfigure/AzureMonitorExportersEndToEndTest.java
Show resolved
Hide resolved
...a/com/azure/monitor/opentelemetry/autoconfigure/AppConfigurationExporterIntegrationTest.java
Show resolved
Hide resolved
...est/java/com/azure/monitor/opentelemetry/autoconfigure/EventHubsExporterIntegrationTest.java
Show resolved
Hide resolved
...est/java/com/azure/monitor/opentelemetry/autoconfigure/EventHubsExporterIntegrationTest.java
Show resolved
Hide resolved
|
I understand the intent around handling environments where temp directories cannot be created. One concern to call out: When tests do an early return like: if (!optionalOpenTelemetry.isPresent()) {
return;
}they are still reported as passed rather than skipped in CI. This can make it hard to distinguish real test success from environmental conditions, and may mask issues if these situations occur more often than expected. Also worth thinking about whether the underlying temp-directory creation issue indicates an environment or infrastructure problem rather than something tests should silently absorb. If this fallback path is expected to occur in normal scenarios, some form of visibility (even lightweight logging) might help future debugging. Just sharing these thoughts, happy to hear the context behind the current approach. |
|
Have we checked with Azure SDK folks to see if there's a better way to handle temp directories in the build? |
|
It might not be related to an infra issue. |
Avoids tests failing if a temporary directory cannot be created