opentelemetry-java-contrib icon indicating copy to clipboard operation
opentelemetry-java-contrib copied to clipboard

Fix flaky test

Open jackshirazi opened this issue 1 month ago • 5 comments

closes #2458

jackshirazi avatar Nov 24 '25 12:11 jackshirazi

@jackshirazi does the CI failure on this PR look like same or different issue than you were trying to fix here? https://scans.gradle.com/s/gfqf5jlzidn6q

trask avatar Dec 05 '25 16:12 trask

it's related. I left shutdown() instead of shutdownNow() which meant the previous test session can on occasion not finish before the next is started. I've added an interrupt for that too with the latest commit

jackshirazi avatar Dec 08 '25 15:12 jackshirazi

another failure in the latest SamplingProfilerTest: https://scans.gradle.com/s/2dbaqboab5fr6

trask avatar Dec 08 '25 16:12 trask

lol, this is not getting better

jackshirazi avatar Dec 08 '25 16:12 jackshirazi

@SylvainJuge and/or @JonasKunz I need a careful review, this has turned into significantly more extensive changes than I expected. there's a bunch of different things

  • the change to let the test framework to handle the temp dir - that's standard and should be solid
  • the changes to better manage interrupts, including a new lock - should be good, there's a chunk of them but I don't think there's anything that can be negative there
  • the changes to ensure profiler is stopped - should be fine
  • the change to handle empty jfr files - should be fine
  • the PostProcessing management changes - this worries me a little, especially the setProfilingSessionOngoing(postProcessingEnabled) to setProfilingSessionOngoing(true) change; it seems to be needed to avoid flaking, and doesn't have any bad effects as far as I can tell by working through code paths, but I need that to be considered carefully
  • the ProfilingActivationListener.ensureInitialized() is only in the test framework, so should be fine

jackshirazi avatar Dec 08 '25 23:12 jackshirazi