From 6eca88ce530d93dea569a083ec19ea394f17d59c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Wielich?= <mwielich@gitlab.com>
Date: Mon, 26 Feb 2024 16:56:20 +0000
Subject: [PATCH] Deprecate `option` key for internal_events yaml
Deprecate `option` key for internal_events yaml
---
config/metrics/schema/internal_events.json | 18 +-----------------
.../migration.md | 3 ++-
.../cli/helpers/metric_options.rb | 6 ++++--
scripts/internal_events/cli/metric.rb | 5 ++---
scripts/internal_events/cli/metric_definer.rb | 4 ++--
.../metrics/ee_total_28d_single_event.yml | 3 ---
.../metrics/ee_total_7d_single_event.yml | 3 ---
.../metrics/ee_total_single_event.yml | 3 ---
.../metrics/keyboard_smashed_metric_28d.yml | 3 ---
.../metrics/keyboard_smashed_metric_7d.yml | 3 ---
.../metrics/project_id_28d_multiple_events.yml | 4 ----
.../metrics/project_id_7d_multiple_events.yml | 4 ----
.../metrics/total_single_event.yml | 3 ---
.../metrics/user_id_28d_single_event.yml | 3 ---
.../metrics/user_id_7d_single_event.yml | 3 ---
spec/scripts/internal_events/cli_spec.rb | 3 +--
16 files changed, 12 insertions(+), 59 deletions(-)
diff --git a/config/metrics/schema/internal_events.json b/config/metrics/schema/internal_events.json
index d35a2fa22d75e..d09219392736f 100644
--- a/config/metrics/schema/internal_events.json
+++ b/config/metrics/schema/internal_events.json
@@ -9,21 +9,6 @@
"then": {
"properties": {
"instrumentation_class": false,
- "options": {
- "type": "object",
- "properties": {
- "events": {
- "type": "array",
- "items": {
- "type": "string"
- }
- }
- },
- "required": [
- "events"
- ],
- "additionalProperties": false
- },
"events": {
"type": "array",
"items": {
@@ -49,8 +34,7 @@
}
},
"required": [
- "events",
- "options"
+ "events"
]
}
}
diff --git a/doc/development/internal_analytics/internal_event_instrumentation/migration.md b/doc/development/internal_analytics/internal_event_instrumentation/migration.md
index b37932764b43d..db57eb4886b3a 100644
--- a/doc/development/internal_analytics/internal_event_instrumentation/migration.md
+++ b/doc/development/internal_analytics/internal_event_instrumentation/migration.md
@@ -137,7 +137,8 @@ To start using Internal Events Tracking, follow these steps:
```
Use `project.id` or `namespace.id` instead of `user.id` if your metric is counting something other than unique users.
-1. Call `InternalEvents.tract_event` instead of `HLLRedisCounter.track_event`:
+1. Remove the `options` section from both metric definition files.
+1. Call `InternalEvents.track_event` instead of `HLLRedisCounter.track_event`:
```diff
- Gitlab::UsageDataCounters::HLLRedisCounter.track_event(:git_write_action, values: current_user.id)
diff --git a/scripts/internal_events/cli/helpers/metric_options.rb b/scripts/internal_events/cli/helpers/metric_options.rb
index 01512115e05d7..c2f2c4f81cddd 100755
--- a/scripts/internal_events/cli/helpers/metric_options.rb
+++ b/scripts/internal_events/cli/helpers/metric_options.rb
@@ -71,9 +71,11 @@ def get_existing_metrics_for_events(events)
fields = InternalEventsCli::NEW_METRIC_FIELDS.map(&:to_s)
metric = Metric.new(**details.slice(*fields))
- next unless metric.actions
- metric if (metric.actions & actions).any?
+ metric_actions = metric.events&.map { |event| event['name'] }
+ next unless metric_actions
+
+ metric if (metric_actions & actions).any?
end
end
diff --git a/scripts/internal_events/cli/metric.rb b/scripts/internal_events/cli/metric.rb
index 0d39b0b4e2b57..5bb46d3d1363f 100755
--- a/scripts/internal_events/cli/metric.rb
+++ b/scripts/internal_events/cli/metric.rb
@@ -18,7 +18,6 @@ module InternalEventsCli
:product_category,
:distribution,
:tier,
- :options,
:events
].freeze
@@ -43,7 +42,7 @@ module InternalEventsCli
performance_indicator_type: []
}.freeze
- Metric = Struct.new(*NEW_METRIC_FIELDS, *ADDITIONAL_METRIC_FIELDS, :identifier, keyword_init: true) do
+ Metric = Struct.new(*NEW_METRIC_FIELDS, *ADDITIONAL_METRIC_FIELDS, :identifier, :actions, keyword_init: true) do
def formatted_output
METRIC_DEFAULTS
.merge(to_h.compact)
@@ -102,7 +101,7 @@ def key_path_prefix
end
def actions
- options&.dig('events')&.sort || []
+ self[:actions] || []
end
def identifier_prefix
diff --git a/scripts/internal_events/cli/metric_definer.rb b/scripts/internal_events/cli/metric_definer.rb
index 7688f03200fc3..7d52517485dfe 100755
--- a/scripts/internal_events/cli/metric_definer.rb
+++ b/scripts/internal_events/cli/metric_definer.rb
@@ -114,9 +114,9 @@ def prompt_for_metrics
@metrics = cli.select('Which metrics do you want to add?', eligible_metrics, **select_opts)
- assign_shared_attrs(:options, :milestone) do
+ assign_shared_attrs(:actions, :milestone) do
{
- options: { 'events' => selected_events.map(&:action) },
+ actions: selected_events.map(&:action).sort,
milestone: MILESTONE
}
end
diff --git a/spec/fixtures/scripts/internal_events/metrics/ee_total_28d_single_event.yml b/spec/fixtures/scripts/internal_events/metrics/ee_total_28d_single_event.yml
index 5238e997044fd..647fe9e04b2ae 100644
--- a/spec/fixtures/scripts/internal_events/metrics/ee_total_28d_single_event.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/ee_total_28d_single_event.yml
@@ -17,8 +17,5 @@ distribution:
tier:
- premium
- ultimate
-options:
- events:
- - internal_events_cli_used
events:
- name: internal_events_cli_used
diff --git a/spec/fixtures/scripts/internal_events/metrics/ee_total_7d_single_event.yml b/spec/fixtures/scripts/internal_events/metrics/ee_total_7d_single_event.yml
index fdbf137f6994d..700b461061824 100644
--- a/spec/fixtures/scripts/internal_events/metrics/ee_total_7d_single_event.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/ee_total_7d_single_event.yml
@@ -17,8 +17,5 @@ distribution:
tier:
- premium
- ultimate
-options:
- events:
- - internal_events_cli_used
events:
- name: internal_events_cli_used
diff --git a/spec/fixtures/scripts/internal_events/metrics/ee_total_single_event.yml b/spec/fixtures/scripts/internal_events/metrics/ee_total_single_event.yml
index e928869ca9aa2..b9c918e9a9ddd 100644
--- a/spec/fixtures/scripts/internal_events/metrics/ee_total_single_event.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/ee_total_single_event.yml
@@ -17,8 +17,5 @@ distribution:
tier:
- premium
- ultimate
-options:
- events:
- - internal_events_cli_used
events:
- name: internal_events_cli_used
diff --git a/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_28d.yml b/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_28d.yml
index 4d40e2122cbfd..a0aa259e20731 100644
--- a/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_28d.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_28d.yml
@@ -19,9 +19,6 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - random_name
events:
- name: random_name
unique: user.id
diff --git a/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_7d.yml b/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_7d.yml
index 166cef9041235..d3e3f91e51a70 100644
--- a/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_7d.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/keyboard_smashed_metric_7d.yml
@@ -19,9 +19,6 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - random_name
events:
- name: random_name
unique: user.id
diff --git a/spec/fixtures/scripts/internal_events/metrics/project_id_28d_multiple_events.yml b/spec/fixtures/scripts/internal_events/metrics/project_id_28d_multiple_events.yml
index 122043e6cc026..09f23b07b17bc 100644
--- a/spec/fixtures/scripts/internal_events/metrics/project_id_28d_multiple_events.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/project_id_28d_multiple_events.yml
@@ -19,10 +19,6 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - internal_events_cli_closed
- - internal_events_cli_used
events:
- name: internal_events_cli_closed
unique: project.id
diff --git a/spec/fixtures/scripts/internal_events/metrics/project_id_7d_multiple_events.yml b/spec/fixtures/scripts/internal_events/metrics/project_id_7d_multiple_events.yml
index 11a4ba41c07d9..5b6ebcfb11a1a 100644
--- a/spec/fixtures/scripts/internal_events/metrics/project_id_7d_multiple_events.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/project_id_7d_multiple_events.yml
@@ -19,10 +19,6 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - internal_events_cli_closed
- - internal_events_cli_used
events:
- name: internal_events_cli_closed
unique: project.id
diff --git a/spec/fixtures/scripts/internal_events/metrics/total_single_event.yml b/spec/fixtures/scripts/internal_events/metrics/total_single_event.yml
index 038fc738f256a..a4f8ffb54c16e 100644
--- a/spec/fixtures/scripts/internal_events/metrics/total_single_event.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/total_single_event.yml
@@ -19,8 +19,5 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - internal_events_cli_used
events:
- name: internal_events_cli_used
diff --git a/spec/fixtures/scripts/internal_events/metrics/user_id_28d_single_event.yml b/spec/fixtures/scripts/internal_events/metrics/user_id_28d_single_event.yml
index b27e69bd43b4a..1b96ecd40b1a7 100644
--- a/spec/fixtures/scripts/internal_events/metrics/user_id_28d_single_event.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/user_id_28d_single_event.yml
@@ -19,9 +19,6 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - internal_events_cli_used
events:
- name: internal_events_cli_used
unique: user.id
diff --git a/spec/fixtures/scripts/internal_events/metrics/user_id_7d_single_event.yml b/spec/fixtures/scripts/internal_events/metrics/user_id_7d_single_event.yml
index e08733a6bc9c4..5e34991a87d55 100644
--- a/spec/fixtures/scripts/internal_events/metrics/user_id_7d_single_event.yml
+++ b/spec/fixtures/scripts/internal_events/metrics/user_id_7d_single_event.yml
@@ -19,9 +19,6 @@ tier:
- free
- premium
- ultimate
-options:
- events:
- - internal_events_cli_used
events:
- name: internal_events_cli_used
unique: user.id
diff --git a/spec/scripts/internal_events/cli_spec.rb b/spec/scripts/internal_events/cli_spec.rb
index 3bbda97d4b3d0..1c7a5e924a2fb 100644
--- a/spec/scripts/internal_events/cli_spec.rb
+++ b/spec/scripts/internal_events/cli_spec.rb
@@ -1,9 +1,8 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+require 'spec_helper'
require 'tty/prompt/test'
require_relative '../../../scripts/internal_events/cli'
-require_relative '../../support/helpers/wait_helpers'
RSpec.describe Cli, feature_category: :service_ping do
include WaitHelpers
--
GitLab