-
Notifications
You must be signed in to change notification settings - Fork 16
feat(profiler)!: fork-safe managed exporter #1464
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
base: main
Are you sure you want to change the base?
Conversation
d4af4ba to
5d2c1cb
Compare
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🔗 Commit SHA: 3244838 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
5d2c1cb to
ec1141a
Compare
BenchmarksComparisonBenchmark execution time: 2026-01-22 14:54:46 Comparing candidate commit d91ac4e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 57 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
Group 14
Group 15
Group 16
Group 17
Group 18
Group 19
BaselineOmitted due to size. |
da6f1e6 to
d91ac4e
Compare
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-apple-darwin
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-apple-darwin
x86_64-unknown-linux-gnu
|
d91ac4e to
e1ea254
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1464 +/- ##
==========================================
+ Coverage 71.16% 71.46% +0.29%
==========================================
Files 419 420 +1
Lines 67165 67965 +800
==========================================
+ Hits 47795 48568 +773
- Misses 19370 19397 +27
🚀 New features to boost your workflow:
|
e1ea254 to
3244838
Compare
| let cloned_receiver: Receiver<RequestBuilder> = receiver.clone(); | ||
| let cloned_cancel = cancel.clone(); | ||
| let handle = std::thread::spawn(move || { | ||
| let runtime = tokio::runtime::Builder::new_current_thread() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for not using a multi threaded runtime with a single thread ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multi-threaded runtime adds more complexity that we don't need if we're single threaded, afaict?
| @@ -0,0 +1,573 @@ | |||
| // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi we did something similar in libdd-data-pipeline/src/pausable_worker.rs
| receiver: Receiver<RequestBuilder>, | ||
| } | ||
|
|
||
| pub struct SuspendedExporterManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to handle the state management in rust with an enum ?
What does this PR do?
Adds a forksafe exporter where the rust side manages all state.
Motivation
We've had recurrent bugs in Python when it comes to forksafety of code like this. Having a simple API is great.
Additional Notes
Anything else we should know when reviewing?
How to test the change?
See the new tests and the example files.