diff options
author | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-06-09 12:43:12 +0200 |
---|---|---|
committer | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-06-09 22:47:01 +0200 |
commit | 0e7e7c2f2bd0e9c913cda438826a60e761130271 (patch) | |
tree | 0ba18ecb29cabc8cd80c9961daf20414fe368b71 | |
parent | f78fd3de5d55830c84f25cfd50d399e7ddaddf30 (diff) | |
download | gitlab-ce-0e7e7c2f2bd0e9c913cda438826a60e761130271.tar.gz |
Test Additional metrics parser and fix query checking tests
5 files changed, 265 insertions, 11 deletions
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index d81fec3514f..c9df678f97e 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -75,10 +75,6 @@ class PrometheusService < MonitoringService with_reactive_cache(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) end - def with_reactive_cache(*args, &block) - yield calculate_reactive_cache(*args) - end - # Cache metrics for specific environment def calculate_reactive_cache(query_class_name, *args) return unless active? && project && !project.pending_delete? diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index 694001605ef..9bd41b99d81 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -14,22 +14,29 @@ module Gitlab end def metric_from_entry(entry) - missing_fields = [:title, :required_metrics, :weight, :queries].select { |key| !entry.key?(key) } + 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]) end def group_from_entry(entry) - missing_fields = [:group, :priority, :metrics].select { |key| !entry.key?(key) } - raise ParsingError.new("entry missing required fields #{missing_fields}") unless missing_fields.empty? + 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? group = MetricGroup.new(entry[:group], entry[:priority]) group.tap { |g| g.metrics = metrics_from_list(entry[:metrics]) } end def additional_metrics_raw - @additional_metrics_raw ||= YAML.load_file(Rails.root.join('config/additional_metrics.yml'))&.map(&:deep_symbolize_keys).freeze + @additional_metrics_raw ||= load_yaml_file&.map(&:deep_symbolize_keys).freeze + end + + def load_yaml_file + YAML.load_file(Rails.root.join('config/additional_metrics.yml')) end end end diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index a5a401f84cd..e44be770544 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -37,18 +37,19 @@ module Gitlab def query_with_result(query) query[:result]&.any? do |item| - item&.[]('values')&.any? || item&.[]('value')&.any? + item&.[](:values)&.any? || item&.[](:value)&.any? end end def process_query(context, query) query_with_result = query.dup - query_with_result[:result] = + result = if query.key?(:query_range) client_query_range(query[:query_range] % context, start: context[:timeframe_start], stop: context[:timeframe_end]) else client_query(query[:query] % context, time: context[:timeframe_end]) end + query_with_result[:result] = result&.map(&:deep_symbolize_keys) query_with_result end diff --git a/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb new file mode 100644 index 00000000000..97280de173e --- /dev/null +++ b/spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb @@ -0,0 +1,250 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::AdditionalMetricsParser, lib: true do + include Prometheus::MetricBuilders + + let(:parser_error_class) { Gitlab::Prometheus::ParsingError } + + describe '#load_groups_from_yaml' do + subject { described_class.load_groups_from_yaml } + + describe 'parsing sample yaml' do + let(:sample_yaml) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: "title" + required_metrics: [ metric_a, metric_b ] + weight: 1 + queries: [{ query_range: 'query_range_a', label: label, unit: unit }] + - title: "title" + required_metrics: [metric_a] + weight: 1 + queries: [{ query_range: 'query_range_empty' }] + - group: group_b + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: 1 + queries: [{query_range: query_range_a}] + EOF + end + + before do + described_class.instance_variable_set :@additional_metrics_raw, nil + allow(described_class).to receive(:load_yaml_file) { YAML.load(sample_yaml) } + end + + it 'parses to two metric groups with 2 and 1 metric respectively' do + expect(subject.count).to eq(2) + expect(subject[0].metrics.count).to eq(2) + expect(subject[1].metrics.count).to eq(1) + end + + it 'provide group data' do + expect(subject[0]).to have_attributes(name: 'group_a', priority: 1) + expect(subject[1]).to have_attributes(name: 'group_b', priority: 1) + end + + it 'provides metrics data' do + metrics = subject.flat_map(&:metrics) + + 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) + end + + it 'provides query data' do + queries = subject.flat_map(&:metrics).flat_map(&:queries) + + expect(queries.count).to eq(3) + expect(queries[0]).to eq(query_range: 'query_range_a', label: 'label', unit: 'unit') + expect(queries[1]).to eq(query_range: 'query_range_empty') + expect(queries[2]).to eq(query_range: 'query_range_a') + end + end + + shared_examples 'required field' do |field_name| + before do + described_class.instance_variable_set :@additional_metrics_raw, nil + end + + context "when #{field_name} is nil" do + before do + allow(described_class).to receive(:load_yaml_file) { YAML.load(field_missing) } + end + + it 'throws parsing error' do + expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/) + end + end + + context "when #{field_name} are not specified" do + before do + allow(described_class).to receive(:load_yaml_file) { YAML.load(field_nil) } + end + + it 'throws parsing error' do + expect { subject }.to raise_error(parser_error_class, /missing.*#{field_name}/) + end + end + end + + describe 'group required fields' do + it_behaves_like 'required field', :metrics do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + EOF + end + end + + it_behaves_like 'required field', :group do + let(:field_nil) do + <<-EOF.strip_heredoc + - priority: 1 + metrics: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - priority: 1 + metrics: [] + EOF + end + end + + it_behaves_like 'required field', :priority do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: + metrics: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + metrics: [] + EOF + end + end + end + + describe 'metrics fields parsing' do + it_behaves_like 'required field', :title do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: + required_metrics: [] + weight: 1 + queries: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - required_metrics: [] + weight: 1 + queries: [] + EOF + end + end + + it_behaves_like 'required field', :required_metrics do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: + weight: 1 + queries: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + weight: 1 + queries: [] + EOF + end + end + + it_behaves_like 'required field', :weight do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: + queries: [] + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + queries: [] + EOF + end + end + + it_behaves_like 'required field', :queries do + let(:field_nil) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: 1 + queries: + EOF + end + + let(:field_missing) do + <<-EOF.strip_heredoc + - group: group_a + priority: 1 + metrics: + - title: title + required_metrics: [] + weight: 1 + EOF + end + end + end + end +end diff --git a/spec/support/prometheus/metric_builders.rb b/spec/support/prometheus/metric_builders.rb index 18378ec0145..e4b55c22acd 100644 --- a/spec/support/prometheus/metric_builders.rb +++ b/spec/support/prometheus/metric_builders.rb @@ -21,7 +21,7 @@ module Prometheus end def simple_metric_group(name: 'name', metrics: simple_metrics) - Gitlab::Prometheus::MetricGroup.new(name: name, priority: 1, metrics: metrics) + Gitlab::Prometheus::MetricGroup.new( name, 1, metrics) end end end |