Skip to content

Don't send healthcheck events to AppSignal to save storage#842

Open
northeastprince wants to merge 1 commit intomainfrom
no-healthcheck-events
Open

Don't send healthcheck events to AppSignal to save storage#842
northeastprince wants to merge 1 commit intomainfrom
no-healthcheck-events

Conversation

@northeastprince
Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings March 30, 2026 23:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a Rails initializer to prevent health check controller events from being reported to AppSignal, reducing monitoring noise/storage for /up requests.

Changes:

  • Introduces a to_prepare hook that locates the AppSignal ActiveSupport event subscriber.
  • Replaces the original subscriber with a wrapper that skips emitting health check events.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +12
next unless original_subscriber_entry

subscribers.delete original_subscriber_entry

filtered_subscriber = Class.new do
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

subscribers is not defined in this initializer. As written, this will raise a NameError when original_subscriber_entry is present, and the AppSignal subscriber will never be replaced. Assign the subscribers collection to a local (e.g., from Rails.event.instance_variable_get(:@subscribers)) and delete from that collection, or otherwise use the public API (if available) to unsubscribe the original entry before re-subscribing the filtered wrapper.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +21 to +27
def with_filtered_event_reporter
original = ActiveSupport.event_reporter
ActiveSupport.event_reporter = FilteredEventReporter.new(original)
yield
ensure
ActiveSupport.event_reporter = original
end
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

This test mutates the global ActiveSupport.event_reporter. Since the test suite is configured to run in parallel (parallelize in test/test_helper.rb), this can cause cross-test interference and flaky failures when other tests emit events concurrently. Consider making this test non-parallel-safe (disable parallelization for this class) or avoid changing the global reporter by using a local subscriber/spies that only observes events within the block.

Copilot uses AI. Check for mistakes.
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