[PROF-4779] Publish libddprof as a Ruby gem on rubygems.org #16
Conversation
Right now it's empty, still need to add the magic part where we get the libddprof binaries!
There was a problem hiding this comment.
Made some comments, which are more like suggestions: nothing struck me, so LGTM, but this might need to evolve as platfoms are added and CI develops.
I would suggest quickly adding CI (however crude) to build and test gem packages, like there is for libddwaf.
ruby/Rakefile
Outdated
| puts "Found #{target_file} matching the expected sha256, skipping download" | ||
| next | ||
| else | ||
| puts "Found #{target_file} BUT IT DID NOT MATCH THE EXPECTED sha256, skipping download" |
There was a problem hiding this comment.
Outputting the unexpected hash can be useful in practice, so that one can download a known legit file and have the correct hash to fill in.
Also, does this really skip the download? There's no next so it would fall through and proceed, correct?
There was a problem hiding this comment.
I've added printing of the hash in 8d23e24 .
Also, does this really skip the download? There's no next so it would fall through and proceed, correct?
Oops, that was just too much copy pasta. The intention is to just redownload the file when the hash doesn't match, so the code was correct, but the logging was misleading. Fixed in the same commit as above.
| end | ||
|
|
||
| desc "Release all packaged gems" | ||
| task push_to_rubygems: [ |
There was a problem hiding this comment.
Seems uncanny to have the _to_rubygems suffix. Is there a conflicting push task preexisting? We could also name it push:all, WDYT?
There was a problem hiding this comment.
While a bit uncommon, I like that it exactly matches the name of the docker invocation that calls it (docker-compose run push_to_rubygems), which was why I adopted this naming.
My plan was to handle the CI on the dd-trace-rb side, since we already have the multiple variants and libddprof can be validated in context with its user. Since I don't expect |
ivoanjo
changed the title
ruby/README.md
Outdated
| 3. [ ] Update the <lib/libddprof/version.rb> file with the `LIB_VERSION` and `VERSION` to use | ||
| 4. [ ] Commit change, open PR, get it merged | ||
| 5. [ ] Release by running `docker-compose run push_to_rubygems`. | ||
| (When asked for rubygems credentials, check your local friendly 1Password.) |
There was a problem hiding this comment.
Since libddprof is a public-facing repo, it is meaningful to qualify this statement a little bit more (or less)? Strictly speaking, someone should know whether they do not have the credentials, but perhaps a Datadog employee may not realize that this is in fact directed at them.
No clue, just a random thought.
There was a problem hiding this comment.
👍 agreed, clarified in 90af8a2 that these steps are for Datadog employees.
| { | ||
| file: "libddprof-x86_64-alpine-linux-musl.tar.gz", | ||
| sha256: "d519a6241d78260522624b8e79e98502510f11d5d9551f5f80fc1134e95fa146", | ||
| ruby_platform: "x86_64-linux-musl" |
There was a problem hiding this comment.
Pedantic, but will anyone be confused by seeing libddprof-x86_64-alpine-linux-musl on their non-Alpine musl system? As far as I know, the liddprof artifacts don't care. In fact, I wouldn't be surprised if the musl artifact works perfectly fine on glibc systems.
(irrelevant comment is irrelevant, sorry)
There was a problem hiding this comment.
It's a good question!
Ideally we would upload each ruby_platform variant separately to rubygems.org, and Ruby would pick the right one. But there's a bunch of bugs around that and even if the bugs were fixed, we support really old Ruby versions that wouldn't be running the fixed versions.
So actually we're shipping both variants inside the same release on rubygems.org, and then the consumer will pick the right one:
def self.pkgconfig_folder
current_platform = Gem::Platform.local.to_s
return unless available_binaries.include?(current_platform)
pkgconfig_file =
Dir.glob("#{vendor_directory}/#{current_platform}/**/ddprof_ffi.pc").first # <--- magic happens hereBut TL;DR I don't think this will confuse customers because they will only get libddprof through dd-trace-rb, which will use the code above to pick the right one and ignore the other.
| GEM_PRERELEASE_VERSION = ".beta3" | ||
| private_constant :GEM_MAJOR_VERSION, :GEM_MINOR_VERSION, :GEM_PRERELEASE_VERSION | ||
|
|
||
| # The gem version scheme is lib_version.gem_major.gem_minor[.prerelease]. |
What does this PR do?
This PR adds the scaffolding for taking a
libddprofbinary release from GitHub, and releasing it as a Ruby gem on rubygems.org.This allows us to use
libddprofon the Ruby profiler shipped in dd-trace-rb in a quite simple way: the released library has almost no code, and effectively just distributes the release files in a way that can be easily consumed by a Ruby library.The release is mostly-automated (detailed instructions are on README.md)
libbprof, as well as bump the versiondocker-compose run push_to_rubygemsI will be the one responsible for pushing new releases (on behalf of the profiling team), but still anyone can do a release if needed.
In the future we may want to look at triggering this from CI so that new
libddprofreleases are automatically pushed on rubygems.org.Motivation
Having
libddprofon rubygems.org and managed directly by the Profiling team (e.g. me) decouples releasing and distribution of the dd-trace-rb library fromlibddprof.Additional Notes
I've already pushed
libddprof0.2.0 on rubygems.org using this PR: https://rubygems.org/gems/libddprof .The current setup pushes two "variants" of the gem -- one with no binaries, which is used as a "no-op" dependency for platforms that libddprof does not support, and a "fat" one for x86-64 Linux, that includes both the glibc, as well as musl library binaries inside.
How to test the change?
Testing for this will be done on the consumer side: in the Ruby profiler, which will run libddprof on different platforms and Ruby versions.