From b97d5b65dd40fb5d8753c0677534e82cb5636f2d Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Fri, 16 Jun 2017 14:23:33 +0200
Subject: [PATCH] Use include ActiveModel::Model to hold metrics data parsed
from yaml.
---
.../prometheus/additional_metrics_parser.rb | 23 ++++++-------------
lib/gitlab/prometheus/metric.rb | 15 ++++++------
lib/gitlab/prometheus/metric_group.rb | 10 +++-----
.../additional_metrics_parser_spec.rb | 23 ++++++++++---------
spec/support/prometheus/metric_builders.rb | 4 ++--
5 files changed, 32 insertions(+), 43 deletions(-)
diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb
index 9bd41b99d81..70151e8c6ad 100644
--- a/lib/gitlab/prometheus/additional_metrics_parser.rb
+++ b/lib/gitlab/prometheus/additional_metrics_parser.rb
@@ -9,26 +9,17 @@ module Gitlab
private
- def metrics_from_list(list)
- list.map { |entry| metric_from_entry(entry) }
- end
-
- def metric_from_entry(entry)
- required_fields = [:title, :required_metrics, :weight, :queries]
- missing_fields = required_fields.select { |key| entry[key].nil? }
- raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty?
-
- Metric.new(entry[:title], entry[:required_metrics], entry[:weight], entry[:y_label], entry[:queries])
+ def validate!(obj)
+ raise ParsingError.new(obj.errors.full_messages.join('\n')) unless obj.valid?
end
def group_from_entry(entry)
- required_fields = [:group, :priority, :metrics]
- missing_fields = required_fields.select { |key| entry[key].nil? }
-
- raise ParsingError.new("entry missing required fields #{missing_fields.map(&:to_s)}") unless missing_fields.empty?
+ entry[:name] = entry.delete(:group)
+ entry[:metrics]&.map! do |entry|
+ Metric.new(entry).tap(&method(:validate!))
+ end
- group = MetricGroup.new(entry[:group], entry[:priority])
- group.tap { |g| g.metrics = metrics_from_list(entry[:metrics]) }
+ MetricGroup.new(entry).tap(&method(:validate!))
end
def additional_metrics_raw
diff --git a/lib/gitlab/prometheus/metric.rb b/lib/gitlab/prometheus/metric.rb
index 5155064317c..f54b2c6aaff 100644
--- a/lib/gitlab/prometheus/metric.rb
+++ b/lib/gitlab/prometheus/metric.rb
@@ -1,14 +1,15 @@
module Gitlab
module Prometheus
class Metric
- attr_reader :group, :title, :required_metrics, :weight, :y_label, :queries
+ include ActiveModel::Model
- def initialize(title, required_metrics, weight, y_label, queries = [])
- @title = title
- @required_metrics = required_metrics
- @weight = weight
- @y_label = y_label || 'Values'
- @queries = queries
+ attr_accessor :title, :required_metrics, :weight, :y_label, :queries
+
+ validates :title, :required_metrics, :weight, :y_label, :queries, presence: true
+
+ def initialize(params = {})
+ super(params)
+ @y_label ||= 'Values'
end
end
end
diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb
index 12bdf407ce0..729fef34b35 100644
--- a/lib/gitlab/prometheus/metric_group.rb
+++ b/lib/gitlab/prometheus/metric_group.rb
@@ -1,14 +1,10 @@
module Gitlab
module Prometheus
class MetricGroup
- attr_reader :priority, :name
- attr_accessor :metrics
+ include ActiveModel::Model
- def initialize(name, priority, metrics = [])
- @name = name
- @priority = priority
- @metrics = metrics
- end
+ attr_accessor :name, :priority, :metrics
+ validates :name, :priority, :metrics, presence: true
def self.all
AdditionalMetricsParser.load_groups_from_yaml
diff --git a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb
index 97280de173e..f8b2746b43d 100644
--- a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb
+++ b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
priority: 1
metrics:
- title: title
- required_metrics: []
+ required_metrics: ['metric_a']
weight: 1
queries: [{query_range: query_range_a}]
EOF
@@ -54,7 +54,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
expect(metrics.count).to eq(3)
expect(metrics[0]).to have_attributes(title: 'title', required_metrics: %w(metric_a metric_b), weight: 1)
expect(metrics[1]).to have_attributes(title: 'title', required_metrics: %w(metric_a), weight: 1)
- expect(metrics[2]).to have_attributes(title: 'title', required_metrics: [], weight: 1)
+ expect(metrics[2]).to have_attributes(title: 'title', required_metrics: %w{metric_a}, weight: 1)
end
it 'provides query data' do
@@ -78,7 +78,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
it 'throws parsing error' do
- expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/)
+ expect { subject }.to raise_error(parser_error_class, /#{field_name} can't be blank/i)
end
end
@@ -88,13 +88,13 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
it 'throws parsing error' do
- expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/)
+ expect { subject }.to raise_error(parser_error_class, /#{field_name} can't be blank/i)
end
end
end
describe 'group required fields' do
- it_behaves_like 'required field', :metrics do
+ it_behaves_like 'required field', 'metrics' do
let(:field_nil) do
<<-EOF.strip_heredoc
- group: group_a
@@ -111,10 +111,11 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
end
- it_behaves_like 'required field', :group do
+ it_behaves_like 'required field', 'name' do
let(:field_nil) do
<<-EOF.strip_heredoc
- - priority: 1
+ - group:
+ priority: 1
metrics: []
EOF
end
@@ -127,7 +128,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
end
- it_behaves_like 'required field', :priority do
+ it_behaves_like 'required field', 'priority' do
let(:field_nil) do
<<-EOF.strip_heredoc
- group: group_a
@@ -146,7 +147,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
describe 'metrics fields parsing' do
- it_behaves_like 'required field', :title do
+ it_behaves_like 'required field', 'title' do
let(:field_nil) do
<<-EOF.strip_heredoc
- group: group_a
@@ -171,7 +172,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
end
- it_behaves_like 'required field', :required_metrics do
+ it_behaves_like 'required field', 'required metrics' do
let(:field_nil) do
<<-EOF.strip_heredoc
- group: group_a
@@ -196,7 +197,7 @@ describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do
end
end
- it_behaves_like 'required field', :weight do
+ it_behaves_like 'required field', 'weight' do
let(:field_nil) do
<<-EOF.strip_heredoc
- group: group_a
diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb
index e4b55c22acd..c8d056d3fc8 100644
--- a/spec/support/prometheus/metric_builders.rb
+++ b/spec/support/prometheus/metric_builders.rb
@@ -9,7 +9,7 @@ module Prometheus
end
def simple_metric(title: 'title', required_metrics: [], queries: [simple_query])
- Gitlab::Prometheus::Metric.new(title, required_metrics, 1, nil, queries)
+ Gitlab::Prometheus::Metric.new(title: title, required_metrics: required_metrics, weight: 1, queries: queries)
end
def simple_metrics(added_metric_name: 'metric_a')
@@ -21,7 +21,7 @@ module Prometheus
end
def simple_metric_group(name: 'name', metrics: simple_metrics)
- Gitlab::Prometheus::MetricGroup.new( name, 1, metrics)
+ Gitlab::Prometheus::MetricGroup.new(name: name, priority: 1, metrics: metrics)
end
end
end
--
GitLab