-
Notifications
You must be signed in to change notification settings - Fork 810
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
fix(sdk-metrics): await export when async attributes are pending #5126
fix(sdk-metrics): await export when async attributes are pending #5126
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5126 +/- ##
==========================================
+ Coverage 93.17% 93.20% +0.03%
==========================================
Files 315 315
Lines 8086 8084 -2
Branches 1617 1616 -1
==========================================
+ Hits 7534 7535 +1
+ Misses 552 549 -3
|
e81e944
to
617305a
Compare
…ed when resolving the async resource fails
…is called when resolving the async resource fails
e3476c0
to
8b372cb
Compare
packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts
Outdated
Show resolved
Hide resolved
await resourceMetrics.resource.waitForAsyncAttributes?.(); | ||
} catch (e) { | ||
api.diag.debug('Error while resolving async portion of resource: ', e); | ||
globalErrorHandler(e); |
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.
TIL about globalErrorHandler.
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.
globalErrorHandler is a poorly understood and (in my opinion) poorly thought out spec. I'd probably get rid of it if i had the chance.
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.
I'm only requesting changes as a test. I'll dismiss this immediately
Which problem is this PR solving?
When
async
attributes are pending, we don't await the export, which means that ashutdown()
following aforceFlush()
will shut down the exporter beforeforceFlush()
is completed and the export will be cancelled.Fixes #5116
Type of change
How Has This Been Tested?