Skip to content

Add daily coverage data new finder [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Max Orefice requested to merge mo-add-new-coverage-data-finder into master

Ref: #293825 (closed)

This feature will be behind a feature flag called coverage_data_new_finder

This is Step #2 to refactor our daily coverage data finder.

What does this MR do?

This MR creates a new finder which refactors how we fetch our daily coverage data at the project level.

It includes the following items:

  • Add new finder
  • Introduce new feature flag
  • Hook up new finder in controller

Why are we doing this?

As part of our effort to reduce our ~"technical debt", we are refactoring our daily coverage finder

This finder is used at 2 different places, so we decided to introduce a feature flag to mitigate the risk of ~performance degradation.

  1. Aggregate daily coverage by project - https://gitlab.com/gitlab-org/gitlab/-/graphs/master/charts
  2. Aggregate daily coverage by group - https://gitlab.com/groups/gitlab-org/-/analytics/repository_analytics

We are currently adding a new group_id column to fetch our data by group (!53494 (merged)).

In a follow up MR, we will create a specific EE finder to aggregate our daily coverage data by group.

Click to see how it will look like 👇
# frozen_string_literal: true

module EE
  module Ci
    module Testing
      # DailyBuildGroupReportResultsFinder
      #
      # Extends DailyBuildGroupReportResultsFinder
      #
      # Added arguments:
      #   params:
      #     group: integer
      module DailyBuildGroupReportResultsFinder
        extend ::Gitlab::Utils::Override

        override :execute
        def execute
          return super unless @params[:group]
          return ::Ci::DailyBuildGroupReportResult.none unless query_allowed?

          collection = Ci::DailyBuildGroupReportResult.by_group(group_id: @params[:group])
          collection = filter_report_results(collection)
          collection
        end

        private

        override :query_allowed?
        def query_allowed?
          return super unless @params[:group]

          can?(current_user, :read_group_build_report_results, @params[:group])
        end
      end
    end
  end
end

Database review

Note: This is the same query we are using with our existing finder.

SQL Query

SELECT
    ci_daily_build_group_report_results.*
FROM
    ci_daily_build_group_report_results
WHERE
    ci_daily_build_group_report_results.project_id = 278964
    AND ((data -> 'coverage') IS NOT NULL)
    AND ci_daily_build_group_report_results.default_branch = TRUE
    AND ci_daily_build_group_report_results.date BETWEEN '2020-11-11'
    AND '2021-02-09'
ORDER BY
    ci_daily_build_group_report_results.date DESC,
    ci_daily_build_group_report_results.group_name ASC

Query plan

Sort  (cost=6.48..6.48 rows=3 width=102) (actual time=2.288..2.289 rows=0 loops=1)
   Sort Key: ci_daily_build_group_report_results.date DESC, ci_daily_build_group_report_results.group_name
   Sort Method: quicksort  Memory: 25kB
   Buffers: shared hit=9 read=3
   I/O Timings: read=2.220
   ->  Index Scan using index_ci_daily_build_group_report_results_on_project_and_date on public.ci_daily_build_group_report_results  (cost=0.42..6.45 rows=3 width=102) (actual time=2.253..2.254 rows=0 loops=1)
         Index Cond: ((ci_daily_build_group_report_results.project_id = 22) AND (ci_daily_build_group_report_results.date >= '2020-11-11'::date) AND (ci_daily_build_group_report_results.date <= '2021-02-09'::date))
         Buffers: shared hit=3 read=3
         I/O Timings: read=2.220

Cold cache

Time: 17.354 ms
  - planning: 1.291 ms
  - execution: 16.063 ms
    - I/O read: 13.323 ms
    - I/O write: N/A

Shared buffers:
  - hits: 15 (~120.00 KiB) from the buffer pool
  - reads: 193 (~1.50 MiB) from the OS file cache, including disk I/O
  - dirtied: 1 (~8.00 KiB)
  - writes: 0

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Max Orefice

Merge request reports