Skip to content

Reduce App Server Logging of HTTP Requests - #74194

Open
sureshc wants to merge 2 commits into
stagingfrom
infrastructure/reduce-app-server-http-logging
Open

Reduce App Server Logging of HTTP Requests#74194
sureshc wants to merge 2 commits into
stagingfrom
infrastructure/reduce-app-server-http-logging

Conversation

@sureshc

@sureshc sureshc commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Reduce CloudWatch log costs while retaining the ability to increase logging in real-time during investigation of an issue.

  • Enable lograge (compact Rails HTTP request logging) on more environment types
  • Disable ActionView logging because holy hell that shit is verbose
  • Conditionally log HTTP requests based on a DCDO HTTP request log level setting with environment-specific defaults
| Status | Severity | Emitted when threshold is… |
|--------|----------|----------------------------|
| 5xx    | `error`  | `error`, `warn`, or `info` |
| 4xx    | `warn`   | `warn` or `info`           |
| 2xx/3xx| `info`   | `info` only                |

Testing story

Deployment notes

Privacy and security

Co-Authored-By: Claude <claude@users.noreply.github.com>
@sureshc
sureshc marked this pull request as ready for review July 29, 2026 16:47
@sureshc
sureshc requested a review from a team July 29, 2026 16:48
module Cdo
# Replaces lograge's fixed-severity, always-emit logging with the status-based
# policy above. Prepended onto Lograge::RequestLogSubscriber (whose
# process_exception is an alias of process_action, so this covers both).

@cat5inthecradle cat5inthecradle Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this how prepending a module actually works? Or will the process_exception alias still point to the original Lograge implementation?

The tests only execute process_action

RequestStore.store[Cdo::HttpRequestLogging::EXCEPTION_KEY]
# Copy before filtering: CDO.filter_backtrace mutates in place and the
# same exception is reported to Sentry/HoneyBadger.
data[:backtrace] = CDO.filter_backtrace(exception.backtrace.map(&:dup)) if exception&.backtrace

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codex is worried about this possibly being too big and getting truncated into malformed JSON.

Large 5xx backtraces will become malformed JSON
The PR inserts the complete backtrace into one structured log record. The “filter” merely rewrites paths and removes rake frames—it imposes no length limit—and the CEE formatter emits it as one JSON line.
The repository’s rsyslog configuration does not increase the message limit, while rsyslog defaults to 8 KB and truncates oversized messages. Therefore, any sufficiently long Rails trace will arrive in CloudWatch as invalid, unparseable JSON—precisely when the log matters most. rsyslog documentation
Fix: impose a byte-aware ceiling before JSON serialization and include a backtrace_truncated marker, or increase the full pipeline’s limit and verify it end-to-end.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I think I was overzealous about wanting a stack trace. We already have Sentry and HoneyBadger for that. I'm going to rip out this functionality. It makes it unnecessarily complex and changes the web application server HTTP request log into something different from what the intention of lograge is (compact Rails request logging).

Co-Authored-By: Claude <claude@users.noreply.github.com>
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.

2 participants