Skip to content
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

Work around missing logger dependency in Rails 6.1 and 7.0 #4298

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

p-datadog
Copy link
Member

@p-datadog p-datadog commented Jan 16, 2025

What does this PR do?

Downgrades concurrent-ruby to 1.3.4 to stop rails 6.1 from failing during bootstrap with the following error:

app_1                 | Run: bin/rails db:create
app_1                 | Traceback (most recent call last):
app_1                 |         14: from bin/rails:9:in `<main>'
app_1                 |         13: from bin/rails:9:in `require'
app_1                 |         12: from /usr/local/bundle/gems/railties-6.1.7.10/lib/rails/commands.rb:3:in `<top (required)>'
app_1                 |         11: from /usr/local/bundle/gems/railties-6.1.7.10/lib/rails/commands.rb:3:in `require'
app_1                 |         10: from /usr/local/bundle/gems/railties-6.1.7.10/lib/rails/command.rb:3:in `<top (required)>'
app_1                 |          9: from /usr/local/bundle/gems/railties-6.1.7.10/lib/rails/command.rb:3:in `require'
app_1                 |          8: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support.rb:29:in `<top (required)>'
app_1                 |          7: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support.rb:29:in `require'
app_1                 |          6: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger.rb:3:in `<top (required)>'
app_1                 |          5: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger.rb:3:in `require'
app_1                 |          4: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger_silence.rb:5:in `<top (required)>'
app_1                 |          3: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger_silence.rb:5:in `require'
app_1                 |          2: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:8:in `<top (required)>'
app_1                 |          1: from /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:9:in `<module:ActiveSupport>'
app_1                 | /usr/local/bundle/gems/activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:16:in `<module:LoggerThreadSafeLevel>': uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError)
app_1                 | 

Motivation:

CI is red on master

Change log entry
None

Additional Notes:

Commit in concurrent-ruby removing logger dependency: ruby-concurrency/concurrent-ruby@d7ce956

Pointer to this being the issue: https://stackoverflow.com/questions/79360526/uninitialized-constant-activesupportloggerthreadsafelevellogger-nameerror

The issue affects Rails 6.1 and 7.0. 7.0 is fixed in rails/rails#54263. I take it 6.1 is not going to be fixed ever. When the next release after 7.0.8.7 comes out we should be able to remove the pin on concurrent-ruby from Rails 7.0 applications.

How to test the change?
Existing tests

@p-datadog p-datadog requested a review from a team as a code owner January 16, 2025 17:28
@github-actions github-actions bot added the dev/testing Involves testing processes (e.g. RSpec) label Jan 16, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Jan 16, 2025

Datadog Report

Branch report: rails-6-logger-fix
Commit report: 70c5145
Test service: dd-trace-rb

✅ 0 Failed, 22200 Passed, 1477 Skipped, 5m 23.26s Total Time

@p-datadog p-datadog changed the title Work around missing logger dependency in Rails 6 Work around missing logger dependency in Rails 6 and 7.0 Jan 16, 2025
@pr-commenter
Copy link

pr-commenter bot commented Jan 16, 2025

Benchmarks

Benchmark execution time: 2025-01-16 18:17:30

Comparing candidate commit 70c5145 in PR branch rails-6-logger-fix with baseline commit fe9272b in branch master.

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

scenario:profiler - Allocations ()

  • 🟥 throughput [-306483.271op/s; -292427.555op/s] or [-9.149%; -8.729%]

@p-datadog p-datadog changed the title Work around missing logger dependency in Rails 6 and 7.0 Work around missing logger dependency in Rails 6.1 and 7.0 Jan 16, 2025
@marcotc marcotc enabled auto-merge (squash) January 16, 2025 18:42
@marcotc marcotc disabled auto-merge January 16, 2025 18:45
@marcotc marcotc merged commit b5cf594 into master Jan 16, 2025
376 of 377 checks passed
@marcotc marcotc deleted the rails-6-logger-fix branch January 16, 2025 18:45
@github-actions github-actions bot added this to the 2.10.0 milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/testing Involves testing processes (e.g. RSpec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants