From c7032e6103ad0226e45a87e4fdd8e4895e073fc5 Mon Sep 17 00:00:00 2001
From: Nwanna Isong <nisong@gitlab.com>
Date: Mon, 30 Sep 2024 12:40:24 +0000
Subject: [PATCH] Update CLI to allow more than three(3) additional properties
when creating events
---
scripts/internal_events/cli/event_definer.rb | 43 ++++++++++++++++---
scripts/internal_events/cli/text.rb | 12 +++++-
scripts/internal_events/cli/usage_viewer.rb | 3 +-
.../event_with_additional_properties.yml | 2 +
...re_enabled_by_namespace_ids_identifier.yml | 2 +
.../scripts/internal_events/new_events.yml | 31 +++----------
.../scripts/internal_events/new_metrics.yml | 1 +
spec/scripts/internal_events/cli_spec.rb | 16 ++++---
8 files changed, 72 insertions(+), 38 deletions(-)
diff --git a/scripts/internal_events/cli/event_definer.rb b/scripts/internal_events/cli/event_definer.rb
index fb5975b08e6b2..0e5da5b35eb22 100755
--- a/scripts/internal_events/cli/event_definer.rb
+++ b/scripts/internal_events/cli/event_definer.rb
@@ -114,7 +114,7 @@ def format_identifier_choice(choice)
def prompt_for_additional_properties
cli.say Text::ADDITIONAL_PROPERTIES_INTRO
- available_props = [:label, :property, :value]
+ available_props = [:label, :property, :value, :add_extra_prop]
while available_props.any?
disabled = format_help('(already defined)')
@@ -136,7 +136,16 @@ def prompt_for_additional_properties
value: :value,
name: 'Number (attribute will be named `value`)',
disabled: disabled
- ) { !available_props.include?(:value) }
+ ) { !available_props.include?(:value) },
+ disableable_option(
+ value: :add_extra_prop,
+ name: 'Add extra property (attribute will be named the input custom name)',
+ disabled: format_warning('(option disabled - use label/property/value first)')
+ ) do
+ !((!available_props.include?(:label) &&
+ !available_props.include?(:property)) ||
+ !available_props.include?(:value))
+ end
]
# rubocop:enable Rails/NegateInclude
@@ -150,18 +159,38 @@ def prompt_for_additional_properties
if selected_property == :none
available_props.clear
+ elsif selected_property == :add_extra_prop
+ property_name = prompt_for_add_extra_properties
+ property_description = prompt_for_text('Describe what the field will include:')
+ assign_extra_properties(property_name, property_description)
else
available_props.delete(selected_property)
property_description = prompt_for_text('Describe what the field will include:')
-
- event.additional_properties ||= {}
- event.additional_properties[selected_property.to_s] = {
- 'description' => property_description || 'TODO'
- }
+ assign_extra_properties(selected_property, property_description)
end
end
end
+ def assign_extra_properties(property, description = nil)
+ event.additional_properties ||= {}
+ event.additional_properties[property.to_s] = {
+ 'description' => description || 'TODO'
+ }
+ end
+
+ def prompt_for_add_extra_properties
+ primary_props = %w[label property value]
+
+ prompt_for_text('Define a name for the attribute:', **input_opts) do |q|
+ q.required true
+ q.validate ->(input) { input =~ NAME_REGEX && primary_props.none?(input) }
+ q.modify :trim
+ q.messages[:required?] = Text::ADDITIONAL_PROPERTIES_ADD_MORE_HELP
+ q.messages[:valid?] = format_warning("Invalid property name. Only lowercase/numbers/underscores allowed. " \
+ "Ensure %{value} is not one of `property, label, value`.")
+ end
+ end
+
def prompt_for_url
new_page!(4, 7, STEPS)
diff --git a/scripts/internal_events/cli/text.rb b/scripts/internal_events/cli/text.rb
index 5190f02ca3ed7..9c5ff853cd63e 100755
--- a/scripts/internal_events/cli/text.rb
+++ b/scripts/internal_events/cli/text.rb
@@ -178,7 +178,7 @@ module Text
- Service Ping: filter metrics to a specific subset of events
- Snowflake: view/sort/group individual events from GitLab.com
- There are a few specific attributes are available for recording the context of each event. These include 2 strings and 1 numeric value.
+ A few specific attributes are available for recording the context of each event. These include 2 strings and 1 numeric value.
ex) For an event like 'change_merge_request_status', we might want to include:
@@ -189,6 +189,16 @@ module Text
TEXT
+ ADDITIONAL_PROPERTIES_ADD_MORE_HELP = <<~TEXT.freeze
+ #{format_warning('Required. Must be unique within the event context. Must use only letters/numbers/underscores.')}
+
+ #{format_info('It should not be named any of the following:')}
+ - property#{' '}
+ - label
+ - value
+
+ TEXT
+
DATABASE_METRIC_NOTICE = <<~TEXT
For right now, this script can only define metrics for internal events.
diff --git a/scripts/internal_events/cli/usage_viewer.rb b/scripts/internal_events/cli/usage_viewer.rb
index 4c398cbc6e852..d91fa6f3c4ebe 100755
--- a/scripts/internal_events/cli/usage_viewer.rb
+++ b/scripts/internal_events/cli/usage_viewer.rb
@@ -9,7 +9,8 @@ class UsageViewer
PROPERTY_EXAMPLES = {
'label' => "'string'",
'property' => "'string'",
- 'value' => '72'
+ 'value' => '72',
+ 'custom_key' => 'custom_value'
}.freeze
attr_reader :cli, :event
diff --git a/spec/fixtures/scripts/internal_events/events/event_with_additional_properties.yml b/spec/fixtures/scripts/internal_events/events/event_with_additional_properties.yml
index 462dd280a17fc..1ce3a544cf209 100644
--- a/spec/fixtures/scripts/internal_events/events/event_with_additional_properties.yml
+++ b/spec/fixtures/scripts/internal_events/events/event_with_additional_properties.yml
@@ -11,6 +11,8 @@ additional_properties:
description: TODO
value:
description: Time the CLI ran before closing (seconds)
+ custom_key:
+ description: The extra custom property name
product_group: analytics_instrumentation
milestone: '16.6'
introduced_by_url: TODO
diff --git a/spec/fixtures/scripts/internal_events/events/event_with_feature_enabled_by_namespace_ids_identifier.yml b/spec/fixtures/scripts/internal_events/events/event_with_feature_enabled_by_namespace_ids_identifier.yml
index 3b116bea3301e..7bc7c035f15df 100644
--- a/spec/fixtures/scripts/internal_events/events/event_with_feature_enabled_by_namespace_ids_identifier.yml
+++ b/spec/fixtures/scripts/internal_events/events/event_with_feature_enabled_by_namespace_ids_identifier.yml
@@ -12,6 +12,8 @@ additional_properties:
description: TODO
value:
description: Time the CLI ran before closing (seconds)
+ custom_key:
+ description: The extra custom property name
product_group: analytics_instrumentation
milestone: '16.6'
introduced_by_url: TODO
diff --git a/spec/fixtures/scripts/internal_events/new_events.yml b/spec/fixtures/scripts/internal_events/new_events.yml
index b9499cf20c0cb..3e68297fec92f 100644
--- a/spec/fixtures/scripts/internal_events/new_events.yml
+++ b/spec/fixtures/scripts/internal_events/new_events.yml
@@ -222,6 +222,9 @@
- "\n" # Skip label description
- "\e[B\e[B\n" # Select: Number (aka value)
- "Time the CLI ran before closing (seconds)\n" # value description
+ - "\e[B\e[B\n" # Select: Add extra property (aka value)
+ - "custom_key\n" # Submit property name
+ - "The extra custom property name\n" # Submit property description
- "\n" # Select (add props): None! Continue to next step!
- "\n" # Skip MR URL
- "instrumentation" # Filters to the analytics instrumentation group
@@ -265,30 +268,6 @@
- path: ee/config/events/internal_events_cli_opened.yml
content: spec/fixtures/scripts/internal_events/events/ee_event_without_identifiers.yml
-- description: Auto-proceeds to MR selection when event uses all additional properties
- inputs:
- keystrokes:
- - "1\n" # Enum-select: New Event -- start tracking when an action or scenario occurs on gitlab instances
- - "Engineer uses Internal Event CLI to define a new event\n" # Submit description
- - "internal_events_cli_used\n" # Submit action name
- - "1\n" # Select: [namespace, project, user]
- - "\e[B\n" # Arrow down to & Select: String 1 (aka label)
- - "\n" # Skip label description
- - "\e[B\n" # Arrow down to: String 2 (aka property)
- - "\n" # Skip property description
- - "\e[B\n" # Arrow down to: Number (aka value)
- - "Time the CLI ran before closing (seconds)\n" # value description
- - "\n" # Skip MR URL
- - "instrumentation" # Filters to the analytics instrumentation group
- - "\n" # Accept analytics:monitor:analytics_instrumentation
- - "1\n" # Select: [free, premium, ultimate]
- - "y\n" # Create file
- - "4\n" # Exit
- outputs:
- files:
- - path: config/events/internal_events_cli_used.yml
- content: spec/fixtures/scripts/internal_events/events/event_with_all_additional_properties.yml
-
- description: Event with feature_enabled_by_namespace_ids identifier
inputs:
keystrokes:
@@ -302,6 +281,10 @@
- "\n" # Skip property description
- "\e[B\n" # Arrow down to: Number (aka value)
- "Time the CLI ran before closing (seconds)\n" # value description
+ - "\e[B\n" # Select: Add extra property (aka custom_key)
+ - "custom_key\n" # Submit property name
+ - "The extra custom property name\n" # Submit property description
+ - "\n" # Select (add props): None! Continue to next step!
- "\n" # Skip MR URL
- "instrumentation" # Filters to the analytics instrumentation group
- "\n" # Accept analytics:monitor:analytics_instrumentation
diff --git a/spec/fixtures/scripts/internal_events/new_metrics.yml b/spec/fixtures/scripts/internal_events/new_metrics.yml
index 58916bce532a6..f9c1bbaacdf0f 100644
--- a/spec/fixtures/scripts/internal_events/new_metrics.yml
+++ b/spec/fixtures/scripts/internal_events/new_metrics.yml
@@ -319,6 +319,7 @@
- "\n" # Select: Weekly count of unique users where label/value is...
- "failure, incomplete\n" # Input multiple values for "label" filter
- "\n" # Skip "value" filter
+ - "\n" # Skip "Add extra property" filter
- "who tried and failed to define an internal event using the CLI\n" # Input description
- "failed_usage_attempts\n" # Input metric key path
- "\n" # Submit monthly description for weekly
diff --git a/spec/scripts/internal_events/cli_spec.rb b/spec/scripts/internal_events/cli_spec.rb
index 6ae5e780f3889..9f41437d4cd14 100644
--- a/spec/scripts/internal_events/cli_spec.rb
+++ b/spec/scripts/internal_events/cli_spec.rb
@@ -998,7 +998,8 @@ def select_event_from_list
user: user,
additional_properties: {
label: 'string', # TODO
- value: 72 # Time the CLI ran before closing (seconds)
+ value: 72, # Time the CLI ran before closing (seconds)
+ custom_key: custom_value # The extra custom property name
}
)
@@ -1018,7 +1019,8 @@ def select_event_from_list
let(:additional_properties) do
{
label: 'string',
- value: 72
+ value: 72,
+ custom_key: custom_value
}
end
end
@@ -1048,6 +1050,7 @@ def select_event_from_list
{
label: 'string', // TODO
value: 72, // Time the CLI ran before closing (seconds)
+ custom_key: custom_value, // The extra custom property name
},
);
},
@@ -1076,6 +1079,7 @@ def select_event_from_list
{
label: 'string', // TODO
value: 72, // Time the CLI ran before closing (seconds)
+ custom_key: custom_value, // The extra custom property name
},
);
@@ -1104,6 +1108,7 @@ def select_event_from_list
data-event-tracking="internal_events_cli_used"
data-event-label="string"
data-event-value=72
+ data-event-custom_key=custom_value
>
Click Me
</gl-button>
@@ -1125,6 +1130,7 @@ def select_event_from_list
data-event-tracking-load="internal_events_cli_used"
data-event-label="string"
data-event-value=72
+ data-event-custom_key=custom_value
>
Click Me
</gl-button>
@@ -1139,18 +1145,18 @@ def select_event_from_list
--------------------------------------------------
# HAML -- ON-CLICK
- .inline-block{ data: { event_tracking: 'internal_events_cli_used', event_label: 'string', event_value: 72 } }
+ .inline-block{ data: { event_tracking: 'internal_events_cli_used', event_label: 'string', event_value: 72, event_custom_key: custom_value } }
= _('Important Text')
--------------------------------------------------
# HAML -- COMPONENT ON-CLICK
- = render Pajamas::ButtonComponent.new(button_options: { data: { event_tracking: 'internal_events_cli_used', event_label: 'string', event_value: 72 } })
+ = render Pajamas::ButtonComponent.new(button_options: { data: { event_tracking: 'internal_events_cli_used', event_label: 'string', event_value: 72, event_custom_key: custom_value } })
--------------------------------------------------
# HAML -- COMPONENT ON-LOAD
- = render Pajamas::ButtonComponent.new(button_options: { data: { event_tracking_load: true, event_tracking: 'internal_events_cli_used', event_label: 'string', event_value: 72 } })
+ = render Pajamas::ButtonComponent.new(button_options: { data: { event_tracking_load: true, event_tracking: 'internal_events_cli_used', event_label: 'string', event_value: 72, event_custom_key: custom_value } })
--------------------------------------------------
TEXT
--
GitLab