-
Notifications
You must be signed in to change notification settings - Fork 12
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
fix(profiling): label handling and fuzzing #959
base: main
Are you sure you want to change the base?
fix(profiling): label handling and fuzzing #959
Conversation
BenchmarksComparisonBenchmark execution time: 2025-03-26 14:54:04 Comparing candidate commit a47b8cc in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 52 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
BaselineOmitted due to size. |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
i686-alpine-linux-musl
i686-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
17b001c
to
0b1769d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #959 +/- ##
==========================================
+ Coverage 72.79% 72.81% +0.01%
==========================================
Files 334 334
Lines 50903 50988 +85
==========================================
+ Hits 37057 37126 +69
- Misses 13846 13862 +16
🚀 New features to boost your workflow:
|
Sorry if obvious, but I've been bit in the past by not having the same version of the licenses tool as CI -- running |
I know we pinned cargo-bundle-licenses in the past. I have pinned it again 😆 |
25eac43
to
69994a9
Compare
This is mostly a testing/fuzzing fix than a real error. But it did catch one error in an existing test that went previously undetected.
69994a9
to
229cef5
Compare
error: unsupported operation: can't call foreign function `kqueue` on OS `macos`
What does this PR do?
This fixes GH #958 by:
From
toTryFrom
).label.str
orlabel.num
+label.num_value
a bit better.Option<*Id>
to just be*Id
. This removes potential issues aroundSome(0)
andNone
not always being considered the same despite the fact they ought to be. Now there's just one way to represent it, (which is 0).Motivation
I had a fuzzer error locally, and also hit one in CI. I figured since it cropped up both places recently, it was time to attack it.
Additional Notes
I hit a 3rd party license job failure. I updated the licenses, but it the CI seems to fetch different values. Not sure what's going on there; we'll see if it keeps happening.
I found another fuzzer error, but it doesn't appear to be around label handling, so I'll leave that to another PR.
How to test the change?
If you use the FFI, there's nothing to change. If you use the
api::*
layer, you'll have to account for some shifts in representation.