summaryrefslogtreecommitdiff
path: root/doc/development/service_ping/review_guidelines.md
diff options
context:
space:
mode:
Diffstat (limited to 'doc/development/service_ping/review_guidelines.md')
-rw-r--r--doc/development/service_ping/review_guidelines.md18
1 files changed, 12 insertions, 6 deletions
diff --git a/doc/development/service_ping/review_guidelines.md b/doc/development/service_ping/review_guidelines.md
index a4fb929a8a0..f961fd376bd 100644
--- a/doc/development/service_ping/review_guidelines.md
+++ b/doc/development/service_ping/review_guidelines.md
@@ -14,7 +14,7 @@ general best practices for code reviews, refer to our [code review guide](../cod
## Resources for reviewers
- [Service Ping Guide](index.md)
-- [Metrics Dictionary](metrics_dictionary.md)
+- [Metrics Dictionary](https://gitlab-org.gitlab.io/growth/product-intelligence/metric-dictionary)
## Review process
@@ -25,7 +25,6 @@ any of the following Service Ping files:
- The Metrics Dictionary, including files in:
- [`config/metrics`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/config/metrics).
- [`ee/config/metrics`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/config/metrics).
- - [`dictionary.md`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/development/usage_ping/dictionary.md).
- [`schema.json`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/metrics/schema.json).
- Product Intelligence tooling. For example,
[`Gitlab::UsageMetricDefinitionGenerator`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/generators/gitlab/usage_metric_definition_generator.rb)
@@ -34,23 +33,28 @@ any of the following Service Ping files:
#### The merge request **author** should
-- Decide whether a Product Intelligence review is needed.
+- Decide whether a Product Intelligence review is needed. You can skip the Product Intelligence
+review and remove the labels if the changes are not related to the Product Intelligence domain and
+are regular backend changes.
- If a Product Intelligence review is needed, add the labels
`~product intelligence` and `~product intelligence::review pending`.
+- For merge requests authored by Product Intelligence team members:
+ - Assign both the `~backend` and `~product intelligence` reviews to another Product Intelligence team member.
+ - Assign the maintainer review to someone outside of the Product Intelligence group.
- Assign an
[engineer](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) from the Product Intelligence team for a review.
- Set the correct attributes in the metric's YAML definition:
- `product_section`, `product_stage`, `product_group`, `product_category`
- Provide a clear description of the metric.
-- Update the
- [Metrics Dictionary](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/development/usage_ping/dictionary.md) if needed.
- Add a changelog [according to guidelines](../changelog.md).
#### The Product Intelligence **reviewer** should
- Perform a first-pass review on the merge request and suggest improvements to the author.
-- Check the [metrics location](index.md#1-naming-and-placing-the-metrics) in
+- Check the [metrics location](implement.md#name-and-place-the-metric) in
the Service Ping JSON payload.
+- Suggest that the author checks the [naming suggestion](implement.md#how-to-get-a-metric-name-suggestion) while
+ generating the metric's YAML definition.
- Add the `~database` label and ask for a [database review](../database_review.md) for
metrics that are based on Database.
- For tracking using Redis HLL (HyperLogLog):
@@ -63,6 +67,8 @@ any of the following Service Ping files:
Read the [stages file](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml).
- Check the file location. Consider the time frame, and if the file should be under `ee`.
- Check the tiers.
+- Metrics instrumentations
+ - Recommend using metrics instrumentation for new metrics, [if possible](metrics_instrumentation.md#support-for-instrumentation-classes).
- Approve the MR, and relabel the MR with `~"product intelligence::approved"`.
## Review workload distribution