Feat: Instrument await using await-tree

xline-kv / Xline

A geo-distributed KV store for metadata management

https://xline.cloud

Apache License 2.0

596 stars 74 forks source link

Feat: Instrument await using await-tree #669

Closed Harsh1s closed 7 months ago

Harsh1s commented 7 months ago

Signed-off-by: Harsh Raj thisishraj@gmail.com

what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?) -> FIxes #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

Harsh1s commented 7 months ago

@iGxnon I'm not sure if these are the correct changes that you were looking for, right now I have just added instrument_await to all the functions with instrument macro. From my reading, this should be enough for the functions for which tracing has already been setup with instrument macro. I'll give my best to learn more and make changes if these are not enough, thanks for your patience!

Also, I'll fix the DCO.

iGxnon commented 7 months ago

It seems that some validations are not passing. Please check your code and provide an example output if possible.

Harsh1s commented 7 months ago

It seems that some validations are not passing. Please check your code and provide an example output if possible.

Sure, I'll report back with some output. Thanks for the help!

codecov[bot] commented 7 months ago

Codecov Report

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

Project coverage is 74.52%. Comparing base (ec50084) to head (bdc3dad).

Files	Patch %	Lines
crates/curp/src/server/mod.rs	69.44%	9 Missing and 2 partials :warning:
crates/xline/src/server/kv_server.rs	33.33%	5 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #669 +/- ## ========================================== - Coverage 74.55% 74.52% -0.04% ========================================== Files 172 172 Lines 25288 25342 +54 Branches 25288 25342 +54 ========================================== + Hits 18854 18886 +32 - Misses 5263 5287 +24 + Partials 1171 1169 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Harsh1s commented 7 months ago

@iGxnon Sorry for not being able to devote my time properly for this issue, was busy with some stuff. From what I can see, I'm failing the codecov/project check. Could you suggest some changes that I could make to pass the test?

Phoenix500526 commented 7 months ago

Hello, @Harsh1s! I pulled down this pr and found that the signed-off-by information was lost. That's the reason behind the DCO failure. I recommend rebasing your pr on the latest commit of the master branch instead of merging them directly.

Harsh1s commented 7 months ago

Hello there @Phoenix500526 , apologies for accidently closing this PR while fixing the DCO. I opened a new PR instead of this one. I'll take care of rebasing from next time. Thanks!