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

Emit log message for incompatible Lograge setup #3839

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Aug 9, 2024

Closes #2012

This PR emits an error-level log message when the lograge gem is used with Rails tagged logging, which is configured by default in Rails since Rails 5.

We can't practically fix the issue here in dd-trace-rb, since the issue is a log formatting conflict between lograge and Rails.
The reason this affects APM Tracing is because it breaks trace-log correlation.

The only alternatives are to either: disable Rails tagged logging; or not use lograge.

The investigation and suggest alternatives are well documented here: #2012 (comment)

@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Aug 9, 2024
@pr-commenter
Copy link

pr-commenter bot commented Aug 9, 2024

Benchmarks

Benchmark execution time: 2024-08-12 23:14:11

Comparing candidate commit 482b48a in PR branch lograge-rails with baseline commit 6b50227 in branch master.

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

scenario:profiler - Allocations ()

  • 🟩 throughput [+165850.392op/s; +173043.914op/s] or [+4.230%; +4.414%]

scenario:profiler - sample timeline=false

  • 🟩 throughput [+0.257op/s; +0.396op/s] or [+3.886%; +5.980%]

@marcotc marcotc force-pushed the lograge-rails branch 2 times, most recently from 83c0965 to d7e71dc Compare August 12, 2024 22:04
@marcotc marcotc changed the title Lograge warning Emit log message for incompatible Lograge setup Aug 12, 2024
@marcotc marcotc marked this pull request as ready for review August 12, 2024 22:34
@marcotc marcotc requested a review from a team as a code owner August 12, 2024 22:34
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.84%. Comparing base (6b50227) to head (482b48a).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3839      +/-   ##
==========================================
- Coverage   97.84%   97.84%   -0.01%     
==========================================
  Files        1264     1264              
  Lines       75682    75696      +14     
  Branches     3720     3723       +3     
==========================================
+ Hits        74053    74061       +8     
- Misses       1629     1635       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marcotc marcotc merged commit 93a2891 into master Aug 13, 2024
186 checks passed
@marcotc marcotc deleted the lograge-rails branch August 13, 2024 19:11
@github-actions github-actions bot added this to the 2.3.0 milestone Aug 13, 2024
@TonyCTHsu TonyCTHsu mentioned this pull request Aug 22, 2024
@armstrjare
Copy link

We've started getting this error message since this update.

However, we already account for this with customisations to the log formatter to output in JSON and include Datadog tags:
Part of our implementation for this is made public here: https://github.com/armstrjare/rails-json-tagged-logging

Can we add a way to disable this warning to stop polluting our startup logs?

@marcotc @TonyCTHsu

@alexevanczuk
Copy link
Contributor

Hey @marcotc just wanted to +1 for a way to disable this warning message.

I'm also confused about the exact behavior that's broken. We set up some log remapping that ensures the datadog trace IDs are properly read as trace IDs and we still have log correlation working. Is there anything else that might be missing with lograge that I'm not seeing?

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lograge in JSON mode in Rails causes double addition of log correlation information and breaks log parsing
5 participants