-
Notifications
You must be signed in to change notification settings - Fork 394
[APMAPI-1607] Upgrade libdatadog dependency to version 22.0.1 #4902
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
Conversation
BenchmarksBenchmark execution time: 2025-10-10 10:23:56 Comparing candidate commit 5023f8b in PR branch Found 3 performance improvements and 2 performance regressions! Performance is the same for 39 metrics, 2 unstable metrics. scenario:profiling - Allocations (baseline)
scenario:profiling - estimated profiler gc per minute (sample 60000 times + serialize result)
scenario:profiling - gvl benchmark samples
scenario:profiling - intern_all 1000 repeated strings
scenario:profiling - stack collector (varying depth)
|
318b6ca
to
9a1e91f
Compare
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 5023f8b | Docs | Was this helpful? Give us feedback! |
Did you mean libdatadog 21? |
Yes I didn't caught that, thanks ! |
0f67946
to
d53a4e1
Compare
dd2edf2
to
ddaa7f7
Compare
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.
This PR had a bunch of things that surprised me...
I think the changes on install_datadog_deps.rb
might've been trying to fix the issue I pointed out with the upstream libdatadog v21 package that's on rubygems.org, but that's the wrong place to fix them -- we need to respin that package, otherwise only SSI customers get the "fix", and all other customers don't.
I suspect that once we fix the libdatadog release, most (all?) changes in install_datadog_deps.rb
can be reverted.
Because libdatadog v21.0.0.1.0 on rubygems is in bad shape, I'm marking this PR as "Request changes" as we mustn't merge it until the libdatadog release gets fixed. Otherwise our non-SSI customers would find the next dd-trace-rb release would suddenly be 10x as big as the previous one because of libdatadog.
There's a few more things I don't quite understand why they were changed, so I've left a few questions about them.
b9e113d
to
898254a
Compare
The diff has grown considerably since my review
Are all of the changes in this PR required to be in one PR? 515f625 for example looks to be independent from "updating libdatadog dependency". |
898254a
to
8b5d033
Compare
8b5d033
to
7ee0486
Compare
This PR also looks to have been incorrectly rebased. |
7ee0486
to
7465470
Compare
sorry about screwing up this branch. I removed all my changes and fixed the bad rebase |
605bcd4
to
9ea74ba
Compare
258293b
to
0fa97b8
Compare
…d support for debug logs
libdatadog 22.0.0 introduced a breaking change in the process discovery FFI. This commit uses the new API.
Co-authored-by: Ivo Anjo <[email protected]>
0fa97b8
to
66c6015
Compare
Typing analysisIgnored filesThere are 539 ignored files in the Steepfile out of 840. Ignored files
Note: Ignored files are excluded from the next sections.
|
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.
👍 Left a couple of questions but overall LGTM!
What does this PR do?
Updates the native bits in the gem (stable config, crashtracking) to be able to work with the latest libdatadog 22.0.1.
Motivation:
DSM needs libdatadog 22, and I had to update the native bits of stable config
Change log entry
Yes. Upgrade libdatadog dependency to 22.0.1
Additional Notes:
The branch name is off because it was originally supposed to be libdatadog 21, and only contains new stable config API but DSM and Process Discovery got added
How to test the change?
CI