-
Notifications
You must be signed in to change notification settings - Fork 879
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
Update the OpenTelemetry SDK version to 1.38.0 #11335
Update the OpenTelemetry SDK version to 1.38.0 #11335
Conversation
.hasAttributesSatisfying( | ||
equalTo(stringKey("test"), "test")))))); | ||
|
||
// sleep exporter interval |
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.
Could you please explain the reason of this sleep
and the following one?
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.
This appears to be a standard pattern that is used in these tests. It was already present in the original version of this test (this test is a slightly modified copy of the same test in v1_37
which is a copy from another version). My understanding is that the first sleep is there to ensure that metric data arrives before clearData
is called, but as there is already a call to waitAndAssertMetrics
that also waits for metrics to arrive (actually it waits for assertion to pass, but here assertion requires metrics) I don't think it is really necessary. The second sleep after clearData
should ensure that if there is new metrics are produced they have time to arrive. I think this is necessary because the following waitAndAssertMetrics
asserts that there are no metrics and this could pass if we don't wait until metrics have had time to arrive (assuming that there is a bug and there unexpectedly are metrics).
Changing these is out of the scope for this PR.
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.
thanks ❤️
import org.junit.jupiter.api.TestInfo; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
class MeterTest { |
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.
what do you think of extending an AbstractMeterTest
so we can get coverage of all the normal metrics behavior as well, or do you think it's not really needed?
Co-authored-by: Lauri Tulmin <[email protected]>
Co-authored-by: Lauri Tulmin <[email protected]>
Update the OpenTelemetry SDK version to
1.38.0
.