Merge branch '300156-external-request-tracking-errors-on-ipv6-host' into 'master'

Handle IPv6 hostname in ExternalHTTP instrumenter

See merge request gitlab-org/gitlab!52691

changelogs/unreleased/300156-external-request-tracking-errors-on-ipv6-host.yml

+---
+title: Handle IPv6 hostname in ExternalHTTP instrumenter
+merge_request: 52691
+author:
+type: fixed

lib/peek/views/external_http.rb

@@ -30,15 +30,9 @@ module Peek
       end
 
       def format_call_details(call)
-        uri = URI("")
-        uri.scheme = call[:scheme]
-        uri.host = call[:host]
-        uri.port = call[:port]
-        uri.path = call[:path]
-        uri.query = call[:query]
-
+        full_path = generate_path(call)
         super.merge(
-          label: "#{call[:method]} #{uri}",
+          label: "#{call[:method]} #{full_path}",
           code: code(call),
           proxy: proxy(call),
           error: error(call)
@@ -82,6 +76,22 @@ module Peek
           nil
         end
       end
+
+      def generate_path(call)
+        uri = URI("")
+        uri.scheme = call[:scheme]
+        # The host can be a domain, IPv4 or IPv6.
+        # Ruby handle IPv6 for us at
+        # https://github.com/ruby/ruby/blob/v2_6_0/lib/uri/generic.rb#L662
+        uri.hostname = call[:host]
+        uri.port = call[:port]
+        uri.path = call[:path]
+        uri.query = call[:query]
+
+        uri.to_s
+      rescue URI::Error
+        'unknown'
+      end
     end
   end
 end

spec/lib/peek/views/external_http_spec.rb

@@ -84,4 +84,108 @@ RSpec.describe Peek::Views::ExternalHttp, :request_store do
       results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) }
     ).to match_array(expected)
   end
+
+  context 'when the host is in IPv4 format' do
+    it 'displays IPv4 in the label' do
+      subscriber.request(
+        double(:event, payload: {
+          method: 'POST', code: "200", duration: 0.03,
+          scheme: 'https', host: '1.2.3.4', port: 80, path: '/api/v4/projects',
+          query: 'current=true'
+        })
+      )
+      expect(
+        subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) }
+      ).to match_array(
+        [
+          {
+            duration: 30.0,
+            label: "POST https://1.2.3.4:80/api/v4/projects?current=true",
+            code: "Response status: 200",
+            proxy: nil,
+            error: nil,
+            warnings: []
+          }
+        ]
+      )
+    end
+  end
+
+  context 'when the host is in IPv6 foramat' do
+    it 'displays IPv6 in the label' do
+      subscriber.request(
+        double(:event, payload: {
+          method: 'POST', code: "200", duration: 0.03,
+          scheme: 'https', host: '2606:4700:90:0:f22e:fbec:5bed:a9b9', port: 80, path: '/api/v4/projects',
+          query: 'current=true'
+        })
+      )
+      expect(
+        subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) }
+      ).to match_array(
+        [
+          {
+            duration: 30.0,
+            label: "POST https://[2606:4700:90:0:f22e:fbec:5bed:a9b9]:80/api/v4/projects?current=true",
+            code: "Response status: 200",
+            proxy: nil,
+            error: nil,
+            warnings: []
+          }
+        ]
+      )
+    end
+  end
+
+  context 'when the host is invalid' do
+    it 'displays unknown in the label' do
+      subscriber.request(
+        double(:event, payload: {
+          method: 'POST', code: "200", duration: 0.03,
+          scheme: 'https', host: '!@#%!@#%!@#%', port: 80, path: '/api/v4/projects',
+          query: 'current=true'
+        })
+      )
+      expect(
+        subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) }
+      ).to match_array(
+        [
+          {
+            duration: 30.0,
+            label: "POST unknown",
+            code: "Response status: 200",
+            proxy: nil,
+            error: nil,
+            warnings: []
+          }
+        ]
+      )
+    end
+  end
+
+  context 'when another URI component is invalid' do
+    it 'displays unknown in the label' do
+      subscriber.request(
+        double(:event, payload: {
+          method: 'POST', code: "200", duration: 0.03,
+          scheme: 'https', host: 'invalid', port: 'invalid', path: '/api/v4/projects',
+          query: 'current=true'
+        })
+      )
+      expect(
+        subject.results[:details].map { |data| data.slice(:duration, :label, :code, :proxy, :error, :warnings) }
+      ).to match_array(
+        [
+          {
+            duration: 30.0,
+            label: "POST unknown",
+            code: "Response status: 200",
+            proxy: nil,
+            error: nil,
+            warnings: []
+          }
+        ]
+      )
+    end
+  end
 end