Add support for ActiveSupport::EventReporter #1481
Conversation
jeffkreeftmeijer
force-pushed
the
event-subscriber
branch
from
e5c6880 to
051180b
Compare
jeffkreeftmeijer
requested review from
lipskis,
matsimitsu,
tombruijn and
unflxw
.changesets/report-events-from-rails-8-1-s-activesupport--eventreporter-as-logs.md
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
lib/appsignal/config.rb
Outdated
| :enable_gvl_global_timer => true, | ||
| :enable_gvl_waiting_threads => true, | ||
| :enable_rails_error_reporter => true, | ||
| :enable_active_support_event_reporter => true, |
There was a problem hiding this comment.
Nit: maybe we should make the option specific for logging in case we want to add another reporter that does add events to the transaction events later?
| :enable_active_support_event_reporter => true, | |
| :enable_active_support_event_log_reporter => true, |
This is a message from the daily scheduled checks. |
matsimitsu
left a comment
matsimitsu
left a comment
There was a problem hiding this comment.
Note that these events as they are right now, will probably not be accepted by our log endpoint, since it doesn't contain a few required fields (message being one of them).
But that's a problem for team processor to solve at some point.
We might need a way to filter these in the future. This event system will be used for multiple purposes (including by ourselves).
For example, right now we use PostHog to track certain actions in the back-end. We'd like to migrate that to these events, so that if we ever switch from PostHog to something else, we just need to add a subscriber and go on with our lives.
This means that we can have events that are interesting for logs, and events that aren't (I don't need to log every time a user has created a dashboard, for example). So I think some kind of filtering will be needed at some point (or maybe filtering on log ingestion?)
My reading of it from the implementation is that the event name is used as the log message. Other required attributes (group, hostname, etc) should be added automatically by the AppSignal logger. |
The event name is used as the message, so all lines have one set. I've seen these logs be reported into AppSignal. Anything I should check to make sure this all works?
Anything we should do for this now from the integration side? |
If people use it to track usage, then the message doesn't provide much value. But maybe that's not a problem after we do: https://3.basecamp.com/4738529/buckets/18166700/messages/9330559990
No not yet, but I wouldn't be surprised if people want some filter ability eventually :) |
|
Alright, I'm finishing up work on this. @matsimitsu are we alright merging and releasing this as-is? Note that this will add log lines for all Ruby users that update to the new version. However, we have an option to opt out of that ( I'm also working on adding documentation for this feature, but I don't think that should block releasing this already. |
|
Yeah I see no issues (well apart from the log message, but not much we can do about that I guess) |
Rails 8.1 adds ActiveSupport::EventReporter, an interface for reporting structured events. To catch these, report them as info-level logs in the rails_events category through the newly added Appsignal::Integrations::ActiveSupportEventReporter.
…treporter-as-logs.md Co-authored-by: Tom de Bruijn <[email protected]>
Co-authored-by: Tom de Bruijn <[email protected]>
Co-authored-by: Tom de Bruijn <[email protected]>
Add a configuration option to allow users to opt out of the ActiveSupport::EventReporter integration. The option is enabled by default and can be disabled via APPSIGNAL_ENABLE_ACTIVE_SUPPORT_EVENT_REPORTER or enable_active_support_event_reporter.
jeffkreeftmeijer
force-pushed
the
event-subscriber
branch
from
0486f7d to
48beaa6
Compare
Renames the configuration option from `enable_active_support_event_reporter` to `enable_active_support_event_log_reporter` to better reflect its purpose of reporting events as logs. Changes: - Updated config default and BOOLEAN_OPTIONS mapping - Updated environment variable name to APPSIGNAL_ENABLE_ACTIVE_SUPPORT_EVENT_LOG_REPORTER - Updated YARD documentation - Updated all test fixtures and specs
jeffkreeftmeijer
force-pushed
the
event-subscriber
branch
from
48beaa6 to
17c42ff
Compare
The ActiveSupportEventReporterHook was attempting to call Rails.event.subscribe even in non-Rails environments where Rails is not defined, causing a NameError that would be logged as an error during hook installation. This caused test failures when running the grape gemfile suite. The fix adds a check for Rails being defined in the dependencies_present? method, ensuring the hook only attempts to install in Rails applications where Rails.event is available. This prevents the hook from trying to install in applications that use ActiveSupport (like Grape) but don't have Rails defined.
Add enable_active_support_event_log_reporter config option to both RBI and RBS type signature files.
Add enable_active_support_event_log_reporter configuration option to the diagnose integration test expectations.
Break long regex patterns onto separate lines to fix Layout/LineLength rubocop offenses.
Update the test to properly handle the case where ActiveSupport is loaded without Rails. The test now correctly mocks the event reporter and its subscribers array to verify the hook installation works in non-Rails environments.
Replace the ad-hoc `defined?(::Rails)` check with `DependencyHelper.rails8_1_present?` to ensure the #install test block only runs in appropriate Rails 8.1+ environments. This fixes test failures in JRuby with the no_dependencies gemfile where Rails may not be properly defined.
Update the regex patterns in the diagnose spec to handle JRuby's JSON parse error messages. JRuby returns "unexpected token at 'invalid..." with the full invalid content in the quote, while MRI Ruby returns "unexpected token at 'invalid'" with just the first token. Changed the pattern from matching a closing quote after 'invalid' to just matching the word 'invalid', making it compatible with both MRI Ruby and JRuby error message formats.
Rails 8.1 adds
ActiveSupport::EventReporter, an interface for reporting structured events. To catch these, report them as info-level logs in therails_events categorythrough the newly addedAppsignal::Integrations::ActiveSupportEventReporter.