Skip to content

Provide better output from event and metric definition validator

What does this MR do and why?

Previously the error messages were not very detailed when a metric definition og event defintion were invalid. (See before-screenshot below)

This MR makes sure that all validation errors are printed and that the entire error message and file path is visible.

As part of this change, event definitions are no longer validated when they are instantiated but only through the spec.

Unfortunately the validation messages are not as great as they could be as we are using an old version of json_schemer that doens't provide good human readable error messages. (We have an issue for upgrading the gem)

Screenshots or screen recordings

Before After
image image

How to set up and validate locally

  1. Metric definitions

    1. Start on master
    2. Change - ultimate to - unlimited and remove the line status: active in ee/config/metrics/counts_7d/20210216174838_g_analytics_insights.yml
    3. Run bundle exec rspec spec/lib/gitlab/usage/metric_definition_spec.rb:43
    4. Notice that only one error is printed and that part of the error message isn't displayed.
    5. Switch to this branch and run the spec: bundle exec rspec spec/lib/gitlab/usage/metric_definition_validate_all_spec.rb.
    6. Notice that both error messages are printed and that you can read the full error message.
  2. Event definitions

    1. Start on master
    2. Change - free to -freebie in config/events/1649272430_projectsnew_visit_docs.yml and config/events/1651053267_event_create_service_project_action.yml.
    3. Run bundle exec rspec spec/lib/gitlab/tracking/event_definition_spec.rb:34
    4. Notice that only one error is printed and that part of the error message isn't displayed.
    5. Switch to this branch and run the spec: bundle exec rspec spec/lib/gitlab/tracking/event_definition_validate_all_spec.rb.
    6. Notice that both error messages are printed and that you can read the full error message.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #434694 (closed)

Edited by Jonas Larsen

Merge request reports