-
Notifications
You must be signed in to change notification settings - Fork 296
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
[EventCounters] SpinWait for tests #862
Conversation
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #862 +/- ##
==========================================
+ Coverage 68.33% 68.38% +0.04%
==========================================
Files 183 183
Lines 7024 7025 +1
==========================================
+ Hits 4800 4804 +4
+ Misses 2224 2221 -3
|
test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs
Show resolved
Hide resolved
test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs
Outdated
Show resolved
Hide resolved
test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs
Show resolved
Hide resolved
@@ -64,11 +45,9 @@ public void EventCounter() | |||
|
|||
// Act | |||
counter.WriteMetric(1997.0202); | |||
Task.Delay(Delay).Wait(); | |||
meterProvider.ForceFlush(); | |||
var metric = AwaitExport(meterProvider, metricItems, expectedInstrumentName: "ec.a.c"); |
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.
why no flush first before await?
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.
approving to make progress. some follow ups expected to fully stabilize this.
Thanks for the efforts so far!
Fixes #763 and #910
Changes
Use
SpinWait
instead ofTask.Delay
for a metric item to be exported.The key was to spin until the metric item with the expected name was in the collection since the collection is being populated by all other instruments so simply checking for a count being greater than 1 would not work. Especially in the
Theory
test case.