Skip to content

Add validation for migrated events

Michał Wielich requested to merge michold-redis-uniques-migr-val into master

What does this MR do and why?

Add validation for migrated events

Further explained here

With #415139 (closed), we are adding new Redis keys generation logic for events. Since we will need to simultaneously be able to generate old keys for legacy events & new keys for the new events, this introduces some danger into the code. With this MR, we want to make sure that it will not be possible to mistakenly use new key generation for an old event migrated into the new events system (internal_events). This is further explained with the validation steps.

Related to #415139 (closed)

Script used to generate hll_redis_legacy_events.yml:
comment = "# This file lists all of the events that we don't expect to have property_name set
#
# This file has been generated using the script included in
# the description of https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143240
"

internal_events = []
non_internal_events = []
Gitlab::Usage::MetricDefinition.all.each do |m|
  next unless m.available?
  if m.data_source == 'internal_events'
    internal_events +=  m.events.keys
  else
    non_internal_events += m.events.keys
  end
end ; nil
events = non_internal_events.uniq.sort - internal_events
path = Rails.root.join('lib/gitlab/usage_data_counters/hll_redis_legacy_events.yml')
File.open(path, 'w') { |f| f.write(comment + events.to_yaml) }

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

To make sure this works, we should test out both scenarios that could break the key generation logic:

  1. A new legacy event gets introduced:

    • Try to trigger an event using the legacy RedisHLL call somewhere in the codebase, for example: add Gitlab::UsageDataCounters::HLLRedisCounter.track_event('new_event', values: 1) here
    • Make sure that the event is defined, for example: add - 'new_event' line after this line
    • Trigger the code, for example: bundle exec rspec spec/controllers/admin/plan_limits_controller_spec.rb
    • This should raise a UnknownLegacyEventError error with instructions to add the event name to hll_redis_legacy_events.yml.
  2. A legacy event that got migrated to internal events:

    • Try to trigger an existing non-internal event using the InternalEvents call somewhere in the codebase, for example: add Gitlab::InternalEvents.track_event('error_tracking_view_details', user: User.first) here
    • Modify at least one metric definition that includes the chosen event according to step 5 here, for example: at the end of this file, add this:
events:
  - name: error_tracking_view_details
    unique: user.id
  • Trigger the code, for example: bundle exec rspec spec/controllers/admin/plan_limits_controller_spec.rb
  • This should raise a UnfinishedEventMigrationError error with instructions to remove the event name from hll_redis_legacy_events.yml and add it to hll_redis_key_overrides.yml.
Edited by Michał Wielich

Merge request reports