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

task: also instrument streams #31

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

duarten
Copy link
Contributor

@duarten duarten commented Nov 7, 2022

Using pretty much the same logic as for futures, instrument streams as
well.

Fixes #30

Cargo.toml Outdated
@@ -19,6 +19,7 @@ default = ["rt"]
rt = ["tokio"]

[dependencies]
futures-core = "0.3.25"
Copy link
Member

Choose a reason for hiding this comment

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

Could prob depend on tokio-stream instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change!

@Darksonn
Copy link
Contributor

You will need to merge in master once #32 is merged to fix the CI failures.

@Darksonn
Copy link
Contributor

You can merge master now.

Using pretty much the same logic as for futures, instrument streams as
well.
@duarten
Copy link
Contributor Author

duarten commented Dec 12, 2022

Ping 😅

@duarten
Copy link
Contributor Author

duarten commented Dec 13, 2022

The doc tests also fail for me on main.

@Darksonn
Copy link
Contributor

That's annoying. I'm not sure what's causing it.

@jswrenn
Copy link
Collaborator

jswrenn commented Dec 13, 2022

Since the crate monitors tokio's internals, there's an inherent amount of flakiness. Perhaps we could use nextest with retries in CI instead.

@Darksonn
Copy link
Contributor

Is it just a flaky test? I saw it fail several times in a row.

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Anyway, the test failure isn't your fault, and would need to be fixed in a separate PR regardless. I'm going to merge this now.

@Darksonn Darksonn merged commit 84cbb53 into tokio-rs:main Dec 14, 2022
@Noah-Kennedy Noah-Kennedy mentioned this pull request Mar 6, 2023
Noah-Kennedy added a commit that referenced this pull request Mar 6, 2023
# 0.2.0 (March 6th, 2023)

### Added
- Add `Debug` implementations. ([#28])
- rt: add concrete `RuntimeIntervals` iterator type ([#26])
- rt: add budget_forced_yield_count metric ([#39])
- rt: add io_driver_ready_count metric ([#40])
- rt: add steal_operations metric ([#37])
- task: also instrument streams ([#31])

### Documented
- doc: fix count in `TaskMonitor` docstring ([#24])
- doc: the description of steal_count ([#35])

[#24]: #24
[#26]: #26
[#28]: #28
[#31]: #31
[#35]: #35
[#37]: #37
[#39]: #39
[#40]: #40
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.

Allow Streams to be instrumented
4 participants