Resolve "Fix broken Service Ping metric queries"
What does this MR do and why?
Describe in detail what your merge request does and why.
Resolves #353559 (closed)
summary: During work around !81002 (merged) it was identified that some of generated by https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage/service_ping_report.rb#L39 SQL queries used at https://docs.gitlab.com/ee/api/usage_data.html#export-service-ping-sql-queries have invalid SQL syntax
The syntax errors were due to the following issues
- Invalid syntax due to order_by
-
counts.labels
is the only one in this category - Labels model has a default_scope
order(title: :asc)
which interferes with thecount
sql statement - Resolution: Adding
unscope(:order)
to handle this
-
- Incorrect value datatype (nil vs 0)
- When service ping report is generated for
all_metrics_values
, it calls .send(:count) which type casts values to int - eg.
counts.jira_imports_total_imported_issues_count
returns nil when executed as query, but expects 0 - Resolution: Type casting the values to handle this
- When service ping report is generated for
- Joins with select column on joined table
- In case of joins, when the selected column is of the format
table2.column
, the generated query is of formatselect count(table1.table2.column) from ...
which is invalid syntax - eg. usage_activity_by_stage.create.projects_with_sectional_code_owner_rules
- Resolution: Checking for joins before building the query
- In case of joins, when the selected column is of the format
Screenshots or screen recordings
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
- run the test with
bin/rspec spec/lib/gitlab/usage/service_ping_report_spec.rb:94
to verify
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Tarun Vellishetty