-
Notifications
You must be signed in to change notification settings - Fork 916
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
improve assertions by showing the last result on a timeout #13119
Conversation
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java
Outdated
Show resolved
Hide resolved
...common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java
Outdated
Show resolved
Hide resolved
@steverao can you check again? |
@@ -125,12 +126,9 @@ public List<List<SpanData>> waitForTraces(int numberOfTraces) { | |||
* This waits up to 20 seconds, then times out. | |||
*/ | |||
public List<LogRecordData> waitForLogRecords(int numberOfLogRecords) { |
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.
Firstly I think you should update the PR title or description to make it easier to understand what you are doing. As far as I can tell the improvement is that you are making metric and log assertion similar to trace assertion.
WDYT about moving the log assertion logic to InstrumentationTestRunner
as it is for traces/metrics. Then you would not need the AwaitUtil
class and could have the helper method in InstrumentationTestRunner
.
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.
good point @laurit - fixed
1c69808
to
b19f64b
Compare
When log and metrics assertions fail, they don't show which assertion actually failed - you just see "timed out" - even the stack trace is lost.
This PR restores the stack trace by running the assertion one last time in the thread of the caller.
In addition, we now also tell the user which metric was not found.