Merge pull request #889 from stefanvermaas/main

Adds support for tracking `http.rb` HTTP requests
appsignal · Nov 18, 2022 · 7e63103 · 7e63103
2 parents e1afe79 + babc8e0
commit 7e63103
Show file tree

Hide file tree

Showing 13 changed files with 290 additions and 1 deletion.
diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml
@@ -570,6 +570,24 @@ blocks:
         value: latest
       commands:
       - "./support/bundler_wrapper exec rake test"
+    - name: Ruby 2.6.9 for http5
+      env_vars:
+      - *2
+      - *3
+      - *4
+      - *5
+      - name: RUBY_VERSION
+        value: 2.6.9
+      - name: GEMSET
+        value: http5
+      - name: BUNDLE_GEMFILE
+        value: gemfiles/http5.gemfile
+      - name: _RUBYGEMS_VERSION
+        value: latest
+      - name: _BUNDLER_VERSION
+        value: latest
+      commands:
+      - "./support/bundler_wrapper exec rake test"
     - name: Ruby 2.6.9 for padrino
       env_vars:
       - *2
@@ -923,6 +941,24 @@ blocks:
         value: latest
       commands:
       - "./support/bundler_wrapper exec rake test"
+    - name: Ruby 2.7.5 for http5
+      env_vars:
+      - *2
+      - *3
+      - *4
+      - *5
+      - name: RUBY_VERSION
+        value: 2.7.5
+      - name: GEMSET
+        value: http5
+      - name: BUNDLE_GEMFILE
+        value: gemfiles/http5.gemfile
+      - name: _RUBYGEMS_VERSION
+        value: latest
+      - name: _BUNDLER_VERSION
+        value: latest
+      commands:
+      - "./support/bundler_wrapper exec rake test"
     - name: Ruby 2.7.5 for padrino
       env_vars:
       - *2
@@ -1294,6 +1330,24 @@ blocks:
         value: latest
       commands:
       - "./support/bundler_wrapper exec rake test"
+    - name: Ruby 3.0.3 for http5
+      env_vars:
+      - *2
+      - *3
+      - *4
+      - *5
+      - name: RUBY_VERSION
+        value: 3.0.3
+      - name: GEMSET
+        value: http5
+      - name: BUNDLE_GEMFILE
+        value: gemfiles/http5.gemfile
+      - name: _RUBYGEMS_VERSION
+        value: latest
+      - name: _BUNDLER_VERSION
+        value: latest
+      commands:
+      - "./support/bundler_wrapper exec rake test"
     - name: Ruby 3.0.3 for padrino
       env_vars:
       - *2
@@ -1611,6 +1665,24 @@ blocks:
         value: latest
       commands:
       - "./support/bundler_wrapper exec rake test"
+    - name: Ruby 3.1.1 for http5
+      env_vars:
+      - *2
+      - *3
+      - *4
+      - *5
+      - name: RUBY_VERSION
+        value: 3.1.1
+      - name: GEMSET
+        value: http5
+      - name: BUNDLE_GEMFILE
+        value: gemfiles/http5.gemfile
+      - name: _RUBYGEMS_VERSION
+        value: latest
+      - name: _BUNDLER_VERSION
+        value: latest
+      commands:
+      - "./support/bundler_wrapper exec rake test"
     - name: Ruby 3.1.1 for padrino
       env_vars:
       - *2
@@ -1910,6 +1982,24 @@ blocks:
         value: latest
       commands:
       - "./support/bundler_wrapper exec rake test"
+    - name: Ruby 3.2.0-preview1 for http5
+      env_vars:
+      - *2
+      - *3
+      - *4
+      - *5
+      - name: RUBY_VERSION
+        value: 3.2.0-preview1
+      - name: GEMSET
+        value: http5
+      - name: BUNDLE_GEMFILE
+        value: gemfiles/http5.gemfile
+      - name: _RUBYGEMS_VERSION
+        value: latest
+      - name: _BUNDLER_VERSION
+        value: latest
+      commands:
+      - "./support/bundler_wrapper exec rake test"
     - name: Ruby 3.2.0-preview1 for padrino
       env_vars:
       - *2

diff --git a/README.md b/README.md
@@ -232,6 +232,7 @@ configurations you need to run the spec suite with a specific Gemfile.
 BUNDLE_GEMFILE=gemfiles/capistrano2.gemfile bundle exec rspec
 BUNDLE_GEMFILE=gemfiles/capistrano3.gemfile bundle exec rspec
 BUNDLE_GEMFILE=gemfiles/grape.gemfile bundle exec rspec
+BUNDLE_GEMFILE=gemfiles/http5.gemfile bundle exec rspec
 BUNDLE_GEMFILE=gemfiles/no_dependencies.gemfile bundle exec rspec
 BUNDLE_GEMFILE=gemfiles/padrino.gemfile bundle exec rspec
 BUNDLE_GEMFILE=gemfiles/que.gemfile bundle exec rspec

diff --git a/build_matrix.yml b/build_matrix.yml
@@ -212,6 +212,7 @@ matrix:
     - gem: "capistrano2"
     - gem: "capistrano3"
     - gem: "grape"
+    - gem: "http5"
     - gem: "padrino"
     - gem: "psych-3"
       only:

diff --git a/gemfiles/http5.gemfile b/gemfiles/http5.gemfile
@@ -0,0 +1,5 @@
+source 'https://rubygems.org'
+
+gem 'http', '~> 5.0'
+
+gemspec :path => '../'
diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb
@@ -25,6 +25,7 @@ class Config
       :ignore_actions                 => [],
       :ignore_errors                  => [],
       :ignore_namespaces              => [],
+      :instrument_http_rb             => true,
       :instrument_net_http            => true,
       :instrument_redis               => true,
       :instrument_sequel              => true,
@@ -74,6 +75,7 @@ class Config
       "APPSIGNAL_IGNORE_ACTIONS"                 => :ignore_actions,
       "APPSIGNAL_IGNORE_ERRORS"                  => :ignore_errors,
       "APPSIGNAL_IGNORE_NAMESPACES"              => :ignore_namespaces,
+      "APPSIGNAL_INSTRUMENT_HTTP_RB"             => :instrument_http_rb,
       "APPSIGNAL_INSTRUMENT_NET_HTTP"            => :instrument_net_http,
       "APPSIGNAL_INSTRUMENT_REDIS"               => :instrument_redis,
       "APPSIGNAL_INSTRUMENT_SEQUEL"              => :instrument_sequel,
@@ -119,6 +121,7 @@ class Config
       APPSIGNAL_ENABLE_MINUTELY_PROBES
       APPSIGNAL_ENABLE_STATSD
       APPSIGNAL_FILES_WORLD_ACCESSIBLE
+      APPSIGNAL_INSTRUMENT_HTTP_RB
       APPSIGNAL_INSTRUMENT_NET_HTTP
       APPSIGNAL_INSTRUMENT_REDIS
       APPSIGNAL_INSTRUMENT_SEQUEL

diff --git a/lib/appsignal/hooks.rb b/lib/appsignal/hooks.rb
@@ -94,6 +94,7 @@ def self.const_missing(name)
 require "appsignal/hooks/active_support_notifications"
 require "appsignal/hooks/celluloid"
 require "appsignal/hooks/delayed_job"
+require "appsignal/hooks/http"
 require "appsignal/hooks/mri"
 require "appsignal/hooks/net_http"
 require "appsignal/hooks/passenger"

diff --git a/lib/appsignal/hooks/http.rb b/lib/appsignal/hooks/http.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Appsignal
+  class Hooks
+    # @api private
+    class HttpHook < Appsignal::Hooks::Hook
+      register :http_rb
+
+      def dependencies_present?
+        defined?(HTTP::Client) && Appsignal.config && Appsignal.config[:instrument_http_rb]
+      end
+
+      def install
+        require "appsignal/integrations/http"
+        HTTP::Client.send(:prepend, Appsignal::Integrations::HttpIntegration)
+
+        Appsignal::Environment.report_enabled("http_rb")
+      end
+    end
+  end
+end
diff --git a/lib/appsignal/integrations/http.rb b/lib/appsignal/integrations/http.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Appsignal
+  module Integrations
+    module HttpIntegration
+      def request(verb, uri, opts = {})
+        parsed_request_uri = URI.parse(uri)
+        request_uri = "#{parsed_request_uri.scheme}://#{parsed_request_uri.host}"
+
+        begin
+          Appsignal.instrument("request.http_rb", "#{verb.upcase} #{request_uri}") do
+            super
+          end
+        rescue Exception => error # rubocop:disable Lint/RescueException
+          Appsignal.set_error(error)
+          raise error
+        end
+      end
+    end
+  end
+end
diff --git a/spec/integration/diagnose b/spec/integration/diagnose
diff --git a/spec/lib/appsignal/config_spec.rb b/spec/lib/appsignal/config_spec.rb
@@ -166,6 +166,7 @@
         :ignore_actions                 => [],
         :ignore_errors                  => [],
         :ignore_namespaces              => [],
+        :instrument_http_rb             => true,
         :instrument_net_http            => true,
         :instrument_redis               => true,
         :instrument_sequel              => true,

diff --git a/spec/lib/appsignal/hooks/http_spec.rb b/spec/lib/appsignal/hooks/http_spec.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+describe Appsignal::Hooks::HttpHook do
+  before :context do
+    start_agent
+  end
+
+  if DependencyHelper.http_present?
+    context "with instrument_http_rb set to true" do
+      describe "#dependencies_present?" do
+        subject { described_class.new.dependencies_present? }
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    context "with instrument_http_rb set to false" do
+      describe "#dependencies_present?" do
+        before { Appsignal.config.config_hash[:instrument_http_rb] = false }
+        after { Appsignal.config.config_hash[:instrument_http_rb] = true }
+        subject { described_class.new.dependencies_present? }
+
+        it { is_expected.to be_falsy }
+      end
+    end
+
+    it "installs the HTTP plugin" do
+      expect(HTTP::Client.included_modules)
+        .to include(Appsignal::Integrations::HttpIntegration)
+    end
+  else
+    describe "#dependencies_present?" do
+      subject { described_class.new.dependencies_present? }
+
+      it { is_expected.to be_falsy }
+    end
+  end
+end
diff --git a/spec/lib/appsignal/integrations/http_spec.rb b/spec/lib/appsignal/integrations/http_spec.rb
@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+if DependencyHelper.http_present?
+  require "appsignal/integrations/http"
+
+  describe Appsignal::Integrations::HttpIntegration do
+    let(:transaction) { http_request_transaction }
+
+    around do |example|
+      keep_transactions { example.run }
+    end
+
+    before do
+      set_current_transaction(transaction)
+    end
+
+    before :context do
+      start_agent
+    end
+
+    it "should instrument a HTTP request" do
+      stub_request(:get, "http://www.google.com")
+
+      HTTP.get("http://www.google.com")
+
+      transaction_hash = transaction.to_h
+      expect(transaction_hash).to include("namespace" => Appsignal::Transaction::HTTP_REQUEST)
+      expect(transaction_hash["events"].first).to include(
+        "body" => "",
+        "body_format" => Appsignal::EventFormatter::DEFAULT,
+        "name" => "request.http_rb",
+        "title" => "GET http://www.google.com"
+      )
+    end
+
+    it "should instrument a HTTPS request" do
+      stub_request(:get, "https://www.google.com")
+
+      HTTP.get("https://www.google.com")
+
+      transaction_hash = transaction.to_h
+      expect(transaction_hash).to include("namespace" => Appsignal::Transaction::HTTP_REQUEST)
+      expect(transaction_hash["events"].first).to include(
+        "body" => "",
+        "body_format" => Appsignal::EventFormatter::DEFAULT,
+        "name" => "request.http_rb",
+        "title" => "GET https://www.google.com"
+      )
+    end
+
+    context "with request parameters" do
+      it "does not include the query parameters in the title" do
+        stub_request(:get, "https://www.google.com?q=Appsignal")
+
+        HTTP.get("https://www.google.com", :params => { :q => "Appsignal" })
+
+        expect(transaction.to_h["events"].first).to include(
+          "body" => "",
+          "title" => "GET https://www.google.com"
+        )
+      end
+
+      it "does not include the request body in the title" do
+        stub_request(:post, "https://www.google.com")
+          .with(:body => { :q => "Appsignal" }.to_json)
+
+        HTTP.post("https://www.google.com", :json => { :q => "Appsignal" })
+
+        expect(transaction.to_h["events"].first).to include(
+          "body" => "",
+          "title" => "POST https://www.google.com"
+        )
+      end
+    end
+
+    context "with an HTTP exception" do
+      let(:error) { ExampleException.new("oh no!") }
+
+      it "reports the exception and re-raises it" do
+        stub_request(:get, "https://www.google.com").and_raise(error)
+
+        expect do
+          HTTP.get("https://www.google.com")
+        end.to raise_error(ExampleException)
+
+        transaction_hash = transaction.to_h
+        expect(transaction_hash).to include("namespace" => Appsignal::Transaction::HTTP_REQUEST)
+        expect(transaction_hash["events"].first).to include(
+          "body" => "",
+          "body_format" => Appsignal::EventFormatter::DEFAULT,
+          "name" => "request.http_rb",
+          "title" => "GET https://www.google.com"
+        )
+
+        expect(transaction_hash["error"]).to include(
+          "backtrace" => kind_of(String),
+          "name" => error.class.name,
+          "message" => error.message
+        )
+      end
+    end
+  end
+end
diff --git a/spec/support/helpers/dependency_helper.rb b/spec/support/helpers/dependency_helper.rb
@@ -103,6 +103,10 @@ def capistrano3_present?
       Gem.loaded_specs["capistrano"].version >= Gem::Version.new("3.0")
   end
 
+  def http_present?
+    dependency_present? "http"
+  end
+
   def que_present?
     dependency_present? "que"
   end