Add CLI support for time_frame arrays in metrics

What does this MR do and why?

Related to #478008 (closed)

After !167201 (merged) has been deployed & tested on production, we now know that metrics with time_frame array are working.

Now, we add official support for them, by making the CLI use them as the default & adding them to our documentation.

This also updates all of the tests that were previously creating a separate metric definition for each time_frame.

References

Please include cross links to any resources that are relevant to this MR This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

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 test this, run the internal events CLI:

ruby scripts/internal_events/cli.rb

And try to create new metric definition with time_frame array. Then, verify that the new metrics are working, using the Manual testing in GDK commands listed by the CLI when using the view usage command of the CLI.

assigned to @michold

added devopsmonitor groupanalytics instrumentation sectionanalytics typefeature labels

added pipelinetier-1 label

added analytics instrumentation analytics instrumentationreview pending backend labels

	2 Warnings
	This merge request is quite big (704 lines changed), please consider splitting it into multiple merge requests.
	28ae9f78: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.

	1 Message
	This MR contains docs in the /doc/development directory, but any Maintainer (other than the author) can merge. You do not need tech writer review.

Reviewer roulette

Category	Reviewer	Maintainer
analytics instrumentation	@ankit.panchal (UTC+5.5, 4.5 hours ahead of author)	Maintainer review is optional for analytics instrumentation
backend	@rcobb (UTC-8, 9 hours behind author)	@nmilojevic1 (UTC+1, same timezone as author)
~"Tooling"	Reviewer review is optional for ~"Tooling"	@sgarg_gitlab (UTC+5.5, 4.5 hours ahead of author)

Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

added Data WarehouseImpact Check label

added 1 commit

• 049f2bb4 - Fix cli global state

Compare with previous version

Generated by gitlab_quality-test_tooling.

---

Slow tests detected in this merge request. These slow tests might be related to this merge request's changes.

Click to expand

Job	File	Name	Duration	Expected duration
#8308190806	spec/scripts/internal_events/product_group_renamer_spec.rb#L13	ProductGroupRenamer with real definitions reads all definitions files	56.33 s	< 45.4 s
#8308819202	spec/scripts/internal_events/product_group_renamer_spec.rb#L13	ProductGroupRenamer with real definitions reads all definitions files	54.55 s	< 45.4 s

added rspec:slow test detected label

added 1 commit

• b49f2dbf - Update time_frame documentation

Compare with previous version

added development guidelines docsimprovement documentation maintenancerefactor typemaintenance labels and removed typefeature label

added 511 commits

• b49f2dbf...73937faa - 509 commits from branch master
• d626e7ae - Add CLI handling of time_frames
• c4fff768 - Update time_frame documentation

Compare with previous version

mentioned in issue #478008 (closed)

added 1 commit

• 9b4c45dd - Reduce #reduce_metrics_by_time_frame

Compare with previous version

added 1 commit

• 819c2795 - Reduce no longer necessary code

Compare with previous version

changed the description

changed milestone to %17.6

added missed:17.4 missed:17.5 typefeature workflowin dev labels and removed typemaintenance label

removed maintenancerefactor label

removed missed:17.4 label

removed missed:17.5 label

changed the description

added 1 commit

• e34b2eee - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

added maintenancerefactor typemaintenance labels and removed typefeature label

@j_lar Hello! Can you please give this analytics instrumentation & backend review?

The amount of changed lines may look scary, but most of them are coming from test fixture changes.

requested review from @j_lar

removed review request for @j_lar

changed milestone to %17.7

added missed:17.6 label

added 1 commit

• cbce8701 - Make code more readable

Compare with previous version

requested review from @j_lar

added 1957 commits

• cbce8701...592128b2 - 1953 commits from branch master
• faa0f267 - Add CLI handling of time_frames
• f2a92714 - Update time_frame documentation
• acb24365 - Reduce no longer necessary code
• f7104f3e - Make code more readable

Compare with previous version

approved this merge request

requested review from @syasonik

added pipeline:mr-approved label

added pipelinetier-3 pipeline:run-e2e-omnibus-once labels and removed pipelinetier-1 label

Before you set this MR to auto-merge

This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.

Before you set this MR to auto-merge, please check the following:

• You are the last maintainer of this merge request
• The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
• This pipeline is recent enough (created in the last 8 hours)

If all the criteria above apply, please set auto-merge for this merge request.

See pipeline tiers and merging a merge request for more details.

removed review request for @j_lar

E2E Test Result Summary

allure-report-publisher generated test report!

e2e-test-on-gdk: test report for f4474b60

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Verify      | 43     | 0      | 2       | 0     | 45    | ✅     |
| Data Stores | 33     | 0      | 1       | 0     | 34    | ✅     |
| Plan        | 76     | 0      | 0       | 0     | 76    | ✅     |
| Govern      | 75     | 0      | 3       | 0     | 78    | ✅     |
| Create      | 129    | 0      | 22      | 0     | 151   | ✅     |
| Monitor     | 8      | 0      | 0       | 0     | 8     | ✅     |
| Package     | 25     | 0      | 11      | 0     | 36    | ✅     |
| Fulfillment | 2      | 0      | 0       | 0     | 2     | ✅     |
| Ai-powered  | 0      | 0      | 1       | 0     | 1     | ➖     |
| Analytics   | 2      | 0      | 0       | 0     | 2     | ✅     |
| Release     | 5      | 0      | 0       | 0     | 5     | ✅     |
| Secure      | 4      | 0      | 0       | 0     | 4     | ✅     |
| Manage      | 1      | 0      | 1       | 0     | 2     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 403    | 0      | 41      | 0     | 444   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

e2e-test-on-cng: test report for f4474b60

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Govern      | 84     | 0      | 9       | 1     | 93    | ✅     |
| Monitor     | 8      | 0      | 12      | 0     | 20    | ✅     |
| Data Stores | 33     | 0      | 10      | 0     | 43    | ✅     |
| Verify      | 49     | 0      | 16      | 0     | 65    | ✅     |
| Plan        | 86     | 0      | 8       | 0     | 94    | ✅     |
| Create      | 140    | 0      | 20      | 0     | 160   | ✅     |
| Analytics   | 2      | 0      | 0       | 1     | 2     | ✅     |
| Secure      | 2      | 0      | 5       | 0     | 7     | ✅     |
| Fulfillment | 2      | 0      | 7       | 1     | 9     | ✅     |
| Release     | 5      | 0      | 1       | 0     | 6     | ✅     |
| Configure   | 0      | 0      | 3       | 0     | 3     | ➖     |
| Manage      | 1      | 0      | 9       | 0     | 10    | ✅     |
| Package     | 24     | 0      | 14      | 0     | 38    | ✅     |
| Ai-powered  | 0      | 0      | 2       | 0     | 2     | ➖     |
| ModelOps    | 0      | 0      | 1       | 0     | 1     | ➖     |
| Growth      | 0      | 0      | 2       | 0     | 2     | ➖     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 436    | 0      | 119     | 3     | 555   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

added Data WarehouseNot Impacted label and removed Data WarehouseImpact Check label

added 1 commit

• dc3418e6 - Update MetricOptions spec

Compare with previous version

reset approvals from @j_lar by pushing to the branch

@michold @j_lar Looks mostly ok to me! There's just one functional thing that should be fixed, and then I think this can merge.

There are a few other things that I think could be nice improvements, but they probably don't need to be a part of this issue.

UX improvements:

1. Show a message when prompted to save the file that the definition will create multiple metrics
   • along with this, we should display the key_path of each metric
   • this should ideally happen before the file is saved/created so users have accurate expectations ahead
2. Default the metric options to include all of Total/Monthly/Weekly (except unique-by-identifier metrics)
   • if I want to creates all three metrics, I'll still end up with 2 files, because it's two separate flows in the CLI to add total, then monthly/weekly. And for the 2nd file, I'll have to define a new filename because the default name is already taken by the first definition

Maintenance improvements:

1. Swap metric_definer.rb to operate on a single @metric instead of over @metrics
   • simplifies complexity of grouping logic in MetricOptions (to the point of !171972 (comment 2218951874), as I was confused the same way)
   • treating Metric as essentially a MetricFile better matches how we expect users to interact
   • this can remove the complexity from multiple description inputs, rename inputs, defaults, etc
2. When creating metrics that could be defined in the same file as existing metrics, dedupe the files and relocate to the CLI-suggested filepath

While reviewing & thinking this through, I tested out some of the above suggestions in sy-review-michold-time-frame-array-cli just to get an idea.

added 1 commit

• 01b3bd41 - Link both metric dashboards

Compare with previous version

mentioned in issue #505803

requested review from @syasonik

added 1 commit

• 28ae9f78 - Link both metric dashboards

Compare with previous version

added 1865 commits

• 28ae9f78...8ee08279 - 1864 commits from branch master
• f4474b60 - Merge branch 'master' into michold-time-frame-array-cli

Compare with previous version

approved this merge request

requested review from @j_lar

approved this merge request

added analytics instrumentationapproved label and removed analytics instrumentationreview pending label

resolved all threads

enabled automatic add to merge train when checks pass

started a merge train

merged

mentioned in commit 90931360

Hello @michold

The Analytics Instrumentation team is actively working on improving the metrics implementation and event tracking processes. We would love to hear about your experience and any feedback you might have!

To provide your feedback, please use this Google Form.

Thanks for your help!

---

• Looking for existing metrics data? Check out the Metrics Exploration Dashboard for Group:analytics_instrumentation.
• Need further assistance? Have comments?
  • Mention @gitlab-org/analytics-section/analytics-instrumentation/engineers
  • Reach out in #g_monitor_analytics_instrumentation channel on Slack.

This message was generated automatically. Improve it or delete it.

added workflowstaging-canary label and removed workflowin dev label

added workflowcanary label and removed workflowstaging-canary label

added workflowstaging label and removed workflowcanary label

added workflowproduction label and removed workflowstaging label

added workflowpost-deploy-db-staging label and removed workflowproduction label

added releasedcandidate label

resolved all threads

added releasedpublished label and removed releasedcandidate label