Skip to content

Fix caching to cover whole Service Ping payload

Mikołaj Wawrzyniak requested to merge mwaw/fix_service_usage_data_caching into master

What does this MR do and why?

Fixes https://gitlab.com/gitlab-org/gitlab/-/issues/387818

Service Ping payload presented in admin section http://gdk.test:3000/admin/application_settings/service_usage_data is trying to use pre generated data first looking into cache, then into db and lastly generating new payload. Linked issue describe situation when multiple entities are missing form the payload. That is caused by write to cache only including legacy Service Ping payload generation via UsageData class, but not currently recommended tool: instrumentation classes

This MR changes write to cache to include complete payload from both legacy and current source

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Reproduce bug

On master branch

  1. Visit http://gdk.test:3000/admin/application_settings/metrics_and_profiling
  2. Check if Service Ping is enabled at Usage Statistics section Screenshot_2023-03-17_at_10.45.49
  3. If Service Ping was not enabled click Preview payload to generate payload
  4. Visit http://gdk.test:3000/admin/application_settings/service_usage_data
  5. Click preview payload button
  6. Notice that redis_hll_counters key is missing

Check fix in MR

  1. Enter rails console bin/rails c
  2. Clear cache Rails.cache.delete('usage_data')
  3. Visit http://gdk.test:3000/admin/application_settings/metrics_and_profiling
  4. Click Preview payload to generate payload
  5. Visit http://gdk.test:3000/admin/application_settings/service_usage_data
  6. Click preview payload button
  7. Notice that redis_hll_counters key is present and non empty this time

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mikołaj Wawrzyniak

Merge request reports