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

futures::Stream and futures::Sink instrumentation #198

Closed
iGxnon opened this issue Feb 1, 2024 · 7 comments
Closed

futures::Stream and futures::Sink instrumentation #198

iGxnon opened this issue Feb 1, 2024 · 7 comments

Comments

@iGxnon
Copy link
Contributor

iGxnon commented Feb 1, 2024

I am building a network library, using a large number of Streams and Sinks to implement IO operations, and preparing to migrate from tokio tracing to minitrace.

I am wondering if support for Stream and Sink instrumentation can be added.

A related crate: https://crates.io/crates/tracing-futures

@andylokandy
Copy link
Collaborator

Yes,this is a good idea!

@iGxnon
Copy link
Contributor Author

iGxnon commented Feb 1, 2024

I'd like to help :D

@andylokandy
Copy link
Collaborator

Great to have you on board! Let's delve into the details. You can create a new crate named minitrace-futures, and then add SinkInSpan and StreamInSpan. It can be hosted either in this repository or in a new one of your own, depending on your preference.

@iGxnon
Copy link
Contributor Author

iGxnon commented Feb 1, 2024

It can be hosted either in this repository

Yes, I want to put it in this repo.

I implemented an initial version that meets the needs of my project while also providing more customization for some other scenarios.
Please give me some advice!

https://github.com/iGxnon/raknet-rs/blob/main/src/utils/minitrace.rs

A usecase in my project:

https://github.com/iGxnon/raknet-rs/blob/73e0da225fd1c6975a52c48497532571a497ac37/src/server/incoming/mod.rs#L38-L85

@andylokandy
Copy link
Collaborator

Great job! However, the MakeSpan appears to be overly specific.

Would you consider creating a pull request to include these changes in the repository?

@iGxnon
Copy link
Contributor Author

iGxnon commented Feb 4, 2024

Here #202

@andylokandy
Copy link
Collaborator

Closed by #202

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

No branches or pull requests

2 participants