feat(capablities)!: sleep & spawn capabilities#1873
Open
Aaalibaba42 wants to merge 3 commits intomainfrom
Open
feat(capablities)!: sleep & spawn capabilities#1873Aaalibaba42 wants to merge 3 commits intomainfrom
Aaalibaba42 wants to merge 3 commits intomainfrom
Conversation
Contributor
📚 Documentation Check Results📦
|
Contributor
🔒 Cargo Deny Results📦
|
Contributor
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1873 +/- ##
==========================================
+ Coverage 71.69% 71.72% +0.03%
==========================================
Files 431 433 +2
Lines 69142 69306 +164
==========================================
+ Hits 49569 49710 +141
- Misses 19573 19596 +23
🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: f4b19a7 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
Aaalibaba42
force-pushed
the
jwiriath/runtime-capability
branch
from
0f2d27f to
628bd2c
Compare
Aaalibaba42
changed the title
Aaalibaba42
force-pushed
the
jwiriath/runtime-capability
branch
from
628bd2c to
4145e42
Compare
bantonsson
reviewed
Apr 16, 2026
Comment on lines
+131
to
+134
| let result = tokio::select! { | ||
| _ = capabilities.sleep(timeout) => Err(()), | ||
| r = capabilities.request(req) => Ok(r), | ||
| }; |
Contributor
There was a problem hiding this comment.
This behaves differently than tokio::time::timeout. On simultaneous completion, timeout favors the request while select! picks one at random.
Suggested change
| let result = tokio::select! { | |
| _ = capabilities.sleep(timeout) => Err(()), | |
| r = capabilities.request(req) => Ok(r), | |
| }; | |
| let result = tokio::select! { | |
| biased; | |
| r = capabilities.request(req) => Ok(r), | |
| _ = capabilities.sleep(timeout) => Err(()), | |
| }; |
| } else { | ||
| // The task has been aborted and the worker can't be retrieved. | ||
| *self = PausableWorker::InvalidState; | ||
| Err(PausableWorkerError::TaskAborted) |
Contributor
There was a problem hiding this comment.
Why has the error handIng been removed? Won't the new code panic?
Contributor
Author
There was a problem hiding this comment.
Tokio's JoinHandle implements Future<Output = Result<T, JoinError>>, now I use Box<dyn Future<Output = T>> instead for our WorkerJoinHandle<T>, so we can't directly use the same pattern.
I could make it so our JoinHandle type also is a Result but futures::remote_handle() yields a T and panics on drop-cancel, so this way everything has the same API.
Looking at solution a little, I guess I can do something along the lines of std::panic::AssertUnwindSafe(handle).catch_unwind().await to get back a result here, but I'm not sure it's better.
| })?; | ||
| })? | ||
| .enter(); | ||
| let capabilities = C::new_client(); |
Contributor
There was a problem hiding this comment.
Maybe a comment here that the native C is expecting to be in the context of the tokio runtime, while the node one doesn't care.
| /// | ||
| /// The runtime handle is extracted internally and passed to [`SpawnCapability::spawn`] | ||
| /// as the [`SpawnCapability::RuntimeContext`]. On native this constrains the spawner's | ||
| /// `RuntimeContext` to `tokio::runtime::Handle`; on wasm it is `()`. |
Contributor
There was a problem hiding this comment.
I think this comment should be on the type SpawnCapability::RuntimeContext
| stats = StatsComputationStatus::DisabledByAgent { bucket_size }; | ||
| let info_endpoint = Endpoint::from_url(add_path(&agent_url, INFO_ENDPOINT)); | ||
| // On wasm the AgentInfoFetcher is not spawned (it uses tokio::time::sleep | ||
| // internally), but we still need the ResponseObserver for header checks. |
Contributor
There was a problem hiding this comment.
This comment is about the block below this one.
| stats = StatsComputationStatus::DisabledByAgent { bucket_size }; | ||
| } | ||
|
|
||
| #[cfg(all(not(target_arch = "wasm32"), feature = "telemetry"))] |
Contributor
There was a problem hiding this comment.
Can't this #[cfg...] mess be separated out in some way? This is barely readable.
Contributor
Author
There was a problem hiding this comment.
I'm not sure how without making spaghetti, do you have something in mind ?
bantonsson
reviewed
Apr 16, 2026
| > Worker for StatsExporter<Cap, Con> | ||
| { | ||
| async fn trigger(&mut self) { | ||
| tokio::time::sleep(self.flush_interval).await; |
Contributor
There was a problem hiding this comment.
Shouldn't this use a capability?
Aaalibaba42
force-pushed
the
jwiriath/runtime-capability
branch
from
fb4421b to
166e5c7
Compare
Aaalibaba42
force-pushed
the
jwiriath/runtime-capability
branch
from
166e5c7 to
f4b19a7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Motivation
Before if we were to spawn tasks on wasm, we would have required a current thread tokio runtime blocking JS event loop until the end of the tokio runtime itself. Now in wasm we delegate tasks to the eventloop itself, making the whole thing non blocking, and enabling stuff like sleeping in a JS compatible way.
Additional Notes
/
How to test the change?
DataDog/libdatadog-nodejs#70