Merge pull request #103 from ChrisBr/refactoring

Several refactorings

Author: hennevogel

lib/influxdb/rails/metric.rb

@@ -16,8 +16,6 @@ def write
         return unless enabled?
 
         client.write_point configuration.measurement_name, options
-      rescue StandardError => e
-        ::Rails.logger.error("[InfluxDB::Rails] Unable to write points: #{e.message}")
       end
 
       private

lib/influxdb/rails/middleware/subscriber.rb

@@ -16,6 +16,14 @@ def initialize(configuration, hook_name)
         end
 
         def call(_name, start, finish, _id, payload)
+          write_metric(start, finish, payload)
+        rescue StandardError => e
+          ::Rails.logger.error("[InfluxDB::Rails] Unable to write points: #{e.message}")
+        end
+
+        private
+
+        def write_metric(start, finish, payload)
           InfluxDB::Rails::Metric.new(
             values:        values(start, finish, payload),
             tags:          tags(payload),
@@ -25,8 +33,6 @@ def call(_name, start, finish, _id, payload)
           ).write
         end
 
-        private
-
         def tags(*)
           raise NotImplementedError, "must be implemented in subclass"
         end

spec/requests/action_controller_metrics_spec.rb

@@ -7,7 +7,6 @@
     end
   end
   before do
-    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
     allow_any_instance_of(ActionDispatch::Request).to receive(:request_id).and_return(:request_id)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:application_name).and_return(:app_name)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:tags_middleware).and_return(tags_middleware)

spec/requests/action_view_collection_metrics_spec.rb

@@ -7,7 +7,6 @@
     end
   end
   before do
-    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
     allow_any_instance_of(ActionDispatch::Request).to receive(:request_id).and_return(:request_id)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:application_name).and_return(:app_name)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:tags_middleware).and_return(tags_middleware)

spec/requests/action_view_partial_metrics_spec.rb

@@ -7,7 +7,6 @@
     end
   end
   before do
-    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
     allow_any_instance_of(ActionDispatch::Request).to receive(:request_id).and_return(:request_id)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:application_name).and_return(:app_name)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:tags_middleware).and_return(tags_middleware)

spec/requests/action_view_template_metrics_spec.rb

@@ -7,7 +7,6 @@
     end
   end
   before do
-    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
     allow_any_instance_of(ActionDispatch::Request).to receive(:request_id).and_return(:request_id)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:application_name).and_return(:app_name)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:tags_middleware).and_return(tags_middleware)

spec/requests/active_record_metrics_spec.rb

@@ -7,7 +7,6 @@
     end
   end
   before do
-    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
     allow_any_instance_of(ActionDispatch::Request).to receive(:request_id).and_return(:request_id)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:application_name).and_return(:app_name)
     allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:tags_middleware).and_return(tags_middleware)

spec/requests/context_spec.rb

@@ -1,28 +1,27 @@
 require File.dirname(__FILE__) + "/../spec_helper"
 
 RSpec.describe "Context", type: :request do
-  before do
-    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
-  end
-
-  it "resets the context between requests" do
+  it "resets the context after a request" do
     get "/metrics"
 
     expect_metric(
       tags: a_hash_including(
-        method: "MetricsController#index",
-        hook:   "process_action"
+        location: "MetricsController#index",
+        hook:     "sql"
       )
     )
 
-    get "/exceptions"
+    expect(InfluxDB::Rails.current.tags).to be_empty
+    expect(InfluxDB::Rails.current.values).to be_empty
+  end
 
-    expect_metric(
-      name: "rails",
-      tags: a_hash_including(
-        method: "ExceptionsController#index",
-        hook:   "process_action"
-      )
-    )
+  it "resets the context after a request when exceptioni occurs" do
+    setup_broken_client
+
+    get "/metrics"
+
+    expect_no_metric(hook: "process_action")
+    expect(InfluxDB::Rails.current.tags).to be_empty
+    expect(InfluxDB::Rails.current.values).to be_empty
   end
 end

spec/requests/logger_spec.rb

@@ -0,0 +1,10 @@
+require File.dirname(__FILE__) + "/../spec_helper"
+
+RSpec.describe "Logger", type: :request do
+  it "logs exception" do
+    setup_broken_client
+    expect(Rails.logger).to receive(:error).with(/message/).at_least(:once)
+
+    get "/metrics"
+  end
+end

spec/spec_helper.rb

@@ -2,6 +2,7 @@
 $LOAD_PATH.unshift(File.dirname(__FILE__))
 require "active_support"
 require File.expand_path(File.dirname(__FILE__) + "/support/matchers")
+require File.expand_path(File.dirname(__FILE__) + "/support/broken_client")
 
 ENV["RAILS_ENV"] ||= "test"
 
@@ -34,6 +35,8 @@
     InfluxDB::Rails.configure
 
     allow(InfluxDB::Rails).to receive(:client).and_return(InfluxDB::Rails::TestClient.new)
+    allow_any_instance_of(InfluxDB::Rails::Configuration).to receive(:ignored_environments).and_return(%w[development])
+
     InfluxDB::Rails::TestClient.metrics.clear
   end
 
@@ -43,4 +46,5 @@
 
   config.include ActiveSupport::Testing::TimeHelpers
   config.include InfluxDB::Rails::Matchers
+  config.include InfluxDB::Rails::BrokenClient
 end

spec/support/broken_client.rb

@@ -0,0 +1,13 @@
+require File.expand_path(File.dirname(__FILE__) + "/test_client")
+
+module InfluxDB
+  module Rails
+    module BrokenClient
+      def setup_broken_client
+        client = double
+        allow(client).to receive(:write_point).and_raise("message")
+        allow(InfluxDB::Rails).to receive(:client).and_return(client)
+      end
+    end
+  end
+end