From c0bc0787cfd9c858c32d95fd976e7fe9efb2de27 Mon Sep 17 00:00:00 2001
From: Sebastian Rehm <srehm@gitlab.com>
Date: Wed, 14 Feb 2024 00:22:43 +0000
Subject: [PATCH] Remove local testing from analytics instrumentation review
---
doc/development/internal_analytics/review_guidelines.md | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/doc/development/internal_analytics/review_guidelines.md b/doc/development/internal_analytics/review_guidelines.md
index 97014997d1bb5..50c28464f9fd8 100644
--- a/doc/development/internal_analytics/review_guidelines.md
+++ b/doc/development/internal_analytics/review_guidelines.md
@@ -32,6 +32,10 @@ In most cases, an Analytics Instrumentation review is automatically added, but i
review and remove the labels if the changes are not related to the Analytics Instrumentation domain.
- If an Analytics Instrumentation review is needed and was not assigned automatically, add the labels
`~analytics instrumentation` and `~analytics instrumentation::review pending`.
+- If a change to an event is a part of the MR:
+ - Check that the events are firing locally using one of the [testing tools](internal_event_instrumentation/local_setup_and_debugging.md) available.
+- If a change to a metric is a part of the MR:
+ - Make sure that the new metric is available in Service Ping payload, by running: `Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values).dig(*'key_path'.split('.'))` with `key_path` substituted by the new metric's `key_path`.
- Use reviewer roulette to assign an [Analytics Instrumentation reviewer](https://gitlab-org.gitlab.io/gitlab-roulette/?hourFormat24=true&visible=reviewer%7Canalytics+instrumentation) who is not the author.
- Assign any other reviews as appropriate.
- `~analytics instrumentation` review does not require a maintainer review.
@@ -41,8 +45,8 @@ In most cases, an Analytics Instrumentation review is automatically added, but i
- Perform a first-pass review on the merge request and suggest improvements to the author.
- Make sure that no deprecated analytics methods are used.
- If a change to an event is a part of the review:
+ - Check that the event(s) being fired have corresponding definition files.
- Check that the [event definition file](internal_event_instrumentation/event_definition_guide.md) is correct.
- - Check that the events are firing locally using one of the [testing tools](internal_event_instrumentation/local_setup_and_debugging.md) available.
- If a change to a metric is a part of the review:
- Add the `~database` label and ask for a [database review](../database_review.md) for
metrics that are based on Database.
@@ -54,5 +58,4 @@ In most cases, an Analytics Instrumentation review is automatically added, but i
- Check the file location. Consider the time frame, and if the file should be under `ee`.
- Check the tiers.
- If a metric was changed or removed: Make sure the MR author notified the Customer Success Ops team (`@csops-team`), Analytics Engineers (`@gitlab-data/analytics-engineers`), and Product Analysts (`@gitlab-data/product-analysts`) by `@` mentioning those groups in a comment on the issue for the MR and all of these groups have acknowledged the removal.
- - Make sure that the new metric is available in Service Ping payload, by running: `Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values).dig(*'key_path'.split('.'))` with `key_path` substituted by the new metric's `key_path`.
- Approve the MR, and relabel the MR with `~"analytics instrumentation::approved"`.
--
GitLab