-
Notifications
You must be signed in to change notification settings - Fork 75
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: Instrument await using await-tree #682
base: master
Are you sure you want to change the base?
feat: Instrument await using await-tree #682
Conversation
Signed-off-by: Harsh1s <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #682 +/- ##
==========================================
- Coverage 75.55% 74.59% -0.96%
==========================================
Files 180 172 -8
Lines 26938 25342 -1596
Branches 26938 25342 -1596
==========================================
- Hits 20353 18904 -1449
+ Misses 5366 5270 -96
+ Partials 1219 1168 -51 ☔ View full report in Codecov by Sentry. |
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.
Will this modification affect performance in any way? I suggest adding a switch here so that we can enable it as needed.
Sure, I'll make the changes and let you know. Thanks! |
Can you provide an example after running it? |
@Harsh1s Your PR is in conflict and cannot be merged. |
Hi, @Harsh1s ! This pr has been stalled for 3 weeks. Would you like to update it? 😄 |
Oh I'm really sorry about the stalling, it's been a busy past month at my uni, a lot of tests and project deadlines. My end semester exams are going on currently too. I'll try to update it within a week for sure. Sorry again! |
@@ -14,6 +14,7 @@ version = "0.1.0" | |||
[dependencies] | |||
async-stream = "0.3.4" | |||
async-trait = "0.1.80" | |||
await-tree = "0.1.2" |
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.
The latest version is "0.2.1", would you please update it?
@@ -15,6 +15,7 @@ categories = ["KV"] | |||
anyhow = "1.0.82" | |||
async-stream = "0.3.5" | |||
async-trait = "0.1.80" | |||
await-tree = "0.1.2" |
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.
The same as above.
what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)
-> FIxes [Feature]: Instrument the execution path of the command (i.e.
fast_path
orslow_path
) #580-> Currently, some simple information is logged to display the execution path of the command. This uses more detailed information to show this process (i.e. use await-tree to indicate the duration of each await.).
what changes does this pull request make?
-> Makes use of instrument_await within functions annotated with the #[instrument] macro to add more detailed async tracing.
are there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)
-> No