Spike: Use time_frame as a dimension of metric definition
Problem
A typical MR, such as !162017 (merged), that includes a metric, normally includes that metric with at least two time-frames 7d and 28d.
We also suggest using both time-frames.
At the same time, everything apart from two lines is the same across the definitions of the two metrics for the two time-frames, key_path
and time_frame
itself.
This represents a significant duplication of code which makes these MRs harder to review and the code harder to maintain. There's even a risk that the two metrics, although they should represent the same count only in different time-frames, start deviating when adjustments are made.
Desired Outcome
Only one metric definition necessary for defining a metric across multiple time_frames
Potential Solution
- Create a new RedisHLL internal_events metric definition that would include
time_frame
attribute set as an array, for example:time_frame: ['7d', '28d']
- Modify the RedisHLL instrumentation class [or create a new instrumentation class] so that it can handle
time_frame
attribute as an array, generating a hash metric with a structure like:
{
weekly: 12,
monthly: 134
}
- Make it so that the
api/v4/usage_data/metric_definitions
endpoint returns two metric definitions, generated based on just one metric file: a metric definition with thexx.weekly
and a metric definition with thexx.monthly
key_path. - Test those changes with
@rbacovic
to make sure that this does not cause problems further down in the data pipeline. This should be possible to achieve by using a feature flag and/or the staging environment (Radovan will need to be contacted so that we understand how exactly can we test this change). - Modify Metric Dictionary so that it displays the metrics separately:
- add a
include_path
param to theapi/v4/usage_data/metric_definitions
& make it so that when passed, the metrics returned include their filepaths - modify metric dictionary so that it uses this endpoint instead of traversing Gitlab repo's files
How to Verify
Setting up a new metric with both 7d and 28d creates only a single metric definition.
Further Actions
TBD