Skip to content

extend integration CI workflow to rails-five and rails-six#5785

Open
p-datadog wants to merge 3 commits into
masterfrom
symdb-rails-integration-test-56
Open

extend integration CI workflow to rails-five and rails-six#5785
p-datadog wants to merge 3 commits into
masterfrom
symdb-rails-integration-test-56

Conversation

@p-datadog
Copy link
Copy Markdown
Member

@p-datadog p-datadog commented May 19, 2026

What does this PR do?

Extends the integration test workflow added in #5696 to also run rails-five (Ruby 3.0) and rails-six (Ruby 3.3) integration apps. Stacks on top of #5696 — once that lands, this diff narrows to just the rails-five/six additions.

Motivation:

The rails-seven workflow restored execution for the rails-seven/spec/integration/di_spec.rb that had been silently skipped since CircleCI was retired (commit 3e3511b, Feb 2025). The same orphan exists in rails-five/spec/integration/di_spec.rb and rails-six/spec/integration/di_spec.rb. This PR wires them up.

Approach:

  • Workflow re-shaped as a single matrix-driven job covering {rails-five, 3.0}, {rails-six, 3.3}, {rails-seven, 3.4}.
  • Same two bit-rot fixes the rails-seven path needed are applied to the new versions:
    • Ruby 3.0 / 3.3 base images: switch the yarn repo install from the removed apt-key add to the modern gpg --dearmor + signed-by keyring pattern.
    • rails-five/rails-six docker-compose.ci.yml: add the database name to DATABASE_URL so bin/rails db:prepare does not abort with "No database selected" under Rails 7's URL-merging rules.

Change log entry

None.

@datadog-datadog-prod-us1
Copy link
Copy Markdown
Contributor

datadog-datadog-prod-us1 Bot commented May 19, 2026

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 97.09% (-0.01%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: bb4134b | Docs | Datadog PR Page | Give us feedback!

p-ddsign added 2 commits May 20, 2026 13:55
Extends the integration test workflow (introduced for rails-seven in
PR #5696) to also run rails-five (Ruby 3.0) and rails-six (Ruby 3.3).

Re-shapes the workflow as a single matrix-driven job. Applies the same
two bit-rot fixes that rails-seven needed:

- Ruby 3.0 / 3.3 base images: switch the yarn repo install from the
  removed `apt-key add` to the modern `gpg --dearmor` + signed-by
  keyring pattern, so the image still builds on Debian 13.
- rails-five / rails-six docker-compose.ci.yml: add the database name
  to DATABASE_URL so `bin/rails db:prepare` does not abort with
  "No database selected" under Rails 7's URL-merging rules.
Two follow-ups from the first CI run on this PR:

- rails-five's script/ci explicitly rejects Ruby 3.0 ("Ruby 3 is not
  supported by Rails 5"). Switch the matrix to 2.7, the latest version
  the script and build-images both accept.

- rails-six failed with `NameError: ActiveJob::QueueAdapters::AbstractAdapter`.
  Cause: Gemfile leaves resque unpinned, so it resolves to 3.0 which
  requires Rails 7+. rails-six runs Rails 6.1. Pin `~> 2` on both
  rails-five and rails-six.
@p-datadog p-datadog force-pushed the symdb-rails-integration-test-56 branch from 8587007 to c921b98 Compare May 20, 2026 17:55
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented May 27, 2026

Benchmarks

Benchmark execution time: 2026-05-27 21:24:54

Comparing candidate commit bb4134b in PR branch symdb-rails-integration-test-56 with baseline commit f87530c in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

@p-datadog p-datadog marked this pull request as ready for review May 28, 2026 17:33
@p-datadog p-datadog requested a review from a team as a code owner May 28, 2026 17:33
@p-datadog p-datadog changed the title symdb: extend integration CI workflow to rails-five and rails-six extend integration CI workflow to rails-five and rails-six May 28, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb4134b91e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- "3.4"
include:
- app: rails-five
ruby: "2.7"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fix the Ruby 2.7 image before adding rails-five

This new matrix entry drives ./integration/script/build-images -v 2.7, which uses integration/images/ruby/2.7/Dockerfile; that Dockerfile still has the old Yarn apt-key add setup, while this change only applies the keyring fix to the 3.0 and 3.3 images. In the GitHub Actions runner this means the newly added rails-five leg still hits the same Yarn/apt bit-rot that this patch is fixing for the other selected images, so the workflow can fail before the Rails app is even built. Please either apply the same Dockerfile fix to the 2.7 image or select an image that has been fixed.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

I see the new jobs running:

...and they seem to be working fine!

On a meta level, we do have a bunch of rails variants in system-tests as well so there may be some duplication between these tests and system-tests but... this is more "food for thought and consideration if it makes sense to merge/move things there" than a "for this PR" kinda note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants