Merge pull request #405 from grzuy/fix_instrumentation_event_name_format

Match ActiveSupport Instrumentation event name format spec

README.md

@@ -361,11 +361,26 @@ request.env['rack.attack.throttle_data'][name] # => { :count => n, :period => p,
 
 Rack::Attack uses the [ActiveSupport::Notifications](http://api.rubyonrails.org/classes/ActiveSupport/Notifications.html) API if available.
 
-You can subscribe to 'rack.attack' events and log it, graph it, etc:
+You can subscribe to `rack_attack` events and log it, graph it, etc.
+
+To get notified about specific type of events, subscribe to the event name followed by the `rack_attack` namesapce.
+E.g. for throttles use:
 
 ```ruby
-ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, request_id, payload|
-  puts payload[:request].inspect
+ActiveSupport::Notifications.subscribe("throttle.rack_attack") do |name, start, finish, request_id, payload|
+  # request object available in payload[:request]
+
+  # Your code here
+end
+```
+
+If you want to subscribe to every `rack_attack` event, use:
+
+```ruby
+ActiveSupport::Notifications.subscribe(/rack_attack/) do |name, start, finish, request_id, payload|
+  # request object available in payload[:request]
+
+  # Your code here
 end
 ```
 

examples/instrumentation.rb

@@ -1,3 +1,3 @@
-ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, request_id, payload|
+ActiveSupport::Notifications.subscribe(/rack_attack/) do |name, start, finish, request_id, payload|
   puts payload[:request].inspect
 end

lib/rack/attack.rb

@@ -88,7 +88,13 @@ class Rack::Attack
     end
 
     def instrument(request)
-      notifier.instrument('rack.attack', request: request) if notifier
+      if notifier
+        event_type = request.env["rack.attack.match_type"]
+        notifier.instrument("#{event_type}.rack_attack", request: request)
+
+        # Deprecated: Keeping just for backwards compatibility
+        notifier.instrument("rack.attack", request: request)
+      end
     end
 
     def cache

spec/acceptance/blocking_ip_spec.rb

@@ -23,7 +23,7 @@ describe "Blocking an IP" do
     notified = false
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload|
       notified = true
       notification_type = payload[:request].env["rack.attack.match_type"]
     end

spec/acceptance/blocking_spec.rb

@@ -25,7 +25,7 @@ describe "#blocklist" do
     notification_matched = nil
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_matched = payload[:request].env["rack.attack.matched"]
       notification_type = payload[:request].env["rack.attack.match_type"]
     end

spec/acceptance/blocking_subnet_spec.rb

@@ -29,7 +29,7 @@ describe "Blocking an IP subnet" do
     notified = false
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("blocklist.rack_attack") do |_name, _start, _finish, _id, payload|
       notified = true
       notification_type = payload[:request].env["rack.attack.match_type"]
     end

spec/acceptance/safelisting_ip_spec.rb

@@ -38,7 +38,7 @@ describe "Safelist an IP" do
   it "notifies when the request is safe" do
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_type = payload[:request].env["rack.attack.match_type"]
     end
 

spec/acceptance/safelisting_spec.rb

@@ -41,7 +41,7 @@ describe "#safelist" do
     notification_matched = nil
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_matched = payload[:request].env["rack.attack.matched"]
       notification_type = payload[:request].env["rack.attack.match_type"]
     end

spec/acceptance/safelisting_subnet_spec.rb

@@ -38,7 +38,7 @@ describe "Safelisting an IP subnet" do
   it "notifies when the request is safe" do
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("safelist.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_type = payload[:request].env["rack.attack.match_type"]
     end
 

spec/acceptance/throttling_spec.rb

@@ -125,7 +125,7 @@ describe "#throttle" do
     notification_data = nil
     notification_discriminator = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("throttle.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_matched = payload[:request].env["rack.attack.matched"]
       notification_type = payload[:request].env["rack.attack.match_type"]
       notification_data = payload[:request].env['rack.attack.match_data']

spec/acceptance/track_spec.rb

@@ -11,7 +11,7 @@ describe "#track" do
     notification_matched = nil
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_matched = payload[:request].env["rack.attack.matched"]
       notification_type = payload[:request].env["rack.attack.match_type"]
     end

spec/acceptance/track_throttle_spec.rb

@@ -14,7 +14,7 @@ describe "#track with throttle-ish options" do
     notification_matched = nil
     notification_type = nil
 
-    ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload|
+    ActiveSupport::Notifications.subscribe("track.rack_attack") do |_name, _start, _finish, _id, payload|
       notification_matched = payload[:request].env["rack.attack.matched"]
       notification_type = payload[:request].env["rack.attack.match_type"]
     end

spec/rack_attack_instrumentation_spec.rb

@@ -1,13 +1,20 @@
 # frozen_string_literal: true
 
+require_relative "spec_helper"
+
 # ActiveSupport::Subscribers added in ~> 4.0.2.0
 if ActiveSupport::VERSION::MAJOR > 3
   require_relative 'spec_helper'
   require 'active_support/subscriber'
   class CustomSubscriber < ActiveSupport::Subscriber
-    def rack(event)
-      # Do virtually (but not) nothing.
-      event.inspect
+    @notification_count = 0
+
+    class << self
+      attr_accessor :notification_count
+    end
+
+    def throttle(_event)
+      self.class.notification_count += 1
     end
   end
 
@@ -21,12 +28,14 @@ if ActiveSupport::VERSION::MAJOR > 3
     describe "with throttling" do
       before do
         ActiveSupport::Notifications.stub(:notifier, ActiveSupport::Notifications::Fanout.new) do
-          CustomSubscriber.attach_to("attack")
+          CustomSubscriber.attach_to("rack_attack")
           2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
         end
       end
+
       it 'should instrument without error' do
         last_response.status.must_equal 429
+        assert_equal 1, CustomSubscriber.notification_count
       end
     end
   end

spec/rack_attack_track_spec.rb

@@ -35,7 +35,7 @@ describe 'Rack::Attack.track' do
       # A second track
       Rack::Attack.track("homepage") { |req| req.path == "/" }
 
-      ActiveSupport::Notifications.subscribe("rack.attack") do |*_args|
+      ActiveSupport::Notifications.subscribe("track.rack_attack") do |*_args|
         Counter.incr
       end