Skip to content
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

feat: alternative proposal for diagnostic channel #2857

Closed
wants to merge 2 commits into from

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Feb 26, 2024

Just for comparison

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 80.97561% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 93.76%. Comparing base (e39a632) to head (554f6f0).
Report is 362 commits behind head on main.

Files Patch % Lines
lib/core/diagnostics.js 68.68% 31 Missing ⚠️
lib/web/fetch/diagnostics.js 89.87% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2857      +/-   ##
==========================================
+ Coverage   85.54%   93.76%   +8.21%     
==========================================
  Files          76       85       +9     
  Lines        6858    23424   +16566     
==========================================
+ Hits         5867    21964   +16097     
- Misses        991     1460     +469     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

/(FETCH [0-9]+:) (sending request)/,
/(FETCH [0-9]+:) (received response)/,
/(FETCH [0-9]+:) (trailers received)/,
/FETCH [0-9]+: fetch started/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the order is wrong.
start, asyncStart, end, asyncEnd is the same name as tracing channel and people expect similar function.

start means start of function.
end means end of function (at return).

asyncStart means start of callback.
asyncEnd means end of callback.
In fetch case, since it is a promise, I don't see how asyncStart and asyncEnd would fit in any case. Unless there is a interceptor.

@Uzlopak Uzlopak closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants