Merge branch 'fix-speedscope-production' into 'master'

Fix speedscope rendering in production

See merge request gitlab-org/gitlab!60495

config/application.rb

@@ -173,8 +173,6 @@ module Gitlab
     config.assets.paths << Gemojione.images_path
     config.assets.paths << "#{config.root}/vendor/assets/fonts"
 
-    config.assets.paths << "#{config.root}/vendor/speedscope"
-
     config.assets.precompile << "application_utilities.css"
     config.assets.precompile << "application_utilities_dark.css"
     config.assets.precompile << "application_dark.css"

config/initializers/stackprof.rb

 # frozen_string_literal: true
 
-# trigger stackprof by sending a SIGUSR2 signal
-#
-# Docs: https://docs.gitlab.com/ee/development/performance.html#production
-
-module Gitlab
-  class StackProf
-    DEFAULT_FILE_PREFIX = Dir.tmpdir
-    DEFAULT_TIMEOUT_SEC = 30
-    DEFAULT_MODE = :cpu
-    # Sample interval as a frequency in microseconds (~99hz); appropriate for CPU profiles
-    DEFAULT_INTERVAL_US = 10_100
-    # Sample interval in event occurrences (n = every nth event); appropriate for allocation profiles
-    DEFAULT_INTERVAL_EVENTS = 100
-
-    # this is a workaround for sidekiq, which defines its own SIGUSR2 handler.
-    # by defering to the sidekiq startup event, we get to set up our own
-    # handler late enough.
-    # see also: https://github.com/mperham/sidekiq/pull/4653
-    def self.install
-      require 'stackprof'
-      require 'tmpdir'
-
-      if Gitlab::Runtime.sidekiq?
-        Sidekiq.configure_server do |config|
-          config.on :startup do
-            on_worker_start
-          end
-        end
-      else
-        Gitlab::Cluster::LifecycleEvents.on_worker_start do
-          on_worker_start
-        end
-      end
-    end
-
-    def self.on_worker_start
-      log_event('listening for SIGUSR2 signal')
-
-      # create a pipe in order to propagate signal out of the signal handler
-      # see also: https://cr.yp.to/docs/selfpipe.html
-      read, write = IO.pipe
-
-      # create a separate thread that polls for signals on the pipe.
-      #
-      # this way we do not execute in signal handler context, which
-      # lifts restrictions and also serializes the calls in a thread-safe
-      # manner.
-      #
-      # it's very similar to a goroutine and channel design.
-      #
-      # another nice benefit of this method is that we can timeout the
-      # IO.select call, allowing the profile to automatically stop after
-      # a given interval (by default 30 seconds), avoiding unbounded memory
-      # growth from a profile that was started and never stopped.
-      t = Thread.new do
-        timeout_s = ENV['STACKPROF_TIMEOUT_S']&.to_i || DEFAULT_TIMEOUT_SEC
-        current_timeout_s = nil
-        loop do
-          read.getbyte if IO.select([read], nil, nil, current_timeout_s)
-
-          if ::StackProf.running?
-            stackprof_file_prefix = ENV['STACKPROF_FILE_PREFIX'] || DEFAULT_FILE_PREFIX
-            stackprof_out_file = "#{stackprof_file_prefix}/stackprof.#{Process.pid}.#{SecureRandom.hex(6)}.profile"
-
-            log_event(
-              'stopping profile',
-              profile_filename: stackprof_out_file,
-              profile_timeout_s: timeout_s
-            )
-
-            ::StackProf.stop
-            ::StackProf.results(stackprof_out_file)
-            current_timeout_s = nil
-          else
-            mode = ENV['STACKPROF_MODE']&.to_sym || DEFAULT_MODE
-            interval = ENV['STACKPROF_INTERVAL']&.to_i
-            interval ||= (mode == :object ? DEFAULT_INTERVAL_EVENTS : DEFAULT_INTERVAL_US)
-
-            log_event(
-              'starting profile',
-              profile_mode: mode,
-              profile_interval: interval,
-              profile_timeout: timeout_s
-            )
-
-            ::StackProf.start(
-              mode: mode,
-              raw: Gitlab::Utils.to_boolean(ENV['STACKPROF_RAW'] || 'true'),
-              interval: interval
-            )
-            current_timeout_s = timeout_s
-          end
-        end
-      rescue StandardError => e
-        log_event("stackprof failed: #{e}")
-      end
-      t.abort_on_exception = true
-
-      # in the case of puma, this will override the existing SIGUSR2 signal handler
-      # that can be used to trigger a restart.
-      #
-      # puma cluster has two types of restarts:
-      # * SIGUSR1: phased restart
-      # * SIGUSR2: restart
-      #
-      # phased restart is not supported in our configuration, because we use
-      # preload_app. this means we will always perform a normal restart.
-      # additionally, phased restart is not supported when sending a SIGUSR2
-      # directly to a puma worker (as opposed to the master process).
-      #
-      # the result is that the behaviour of SIGUSR1 and SIGUSR2 is identical in
-      # our configuration, and we can always use a SIGUSR1 to perform a restart.
-      #
-      # thus, it is acceptable for us to re-appropriate the SIGUSR2 signal, and
-      # override the puma behaviour.
-      #
-      # see also:
-      # * https://github.com/puma/puma/blob/master/docs/signals.md#puma-signals
-      # * https://github.com/phusion/unicorn/blob/master/SIGNALS
-      # * https://github.com/mperham/sidekiq/wiki/Signals
-      Signal.trap('SIGUSR2') do
-        write.write('.')
-      end
-    end
-
-    def self.log_event(event, labels = {})
-      Gitlab::AppJsonLogger.info({
-        event: 'stackprof',
-        message: event,
-        pid: Process.pid
-      }.merge(labels.compact))
-    end
-  end
-end
-
 if Gitlab::Utils.to_boolean(ENV['STACKPROF_ENABLED'].to_s)
   Gitlab::StackProf.install
 end

lib/gitlab/middleware/speedscope.rb

@@ -8,35 +8,43 @@ module Gitlab
       end
 
       def call(env)
-        request = Rack::Request.new(env)
+        request = ActionDispatch::Request.new(env)
+
+        return @app.call(env) unless rendering_flamegraph?(request)
 
-        if request.params['performance_bar'] == 'flamegraph' && Gitlab::PerformanceBar.allowed_for_user?(request.env['warden'].user)
         body = nil
 
-          Gitlab::SafeRequestStore[:capturing_flamegraph] = true
+        ::Gitlab::SafeRequestStore[:capturing_flamegraph] = true
 
         require 'stackprof'
 
+        begin
           flamegraph = ::StackProf.run(
             mode: :wall,
             raw: true,
             aggregate: false,
-            interval: (0.5 * 1000).to_i
+            interval: ::Gitlab::StackProf::DEFAULT_INTERVAL_US
           ) do
             _, _, body = @app.call(env)
           end
-
-          path = env['PATH_INFO'].sub('//', '/')
+        ensure
           body.close if body.respond_to?(:close)
+        end
 
-          return flamegraph(flamegraph, path)
+        render_flamegraph(flamegraph, request)
       end
 
-        @app.call(env)
+      private
+
+      def rendering_flamegraph?(request)
+        request.params['performance_bar'] == 'flamegraph' && ::Gitlab::PerformanceBar.allowed_for_user?(request.env['warden']&.user)
       end
 
-      def flamegraph(graph, path)
+      def render_flamegraph(graph, request)
         headers = { 'Content-Type' => 'text/html' }
+        path = request.env['PATH_INFO'].sub('//', '/')
+
+        speedscope_url = ActionController::Base.helpers.asset_url('/-/speedscope/index.html')
 
         html = <<~HTML
           <!DOCTYPE html>
@@ -48,7 +56,7 @@ module Gitlab
               </style>
             </head>
             <body>
-              <script type="text/javascript">
+              <script type="text/javascript" nonce="#{request.content_security_policy_nonce}">
                 var graph = #{Gitlab::Json.generate(graph)};
                 var json = JSON.stringify(graph);
                 var blob = new Blob([json], { type: 'text/plain' });
@@ -56,7 +64,7 @@ module Gitlab
                 var iframe = document.createElement('IFRAME');
                 iframe.setAttribute('id', 'speedscope-iframe');
                 document.body.appendChild(iframe);
-                var iframeUrl = '#{Gitlab.config.gitlab.relative_url_root}/assets/speedscope/index.html#profileURL=' + objUrl + '&title=' + 'Flamegraph for #{CGI.escape(path)}';
+                var iframeUrl = '#{speedscope_url}#profileURL=' + objUrl + '&title=' + 'Flamegraph for #{CGI.escape(path)}';
                 iframe.setAttribute('src', iframeUrl);
               </script>
             </body>

lib/gitlab/stack_prof.rb

+# frozen_string_literal: true
+
+# trigger stackprof by sending a SIGUSR2 signal
+#
+# Docs: https://docs.gitlab.com/ee/development/performance.html#production
+
+module Gitlab
+  class StackProf
+    DEFAULT_FILE_PREFIX = Dir.tmpdir
+    DEFAULT_TIMEOUT_SEC = 30
+    DEFAULT_MODE = :cpu
+    # Sample interval as a frequency in microseconds (~99hz); appropriate for CPU profiles
+    DEFAULT_INTERVAL_US = 10_100
+    # Sample interval in event occurrences (n = every nth event); appropriate for allocation profiles
+    DEFAULT_INTERVAL_EVENTS = 100
+
+    # this is a workaround for sidekiq, which defines its own SIGUSR2 handler.
+    # by defering to the sidekiq startup event, we get to set up our own
+    # handler late enough.
+    # see also: https://github.com/mperham/sidekiq/pull/4653
+    def self.install
+      require 'stackprof'
+      require 'tmpdir'
+
+      if Gitlab::Runtime.sidekiq?
+        Sidekiq.configure_server do |config|
+          config.on :startup do
+            on_worker_start
+          end
+        end
+      else
+        Gitlab::Cluster::LifecycleEvents.on_worker_start do
+          on_worker_start
+        end
+      end
+    end
+
+    def self.on_worker_start
+      log_event('listening for SIGUSR2 signal')
+
+      # create a pipe in order to propagate signal out of the signal handler
+      # see also: https://cr.yp.to/docs/selfpipe.html
+      read, write = IO.pipe
+
+      # create a separate thread that polls for signals on the pipe.
+      #
+      # this way we do not execute in signal handler context, which
+      # lifts restrictions and also serializes the calls in a thread-safe
+      # manner.
+      #
+      # it's very similar to a goroutine and channel design.
+      #
+      # another nice benefit of this method is that we can timeout the
+      # IO.select call, allowing the profile to automatically stop after
+      # a given interval (by default 30 seconds), avoiding unbounded memory
+      # growth from a profile that was started and never stopped.
+      t = Thread.new do
+        timeout_s = ENV['STACKPROF_TIMEOUT_S']&.to_i || DEFAULT_TIMEOUT_SEC
+        current_timeout_s = nil
+        loop do
+          read.getbyte if IO.select([read], nil, nil, current_timeout_s)
+
+          if ::StackProf.running?
+            stackprof_file_prefix = ENV['STACKPROF_FILE_PREFIX'] || DEFAULT_FILE_PREFIX
+            stackprof_out_file = "#{stackprof_file_prefix}/stackprof.#{Process.pid}.#{SecureRandom.hex(6)}.profile"
+
+            log_event(
+              'stopping profile',
+              profile_filename: stackprof_out_file,
+              profile_timeout_s: timeout_s
+            )
+
+            ::StackProf.stop
+            ::StackProf.results(stackprof_out_file)
+            current_timeout_s = nil
+          else
+            mode = ENV['STACKPROF_MODE']&.to_sym || DEFAULT_MODE
+            interval = ENV['STACKPROF_INTERVAL']&.to_i
+            interval ||= (mode == :object ? DEFAULT_INTERVAL_EVENTS : DEFAULT_INTERVAL_US)
+
+            log_event(
+              'starting profile',
+              profile_mode: mode,
+              profile_interval: interval,
+              profile_timeout: timeout_s
+            )
+
+            ::StackProf.start(
+              mode: mode,
+              raw: Gitlab::Utils.to_boolean(ENV['STACKPROF_RAW'] || 'true'),
+              interval: interval
+            )
+            current_timeout_s = timeout_s
+          end
+        end
+      rescue StandardError => e
+        log_event("stackprof failed: #{e}")
+      end
+      t.abort_on_exception = true
+
+      # in the case of puma, this will override the existing SIGUSR2 signal handler
+      # that can be used to trigger a restart.
+      #
+      # puma cluster has two types of restarts:
+      # * SIGUSR1: phased restart
+      # * SIGUSR2: restart
+      #
+      # phased restart is not supported in our configuration, because we use
+      # preload_app. this means we will always perform a normal restart.
+      # additionally, phased restart is not supported when sending a SIGUSR2
+      # directly to a puma worker (as opposed to the master process).
+      #
+      # the result is that the behaviour of SIGUSR1 and SIGUSR2 is identical in
+      # our configuration, and we can always use a SIGUSR1 to perform a restart.
+      #
+      # thus, it is acceptable for us to re-appropriate the SIGUSR2 signal, and
+      # override the puma behaviour.
+      #
+      # see also:
+      # * https://github.com/puma/puma/blob/master/docs/signals.md#puma-signals
+      # * https://github.com/phusion/unicorn/blob/master/SIGNALS
+      # * https://github.com/mperham/sidekiq/wiki/Signals
+      Signal.trap('SIGUSR2') do
+        write.write('.')
+      end
+    end
+
+    def self.log_event(event, labels = {})
+      Gitlab::AppJsonLogger.info({
+        event: 'stackprof',
+        message: event,
+        pid: Process.pid
+      }.merge(labels.compact))
+    end
+  end
+end

spec/features/user_can_display_performance_bar_spec.rb

@@ -47,7 +47,7 @@ RSpec.describe 'User can display performance bar', :js do
     end
   end
 
-  let(:group) { create(:group) }
+  let_it_be(:group) { create(:group) }
 
   before do
     allow(GitlabPerformanceBarStatsWorker).to receive(:perform_in)
@@ -123,4 +123,38 @@ RSpec.describe 'User can display performance bar', :js do
       end
     end
   end
+
+  context 'flamegraphs' do
+    let_it_be(:user) { create(:user) }
+
+    before_all do
+      group.add_guest(user)
+    end
+
+    context 'when user has access' do
+      before do
+        stub_application_setting(performance_bar_allowed_group_id: group.id)
+
+        Warden.on_next_request do |proxy|
+          proxy.set_user(user)
+        end
+      end
+
+      it 'renders flamegraph when requested' do
+        visit root_path(performance_bar: 'flamegraph')
+
+        page.within_frame 'speedscope-iframe' do
+          expect(page).to have_content('Flamegraph for /')
+        end
+      end
+    end
+
+    context 'when user does not have access' do
+      it 'renders the original page' do
+        visit root_path(performance_bar: 'flamegraph')
+
+        expect(page).not_to have_selector('iframe#speedscope-iframe')
+      end
+    end
+  end
 end