Skip to content

Generate Usage ping structure from existing definition YAML files

What does this MR do?

Metrics instrumentation

When adding metrics to Usage Ping we usually add 1 or 2 metrics in one MR

What should be the public API for this?

  • Have custom generators for this files with pre-filled code, easy for devs to work with them
  • Have one method to implement which is returning the data for we add in Usage Ping
  • Have helpers for tests, some shared example maybe
  • Metrics should be independent(consider the possibility of calculating metrics in parallel
  • Classes in metrics yaml, if no class is added use a base class which only ads key and empty value, nils
  • Consider load and share between all <*>_metric.rb files, this way we might not need to send the definition to each metric object

Files needed

  • <metric>.yml
  • <metric>_metric.rb
  • <metric>_metric_spec.rb

Notes

  • Better to be verbose and have one class for each metric in Usage Ping
  • We can consider grouping metrics rb files in specific modules, for example, Issues module could contain metrics related with issues
  • Keep things as simple and boring as possible
  • Think from the perspective of the developer that is adding the metric
  • This instrumentation classes can be used in the current usage ping implementation too, they are having the logic seprate instead of usage_data.rb. by accessing Gitlab::Usage::Metrics::Instrumentations::UuidMetric.new.value we get the value for the metric or together with the key path by using Gitlab::Usage::Metrics::Instrumentations::UuidMetric.new.usage_data.

We could encourage developers to add this classes from this moment on.

Example of usage

in rails console

  1. Usage data for one metric
[10] pry(main)> Gitlab::Usage::Metrics::Instrumentations::UuidMetric.new.usage_data
  ApplicationSetting Load (1.7ms)  SELECT "application_settings".* FROM "application_settings" ORDER BY "application_settings"."id" DESC LIMIT 1 /*application:console,line:/app/models/concerns/cacheable_attributes.rb:19:in `current_without_cache'*/
=> {:uuid=>"7cc411ec-c1c7-46ea-93fd-1563342e6137"}
  1. Usage Ping for all instrumented metrics
Gitlab::UsageDataMetrics.uncached_data

or rake task

bundle exec rake gitlab:usage_data:generate_from_yaml
  1. Run tests
bundle exec rspec  spec/lib/gitlab/usage

Next Steps

  • Create issue for removing the metric.rb class this is not used
  • Create issue to improve generator to generate *_metric.rb and *metric_spec.rb
  • Open MR for docs
  • create issue to change adjust the usage ping to merge with data from the new instrumented metrics

Challenges

  • Naming of the instrumentation classes, as we need one class per metric we will have to be creative with the naming, we might consider an automatic suggestion
  • With this approach we will not have the implementation of the metric in place before the YAML file and we will not be able to use name suggestions. We need to think more about this.

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #299448 (closed)

Edited by Alina Mihaila

Merge request reports