Skip to content

Add dashboard_path to PrometheusMetric

Ryan Cobb requested to merge rc/add_dashboard_path_to_prometheus_metrics into master

What does this MR do?

While reviewing my metric dashboard validator MR, @mikolaj_wawrzyniak brought up a good point.

Once we start importing custom metrics into prometheus_metrics we will need to know what dashboard file they came from in order to know if a user is trying to create a new metric or update an existing one.

For example:

  1. A user has dashboard_1 yml that has a metric imported into the database with identifier = metric_1.
  2. The user creates a new dashboard, dashboard_2 also with a metric identifier = metric_1.
  3. It's clear that the user would like this to be a new metric, but they are unaware they already have another metric with that identifier.
  4. As we import the new dashboard, we have no way of telling the context that this was supposed to be a new metric. Currently we'd only be able to see that the metric exists in the database and we'd have to assume that the user intended to update the metric on dashboard_1, which is not the case! Instead we should send an error to the user alerting them that they already have used that identifier.
  5. In order to have the correct context, we need to store the dashboard path along with the metric. With that extra information we can make the assumption that if the dashboard path is the same as the currently importing metric, that we should update the prometheus_metric. If it is coming from a different dashboard path we need to error.

up

== 20200729175935 AddDashboardPathToPrometheusMetrics: migrating ==============
-- add_column(:prometheus_metrics, :dashboard_path, :text)
   -> 0.0012s
== 20200729175935 AddDashboardPathToPrometheusMetrics: migrated (0.0012s) =====

== 20200730210506 AddTextLimitToDashboardPath: migrating ======================
-- transaction_open?()
   -> 0.0000s
-- execute("ALTER TABLE prometheus_metrics\nADD CONSTRAINT check_0ad9f01463\nCHECK ( char_length(dashboard_path) <= 2048 )\nNOT VALID;\n")
   -> 0.0009s
-- execute("ALTER TABLE prometheus_metrics VALIDATE CONSTRAINT check_0ad9f01463;")
   -> 0.0006s
== 20200730210506 AddTextLimitToDashboardPath: migrated (0.0106s) =============

down

== 20200730210506 AddTextLimitToDashboardPath: reverting ======================
-- execute("ALTER TABLE prometheus_metrics\nDROP CONSTRAINT IF EXISTS check_0ad9f01463\n")
   -> 0.0009s
== 20200730210506 AddTextLimitToDashboardPath: reverted (0.0044s) =============

== 20200729175935 AddDashboardPathToPrometheusMetrics: reverting ==============
-- remove_column(:prometheus_metrics, :dashboard_path)
   -> 0.0012s
== 20200729175935 AddDashboardPathToPrometheusMetrics: reverted (0.0013s) =====

Screenshots

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
Edited by Ryan Cobb

Merge request reports