-
Notifications
You must be signed in to change notification settings - Fork 377
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
[🤖] Update Supported Versions #4236
base: master
Are you sure you want to change the base?
Conversation
👋 Hey @DataDog/ruby-guild, please fill "Change log entry" section in the pull request description. If changes need to be present in CHANGELOG.md you can state it this way **Change log entry**
Yes. A brief summary to be placed into the CHANGELOG.md (possible answers Yes/Yep/Yeah) Or you can opt out like that **Change log entry**
None. (possible answers No/Nope/None) Visited at: 2024-12-19 19:58:39 UTC |
Datadog ReportBranch report: ❌ 1 Failed (0 Known Flaky), 22056 Passed, 1476 Skipped, 5m 47.39s Total Time ❌ Failed Tests (1)
|
BenchmarksBenchmark execution time: 2025-01-15 22:53:37 Comparing candidate commit 475c6f5 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics. |
486ab51
to
392a80f
Compare
392a80f
to
68287a9
Compare
9ad6d48
to
5ce75ce
Compare
797133f
to
31fcc83
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4236 +/- ##
==========================================
- Coverage 97.71% 97.71% -0.01%
==========================================
Files 1356 1356
Lines 82490 82489 -1
Branches 4219 4219
==========================================
- Hits 80607 80606 -1
Misses 1883 1883 ☔ View full report in Codecov by Sentry. |
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.
Just a question on Net::HTTP
as I think the versions we're reporting are actually http.rb
"http": [ | ||
"5.0.1", | ||
"5.2.0", | ||
"4.4.1", | ||
"4.4.1" | ||
], |
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.
I am unsure if this is correct - I think it is supposed to be Net::HTTP
which is a library built into Ruby whereas http.rb
is an actual Ruby gem.
So I think this one we may need to treat differently as it should map to the tested Ruby versions of Net::HTTP
Thoughts?
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.
All keys in this JSON file are Ruby gem names in https://rubygems.org/gems/.
So "http"
is https://rubygems.org/gems/http, which they describe as "HTTP (The Gem! a.k.a. http.rb)": https://github.com/httprb/http?tab=readme-ov-file#about
It's ok to leave this is as http
, since that's unambiguously the gem name.
Regarding Net::HTTP
, it's always bundled with the Ruby VM, and it's this gem https://rubygems.org/gems/net-http.
Today we say this for its support:
I see it shows up in our gemfiles for newer versions of Ruby, so I'd say let it show up in this gem_output.json
too, with whatever version we test against.
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.
Ahh okay, so we essentially then have http
and http.rb
duplicated and are missing net-http
in the table thanks!
| httprb | 5.0.1 | 5.2.0 | 4.4.1 | 4.4.1 | | ||
| httpx | 0.11 | 0.11 | None | None | | ||
| kafka | 1.5.0 | 1.5.0 | 1.5.0 | 1.5.0 | | ||
| lograge | 0.12.0 | 0.14.0 | 0.12.0 | 0.14.0 | |
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 isn't in the docs, but we do have it as a contrib
| grape | 1.7.0 | 2.1.2 | 1.7.0 | 1.8.0 | | ||
| graphql | 1.13.21 | 2.3.7 | 1.13.21 | 2.3.6 | | ||
| grpc | 1.48.0 | 1.67.0 | None | None | | ||
| hanami | 1.3.5 | 1.3.5 | None | None | |
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.
check this for jruby
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.
Checked: did not find hanami
defined in any jruby
gemfiles, despite the docs saying we do support for jruby
.
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.
Well I guess that is for no tested versions right?
Does it "work" for JRuby just lacks any tests?
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.
if it's not in the jruby gemfile locks, then we can set it to no support right now.
e0bdd12
to
a29bf91
Compare
integration_versions.md
Outdated
| graphql | 1.13.21 | 2.3.7 | 1.13.21 | 2.3.6 | | ||
| grpc | 1.48.0 | latest | None | None | | ||
| hanami | 1.3.5 | 1.3.5 | None | None | | ||
| http | 5.0.1 | latest | 4.4.1 | 4.4.1 | |
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.
check this, our docs say >= 2.0
| resque | 2.4.0 | latest | 2.4.0 | latest | | ||
| rest_client | 2.1.0 | latest | 2.1.0 | latest | | ||
| roda | 3.65.0 | 3.81.0 | 3.64.0 | 3.72.0 | | ||
| semantic_logger | 4.12.0 | 4.15.0 | 4.12.0 | 4.14.0 | |
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.
Do we need this? Not in the docs
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 is good, it's a real ruby gem we support for trace->log correlation.
we probably forgot to add it here and you are now improving it :)
integration_versions.md
Outdated
| que | 1.4.1 | 2.3.0 | 1.4.1 | 2.2.0 | | ||
| racecar | 2.6.0 | 2.11.0 | 2.6.0 | 2.8.2 | | ||
| rack | 1.6.13 | latest | 1.6.13 | latest | | ||
| rails | 4.2.11.3 | latest | 5.2.8.1 | 6.1.7.8 | |
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.
Similarly here, we should check rails support. The docs say >= 4.0
for both ruby and jruby.
92419ea
to
475c6f5
Compare
This is a PR to update the table for supported integration versions.
Workflow run: Generate Supported Versions
This should be tied to tracer releases, or triggered manually.