Skip to content

Remove `options` from internal event metrics vol.3

Michał Wielich requested to merge michold-ie-remove-options-v3 into master

What does this MR do and why?

As a last step to resolve #427298 (closed), we remove all options keys from existing metrics. This should not affect any metrics thanks to the previous MRs completed for this issue.

Here's the script I used to achieve that:

i = 0
Gitlab::Usage::MetricDefinition.all.sort_by(&:key_path).each do |metric|
  events = metric.attributes[:events]
  next if !events || metric.key_path.in?(["counts.web_ide_commits", "counts.commit_comment", "counts.source_code_pushes", "counts.package_events_i_package_pull_package_by_guest", "counts.package_events_i_package_push_package_by_deploy_token"]) # non-internal-events metrics that have `:events` for some reason

  i += 1
  next if (i > 300 || i <= 200)

  raise StandardError, "#{metric.key_path} not internal event" unless metric.data_source == 'internal_events'
  raise StandardError, "#{metric.key_path} mismatched config" if events.pluck(:name).sort != metric.attributes[:options][:events].sort

  new_file = ""
  is_options = false
  File.open(metric.path, "r") do |f|
    f.each_line do |line|
      if line.starts_with?("options:")
        is_options = true
      elsif !(line.starts_with?(/\s/))
        is_options = false
      end

      new_file += line unless is_options
    end
  end

  File.open(metric.path, 'w') { |f| f.write(new_file) }
end


MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

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

Before After

How to set up and validate locally

Something that we want to be fully sure about is that we're not affecting any metrics with this code. To check that, we can:

  1. Enable the gdk and perform some actions: for example, create an issue, add comments, create an epic, add an emote reaction
  2. Checkout the master branch and generate the next week's Service Ping in the rails console:
require 'active_support/testing/time_helpers'
include ActiveSupport::Testing::TimeHelpers
sp = travel_to(Date.today + 7.days) { Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) }
  1. Checkout this MR's branch and go back to the rails console:
reload!
sp2 = travel_to(Date.today + 7.days) { Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) }
  1. Select all the keys that are different between these two service pings:
def find_different_keys(h1, h2, result, current_key = '')
  raise StandardError if h1.keys != h2.keys

  diffs = h1.keys.each do |k|
    if h1[k].is_a?(Hash)
      result = find_different_keys(h1[k], h2[k], result, current_key + "#{k}.")
    else
      (h1[k] != h2[k]) ? result += [current_key + "#{k}"] : nil
    end
  end

  result
end

find_different_keys(sp, sp2, [])

This should only list metrics that are not calculated using RedisHLL. For example, in my case, it was:

["elasticsearch_enabled",
 "geo_enabled",
 "gitlab_pages.enabled",
 "database.flavor",
 "object_store.artifacts.enabled",
 "object_store.artifacts.object_store.enabled",
 "object_store.artifacts.object_store.direct_upload",
 "object_store.artifacts.object_store.background_upload",
 "object_store.external_diffs.enabled",
 "object_store.external_diffs.object_store.enabled",
 "object_store.external_diffs.object_store.direct_upload",
 "object_store.external_diffs.object_store.background_upload",
 "object_store.lfs.enabled",
 "object_store.lfs.object_store.enabled",
 "object_store.lfs.object_store.direct_upload",
 "object_store.lfs.object_store.background_upload",
 "object_store.uploads.enabled",
 "object_store.uploads.object_store.enabled",
 "object_store.uploads.object_store.direct_upload",
 "object_store.uploads.object_store.background_upload",
 "object_store.packages.enabled",
 "object_store.packages.object_store.enabled",
 "object_store.packages.object_store.direct_upload",
 "object_store.packages.object_store.background_upload",
 "topology.duration_s"]

If you got any other keys, they can be checked by searching for the key in https://metrics.gitlab.com/ [for exmaple: here]. After loading a row, make sure that the source column for it is not redis_hll or internal_events. For the given example, the source is system.

Edited by Michał Wielich

Merge request reports