From 138cd298585ccc8bd7045303622d1af7ab59689f Mon Sep 17 00:00:00 2001 From: Dimitrij Denissenko Date: Thu, 16 Mar 2017 10:17:11 +0000 Subject: [PATCH] HACK: support custom error handlers for grape instrumentation (https://github.com/ruby-grape/grape/issues/1496) --- Gemfile.lock | 30 +++++++++---------- lib/datadog/notifications/plugins/grape.rb | 11 +++++-- lib/datadog/notifications/version.rb | 2 +- .../notifications/plugins/grape_spec.rb | 28 +++++++++++++++++ spec/spec_helper.rb | 5 +++- 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 96357f2..0bfb5ae 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,13 +8,13 @@ PATH GEM remote: https://rubygems.org/ specs: - activemodel (5.0.1) - activesupport (= 5.0.1) - activerecord (5.0.1) - activemodel (= 5.0.1) - activesupport (= 5.0.1) + activemodel (5.0.2) + activesupport (= 5.0.2) + activerecord (5.0.2) + activemodel (= 5.0.2) + activesupport (= 5.0.2) arel (~> 7.0) - activesupport (5.0.1) + activesupport (5.0.2) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (~> 0.7) minitest (~> 5.1) @@ -24,16 +24,16 @@ GEM descendants_tracker (~> 0.0.4) ice_nine (~> 0.11.0) thread_safe (~> 0.3, >= 0.3.1) - builder (3.2.2) + builder (3.2.3) coercible (1.0.0) descendants_tracker (~> 0.0.1) - concurrent-ruby (1.0.4) + concurrent-ruby (1.0.5) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) - diff-lcs (1.2.5) + diff-lcs (1.3) dogstatsd-ruby (2.0.0) equalizer (0.0.11) - grape (0.19.0) + grape (0.19.1) activesupport builder hashie (>= 2.1.0) @@ -43,8 +43,8 @@ GEM rack (>= 1.3.0) rack-accept virtus (>= 1.0.0) - hashie (3.4.6) - i18n (0.7.0) + hashie (3.5.5) + i18n (0.8.1) ice_nine (0.11.2) minitest (5.10.1) multi_json (1.12.1) @@ -72,8 +72,8 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.5.0) rspec-support (3.5.0) - sqlite3 (1.3.12) - thread_safe (0.3.5) + sqlite3 (1.3.13) + thread_safe (0.3.6) tool (0.2.3) tzinfo (1.2.2) thread_safe (~> 0.1) @@ -97,4 +97,4 @@ DEPENDENCIES sqlite3 BUNDLED WITH - 1.11.2 + 1.14.5 diff --git a/lib/datadog/notifications/plugins/grape.rb b/lib/datadog/notifications/plugins/grape.rb index ac2669c..2793083 100644 --- a/lib/datadog/notifications/plugins/grape.rb +++ b/lib/datadog/notifications/plugins/grape.rb @@ -1,15 +1,17 @@ module Datadog::Notifications::Plugins class Grape < Base - attr_reader :metric_name + attr_reader :metric_name, :exception_handler # Options: # # *:metric_name - the metric name, defaults to "grape.request" + # *:exception_handler - a custom exception handler proc which accepts an exception object and returns a status # *:tags - additional tags def initialize(opts = {}) super @metric_name = opts[:metric_name] || "grape.request" + @exception_handler = opts[:exception_handler] || ->_ { 500 } Datadog::Notifications.subscribe 'endpoint_run.grape' do |reporter, event| record reporter, event @@ -24,6 +26,11 @@ def record(reporter, event) route = endpoint.route version = route.version method = route.request_method + status = endpoint.status + + if payload[:exception_object] + status = exception_handler.call(payload[:exception_object]) + end path = route.pattern.path.dup path.sub!(/\(\.\:format\)$/, '') @@ -31,7 +38,7 @@ def record(reporter, event) path.gsub!(/:(\w+)/) {|m| m[1..-1].upcase } path.gsub!(/[^\w\/\-]+/, '_') - tags = self.tags + %W|method:#{method} path:#{path} status:#{endpoint.status}| + tags = self.tags + %W|method:#{method} path:#{path} status:#{status}| tags.push "version:#{version}" if version reporter.batch do diff --git a/lib/datadog/notifications/version.rb b/lib/datadog/notifications/version.rb index 6be1f05..e0bfb22 100644 --- a/lib/datadog/notifications/version.rb +++ b/lib/datadog/notifications/version.rb @@ -1,5 +1,5 @@ module Datadog class Notifications - VERSION = "0.4.4" + VERSION = "0.4.5" end end diff --git a/spec/datadog/notifications/plugins/grape_spec.rb b/spec/datadog/notifications/plugins/grape_spec.rb index ea8a656..98e95e3 100644 --- a/spec/datadog/notifications/plugins/grape_spec.rb +++ b/spec/datadog/notifications/plugins/grape_spec.rb @@ -4,6 +4,8 @@ include Rack::Test::Methods let(:app) do + unauthorized = Class.new(RuntimeError) + sub_api = Class.new(Grape::API) do version 'v1' prefix 'api' @@ -12,10 +14,19 @@ end Class.new(Grape::API) do + + rescue_from unauthorized do |e| + error!({ message: "unauthorized", error: '401 Unauthorized' }, 401) + end + get 'echo/:key1/:key2' do "#{params['key1']} #{params['key2']}" end + get '/rescued' do + raise unauthorized.new("unauthorized") + end + namespace :sub do mount sub_api @@ -59,4 +70,21 @@ ]) end + it 'should handle rescued errors' do + get '/rescued' + expect(last_response.status).to eq(401) + + expect(buffered).to eq([ + "api.request:1|c|#custom:tag,env:test,host:test.host,more:tags,method:GET,path:/rescued,status:401", + "api.request.time:333|ms|#custom:tag,env:test,host:test.host,more:tags,method:GET,path:/rescued,status:401", + ]) + end + + it 'should not report paths on 404s' do + get '/sub/missing' + expect(last_response.status).to eq(404) + + expect(buffered).to eq([]) + end + end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bb0d62a..4c57dba 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -62,5 +62,8 @@ def buffered c.tags = ["custom:tag"] c.use Datadog::Notifications::Plugins::ActiveRecord - c.use Datadog::Notifications::Plugins::Grape, tags: ["more:tags"], metric_name: "api.request" + c.use Datadog::Notifications::Plugins::Grape, + tags: ["more:tags"], + metric_name: "api.request", + exception_handler: ->e { e.message.include?("unauthorized") ? 401 : 500 } end