[PROF-8289] Upgrade to libdatadog 5#3169
Merged
Merged
Conversation
Base automatically changed from
ivoanjo/prof-7440-timeline-states-v1
to
master
October 3, 2023 11:35
ivoanjo
added a commit
to DataDog/libdatadog
that referenced
this pull request
Oct 3, 2023
**What does this PR do?** This PR bumps the libdatadog version to 5.0.0. This PR looks a bit different from previous PRs to bump libdatadog version [(here's the 4.0.0 PR as an example)](#236) because in [this PR](#247) we've centralized the version on the single `Cargo.toml` file. Furthermore, we're going from 4.0.0 to 5.0.0 because there were a number of backwards-incompatible changes to the profiling APIs. **Motivation:** Release libdatadog 5.5.0. **Additional Notes:** If I haven't missed anything, the backwards incompatible API changes were the following: * The value of the `end_timestamp_ns` label is now provided as a regular argument to `ddog_prof_Profile_add` * The libdatadog 5 serializer outputs compressed pprof files * The exporter has a new API that takes two lists, a list of files to compress and a list of files to assume are compressed when exporting * The libdatadog 5 serializer now resets profiles as part of serializing them * The `ddog_prof_Profile_new` now returns a result structure **How to test the change?** I've tested the libdatadog 5 releases using the Ruby profiler, see DataDog/dd-trace-rb#3169 for my draft PR.
2 tasks
…og_prof_Profile_add` directly (Haven't fully tested it since other things aren't compiling yet either).
This is why the `start_time` argument from `reset` is also needed for `_serialize`.
In libdatadog 5, the exporter gets two arguments: * files to be compressed * files that are already assumed to be compressed This change matches up with the fact that the libdatadog 5 serializer now produces compressed pprofs, but some other files (e.g. code provenance) are still handed over uncompressed.
ivoanjo
added a commit
to DataDog/libdatadog
that referenced
this pull request
Oct 4, 2023
**What does this PR do?** This PR includes the changes documented in the "Releasing a new version to rubygems.org" part of the README: <https://github.com/datadog/libdatadog/tree/main/ruby#releasing-a-new-version-to-rubygemsorg> (It's also exactly the same as [the v4.0.0 release PR](#238)). **Motivation:** Enable Ruby to use libdatadog v5.0.0. **Additional Notes:** N/A **How to test the change?** I've tested this release locally against the changes in DataDog/dd-trace-rb#3169 . As a reminder, new libdatadog releases don't get automatically picked up by dd-trace-rb, so the PR that bumps the Ruby profiler will also test this release against all supported Ruby versions.
ivoanjo
force-pushed
the
ivoanjo/prof-8289-libdatadog5-upgrade
branch
from
October 4, 2023 10:07
8b5031f to
f3f0dbd
Compare
2 tasks
ivoanjo
force-pushed
the
ivoanjo/prof-8289-libdatadog5-upgrade
branch
from
October 4, 2023 10:48
f3f0dbd to
df15be8
Compare
ivoanjo
marked this pull request as ready for review
October 4, 2023 10:49
TonyCTHsu
approved these changes
Oct 4, 2023
TonyCTHsu
left a comment
TonyCTHsu
left a comment
Collaborator
There was a problem hiding this comment.
Just a reminder to run bundle exec appraisal:lock before merging.
Member
Author
Right! Thanks for the reminder, I totally forgot about it. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3169 +/- ##
==========================================
- Coverage 98.21% 98.20% -0.01%
==========================================
Files 1247 1247
Lines 71663 71666 +3
Branches 3329 3330 +1
==========================================
+ Hits 70381 70383 +2
- Misses 1282 1283 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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?
This PR upgrades dd-trace-rb to use libdatadog 5. This release includes BIG memory footprint improvements, but there were also a few breaking API changes, which needed adjustments in our code.
Motivation:
Use the latest libdatadog release.
Additional Notes:
I recommend reviewing this PR commit-by-commit, as it makes more sense for what changed why instead of the whole diff.
The backwards incompatible changes were the following:
end_timestamp_nslabel is now provided as a regular argument toddog_prof_Profile_addHttpTransportneeded to be adjusted to the new API that allows us to tell libdatadog which files to compress and which to assume are compressed when exportingddog_prof_Profile_newnow returns a result structureI'm opening this PR on top of #3162 because I'm making use of thesample_labelsstruct that was introduced in that PR. Otherwise the two PRs are independant.Furthermore, I'm opening it as a draft since libdatadog 5 is not yet available on rubygems.org. Once a release is out, I'll mark this PR as non-draft.How to test the change?
Our existing test coverage also includes a lot of libdatadog integration testing.
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.