Add dashboard_path to PrometheusMetric
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:
- A user has
dashboard_1
yml that has a metric imported into the database withidentifier = metric_1
. - The user creates a new dashboard,
dashboard_2
also with a metricidentifier = metric_1
. - 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
. - 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. - 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
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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