You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/07/23 15:47:54 UTC
[GitHub] [skywalking-rust] jmjoy opened a new pull request, #36: Add feature sync and sync collect method.
jmjoy opened a new pull request, #36:
URL: https://github.com/apache/skywalking-rust/pull/36
Change `Tracer::reporting` to support both `async` and `sync` collection, the `sync` mode is used by non tokio async runtime (such as my [php-skywalking-agent](https://github.com/jmjoy/php-skywalking-agent)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] wu-sheng commented on a diff in pull request #36: Add sync collect method.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36#discussion_r928177586
##########
README.md:
##########
@@ -81,11 +81,12 @@ async fn main() -> Result<(), Box<dyn Error>> {
tokio::spawn(handle_request(tracer.clone()));
// Start to report.
- let handle = tracer.reporting(async move {
- let _ = signal::ctrl_c().await;
- });
-
- handle.await?;
+ tracer
+ .reporting()
+ .with_graceful_shutdown(async move {
+ let _ = signal::ctrl_c().await;
+ })
+ .await?;
Review Comment:
We need to add sync mode in the doc, and provide a clear explanation about when should use sync, and what would be the possible impact? Such as, sync mode usually means it would block the current thread until reported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] wu-sheng merged pull request #36: Improve LogReporter and fix tests.
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] jmjoy commented on pull request #36: Improve LogReporter and fix tests.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36#issuecomment-1195371783
I thought about it for a while, sync mode should not be a good solution, but this PR still improved `LogReporter` and fixed the test in `CI`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] wu-sheng commented on pull request #36: Improve LogReporter and fix tests.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36#issuecomment-1195590145
Somehow you changed the CI settings, and the required task name is missing. You need to add it back and depends on all other tasks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] wu-sheng commented on pull request #36: Improve LogReporter and fix tests.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36#issuecomment-1195445803
I think you should update PR descriptions accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] jmjoy commented on pull request #36: Improve LogReporter and fix tests.
Posted by GitBox <gi...@apache.org>.
jmjoy commented on PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36#issuecomment-1195449868
> I think you should update PR descriptions accordingly.
Yes, updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rust] codecov-commenter commented on pull request #36: Add feature sync and sync collect method.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #36:
URL: https://github.com/apache/skywalking-rust/pull/36#issuecomment-1193146267
# [Codecov](https://codecov.io/gh/apache/skywalking-rust/pull/36?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#36](https://codecov.io/gh/apache/skywalking-rust/pull/36?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7340aeb) into [master](https://codecov.io/gh/apache/skywalking-rust/commit/9f777ed76c816abd118f43c5a5a218f8c4b2ed3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9f777ed) will **decrease** coverage by `6.58%`.
> The diff coverage is `14.06%`.
```diff
@@ Coverage Diff @@
## master #36 +/- ##
==========================================
- Coverage 38.88% 32.30% -6.59%
==========================================
Files 13 13
Lines 306 390 +84
==========================================
+ Hits 119 126 +7
- Misses 187 264 +77
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking-rust/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [src/reporter/grpc.rs](https://codecov.io/gh/apache/skywalking-rust/pull/36/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JlcG9ydGVyL2dycGMucnM=) | `0.00% <0.00%> (ø)` | |
| [tests/trace\_context.rs](https://codecov.io/gh/apache/skywalking-rust/pull/36/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdHMvdHJhY2VfY29udGV4dC5ycw==) | `0.00% <ø> (ø)` | |
| [src/context/tracer.rs](https://codecov.io/gh/apache/skywalking-rust/pull/36/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2NvbnRleHQvdHJhY2VyLnJz) | `17.17% <9.75%> (-24.01%)` | :arrow_down: |
| [src/reporter/log.rs](https://codecov.io/gh/apache/skywalking-rust/pull/36/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JlcG9ydGVyL2xvZy5ycw==) | `19.04% <21.05%> (+19.04%)` | :arrow_up: |
| [tests/propagation.rs](https://codecov.io/gh/apache/skywalking-rust/pull/36/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdHMvcHJvcGFnYXRpb24ucnM=) | `100.00% <100.00%> (ø)` | |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org