Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
/spec/dummy/log/*.log
/spec/dummy/storage/
/spec/dummy/tmp/
.DS_Store
.rspec_status
.yardoc
20 changes: 9 additions & 11 deletions lib/dfe/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@
require 'i18n'
require 'httparty'
require 'google/cloud/bigquery'
require 'dfe/analytics/activerecord' if defined?(ActiveRecord)
require 'dfe/analytics/event_schema'
require 'dfe/analytics/fields'
require 'dfe/analytics/entities'
require 'dfe/analytics/services/entity_table_checks'
require 'dfe/analytics/services/checksum_calculator'
require 'dfe/analytics/services/generic_checksum_calculator'
require 'dfe/analytics/services/postgres_checksum_calculator'
require 'dfe/analytics/shared/service_pattern'
require 'dfe/analytics/event'
require 'dfe/analytics/event_matcher'
Expand All @@ -28,7 +25,6 @@
require 'dfe/analytics/big_query_api'
require 'dfe/analytics/big_query_legacy_api'
require 'dfe/analytics/azure_federated_auth'
require 'dfe/analytics/transaction_changes'

module DfE
module Analytics
Expand Down Expand Up @@ -96,19 +92,21 @@ def self.configure
end

def self.initialize!
unless defined?(ActiveRecord)
# bail if we don't have AR at all
Rails.logger.error('ActiveRecord not loaded; DfE Analytics not initialized')
return
end

unless Rails.env.production? || File.exist?(Rails.root.join('config/initializers/dfe_analytics.rb'))
message = "Warning: DfE Analytics is not set up. Run: 'bundle exec rails generate dfe:analytics:install'"
Rails.logger.error(message)
puts message
return
end

if defined?(ActiveRecord)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We may want to introduce a config item, such as send_db_events to control this rather than just relying on ActiveRecord being present or not. TBD.

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.

I did try that but the gem code would throw an error while loading in the Rails app. I agree that a config item would be nice, but maybe we can do it as an improvement in the future.

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.

What was the error?

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.

I got a NameError for ActiveRecord when starting the Rails server. I think that happened when the gem code was loaded before the config got read.

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.

@asatwal I think we could look at the config issue later, I will have a tilt at it myself. If this has been tested @hrmtl with an app running ActiveRecord we can approve

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.

@goodviber Thanks. I haven't tested it with an app running ActiveRecord. Does the test suite not cover that?

setup_entities
else
Rails.logger.info('ActiveRecord not loaded; DfE Analytics will only track non-database requests.')
end
end

def self.setup_entities
if Rails.version.to_f > 7.1
ActiveRecord::Base.with_connection do |connection|
raise ActiveRecord::PendingMigrationError if connection.pool.migration_context.needs_migration?
Expand Down
7 changes: 7 additions & 0 deletions lib/dfe/analytics/activerecord.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

require 'dfe/analytics/services/entity_table_checks'
require 'dfe/analytics/services/checksum_calculator'
require 'dfe/analytics/services/generic_checksum_calculator'
require 'dfe/analytics/services/postgres_checksum_calculator'
require 'dfe/analytics/transaction_changes'
12 changes: 11 additions & 1 deletion spec/dfe/analytics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
it 'recovers and logs' do
hide_const('ActiveRecord')

expect(Rails.logger).to receive(:error).with(/ActiveRecord not loaded/)
expect(Rails.logger).to receive(:info).with(/ActiveRecord not loaded; DfE Analytics will only track web requests/)
expect { DfE::Analytics.initialize! }.not_to raise_error
end
end
Expand Down Expand Up @@ -330,4 +330,14 @@
expect(DfE::Analytics.all_entities_in_application.first.to_s).to match(/with_model_candidates/)
end
end

describe 'when ActiveRecord is not available' do
before do
hide_const('ActiveRecord')
end

it 'initializes without errors' do
expect { DfE::Analytics.initialize! }.not_to raise_error
end
end
end