Commit b144a9f1 authored by Ahmad Sherif's avatar Ahmad Sherif

Merge branch 'bjk/null_values' into 'master'

Add validation of metric values

Closes #60

See merge request gitlab-org/gitlab-exporter!102
parents 2d4a7c3f 2f3355b4
Pipeline #135317 passed with stage
in 1 minute and 4 seconds
...@@ -60,7 +60,7 @@ module GitLab ...@@ -60,7 +60,7 @@ module GitLab
def probe_for_type(type) def probe_for_type(type)
collector.run(type).each do |query_name, data| collector.run(type).each do |query_name, data|
METRIC_KEYS.each do |key| METRIC_KEYS.each do |key|
metrics.add("gitlab_database_bloat_#{type}_#{key}", data[key], query_name: query_name) metrics.add("gitlab_database_bloat_#{type}_#{key}", data[key].to_f, query_name: query_name)
end end
end end
......
...@@ -463,7 +463,7 @@ module GitLab ...@@ -463,7 +463,7 @@ module GitLab
other_values[key] ||= 0 other_values[key] ||= 0
other_values[key] += metric[:value] other_values[key] += metric[:value]
else else
add_ci_created_pending_builds(metric_name, metric[:value], metric) add_ci_created_pending_builds(metric_name, metric[:value].to_f, metric)
end end
end end
...@@ -479,7 +479,7 @@ module GitLab ...@@ -479,7 +479,7 @@ module GitLab
end end
def ci_stale_builds_metrics def ci_stale_builds_metrics
@metrics.add("ci_stale_builds", @results[:stale_builds]) @metrics.add("ci_stale_builds", @results[:stale_builds].to_f)
end end
def metrics_per_runner def metrics_per_runner
...@@ -519,19 +519,19 @@ module GitLab ...@@ -519,19 +519,19 @@ module GitLab
labels[:namespace] = "" unless labels[:namespace] labels[:namespace] = "" unless labels[:namespace]
selected_labels = labels.select { |k, _| allowed_labels.include?(k) }.sort.to_h selected_labels = labels.select { |k, _| allowed_labels.include?(k) }.sort.to_h
@metrics.add(metric_name, value, selected_labels) @metrics.add(metric_name, value.to_f, selected_labels)
end end
def repeated_commands_metrics def repeated_commands_metrics
@results[:repeated_commands].each do |metric| @results[:repeated_commands].each do |metric|
value = metric.delete(:value) value = metric.delete(:value)
@metrics.add("ci_repeated_commands_builds", value, metric) @metrics.add("ci_repeated_commands_builds", value.to_f, metric)
end end
end end
def unarchived_traces_metrics def unarchived_traces_metrics
@metrics.add("ci_unarchived_traces", @results[:unarchived_traces]) @metrics.add("ci_unarchived_traces", @results[:unarchived_traces].to_f)
end end
end end
end end
......
...@@ -48,13 +48,13 @@ module GitLab ...@@ -48,13 +48,13 @@ module GitLab
results.to_a.each do |row| results.to_a.each do |row|
@metrics.add( @metrics.add(
"project_remote_mirror_last_successful_update_time_seconds", "project_remote_mirror_last_successful_update_time_seconds",
row["last_successful_update_at"].to_i, row["last_successful_update_at"].to_f,
project_id: row["project_id"], project_id: row["project_id"],
url: row["url"] url: row["url"]
) )
@metrics.add( @metrics.add(
"project_remote_mirror_last_update_time_seconds", "project_remote_mirror_last_update_time_seconds",
row["last_update_at"].to_i, row["last_update_at"].to_f,
project_id: row["project_id"], project_id: row["project_id"],
url: row["url"] url: row["url"]
) )
......
...@@ -147,7 +147,7 @@ module GitLab ...@@ -147,7 +147,7 @@ module GitLab
def probe_db def probe_db
results = @collector.run results = @collector.run
results.each do |key, value| results.each do |key, value|
@metrics.add("gitlab_database_rows", value.to_i, query_name: key.to_s) @metrics.add("gitlab_database_rows", value.to_f, query_name: key.to_s)
end end
self self
......
...@@ -35,7 +35,9 @@ module GitLab ...@@ -35,7 +35,9 @@ module GitLab
result.each do |table_name, tuple_stats| result.each do |table_name, tuple_stats|
tuple_stats.each do |column_name, value| tuple_stats.each do |column_name, value|
@metrics.add("gitlab_database_stat_table_#{column_name}", value.to_s, table_name: table_name) @metrics.add("gitlab_database_stat_table_#{column_name}",
value.to_f,
table_name: table_name) unless value.is_a?(Numeric)
end end
end end
......
...@@ -16,6 +16,8 @@ module GitLab ...@@ -16,6 +16,8 @@ module GitLab
end end
def add(name, value, quantile = false, **labels) def add(name, value, quantile = false, **labels)
fail "value '#{value}' must be a number" unless value.is_a?(Numeric)
if quantile if quantile
@quantiles[{ name: name, labels: labels }] << value @quantiles[{ name: name, labels: labels }] << value
else else
......
...@@ -206,27 +206,27 @@ describe GitLab::Exporter::Database do ...@@ -206,27 +206,27 @@ describe GitLab::Exporter::Database do
context "when executed on EE" do context "when executed on EE" do
let(:expected_pending_builds) do let(:expected_pending_builds) do
[{ namespace: "1", shared_runners: "yes", has_minutes: "yes", value: 30 }, [{ namespace: "1", shared_runners: "yes", has_minutes: "yes", value: 30.0 },
{ namespace: "2", shared_runners: "yes", has_minutes: "yes", value: 50 }, { namespace: "2", shared_runners: "yes", has_minutes: "yes", value: 50.0 },
{ namespace: "3", shared_runners: "yes", has_minutes: "no", value: 1 }, { namespace: "3", shared_runners: "yes", has_minutes: "no", value: 1.0 },
{ namespace: "4", shared_runners: "yes", has_minutes: "yes", value: 2 }, { namespace: "4", shared_runners: "yes", has_minutes: "yes", value: 2.0 },
{ namespace: "5", shared_runners: "no", has_minutes: "no", value: 2 }] { namespace: "5", shared_runners: "no", has_minutes: "no", value: 2.0 }]
end end
let(:expected_created_builds) do let(:expected_created_builds) do
[{ namespace: "1", shared_runners: "no", has_minutes: "no", value: 10 }, [{ namespace: "1", shared_runners: "no", has_minutes: "no", value: 10.0 },
{ namespace: "2", shared_runners: "no", has_minutes: "no", value: 20 }] { namespace: "2", shared_runners: "no", has_minutes: "no", value: 20.0 }]
end end
let(:expected_per_runner) do let(:expected_per_runner) do
[{ runner: "1", shared_runner: "yes", namespace: "1", mirror: "no", mirror_trigger_builds: "no", scheduled: "yes", triggered: "no", has_minutes: "yes", value: 15 }, [{ runner: "1", shared_runner: "yes", namespace: "1", mirror: "no", mirror_trigger_builds: "no", scheduled: "yes", triggered: "no", has_minutes: "yes", value: 15.0 },
{ runner: "2", shared_runner: "no", namespace: "2", mirror: "yes", mirror_trigger_builds: "yes", scheduled: "no", triggered: "yes", has_minutes: "no", value: 5 }, { runner: "2", shared_runner: "no", namespace: "2", mirror: "yes", mirror_trigger_builds: "yes", scheduled: "no", triggered: "yes", has_minutes: "no", value: 5.0 },
{ runner: "2", shared_runner: "no", namespace: "3", mirror: "yes", mirror_trigger_builds: "yes", scheduled: "no", triggered: "yes", has_minutes: "yes", value: 5 }, { runner: "2", shared_runner: "no", namespace: "3", mirror: "yes", mirror_trigger_builds: "yes", scheduled: "no", triggered: "yes", has_minutes: "yes", value: 5.0 },
{ runner: "3", shared_runner: "no", namespace: "4", mirror: "yes", mirror_trigger_builds: "yes", scheduled: "no", triggered: "yes", has_minutes: "no", value: 5 }] { runner: "3", shared_runner: "no", namespace: "4", mirror: "yes", mirror_trigger_builds: "yes", scheduled: "no", triggered: "yes", has_minutes: "no", value: 5.0 }]
end end
let(:expected_repeated_commands) do let(:expected_repeated_commands) do
[{ namespace: "1", project: "1", shared_runners: "yes", status: "pending", has_minutes: "yes", value: 10 }, [{ namespace: "1", project: "1", shared_runners: "yes", status: "pending", has_minutes: "yes", value: 10.0 },
{ namespace: "2", project: "2", shared_runners: "no", status: "running", has_minutes: "no", value: 20 }, { namespace: "2", project: "2", shared_runners: "no", status: "running", has_minutes: "no", value: 20.0 },
{ namespace: "1", project: "3", shared_runners: "no", status: "pending", has_minutes: "yes", value: 30 }, { namespace: "1", project: "3", shared_runners: "no", status: "pending", has_minutes: "yes", value: 30.0 },
{ namespace: "2", project: "4", shared_runners: "yes", status: "running", has_minutes: "no", value: 40 }] { namespace: "2", project: "4", shared_runners: "yes", status: "running", has_minutes: "no", value: 40.0 }]
end end
before do before do
...@@ -331,11 +331,11 @@ describe GitLab::Exporter::Database do ...@@ -331,11 +331,11 @@ describe GitLab::Exporter::Database do
end end
it "responds with stale builds Prometheus metrics" do it "responds with stale builds Prometheus metrics" do
expect(subject).to match(/^ci_stale_builds 2$/m) expect(subject).to match(/^ci_stale_builds 2.0$/m)
end end
it "responds with unarchived traces Prometheus metrics" do it "responds with unarchived traces Prometheus metrics" do
expect(subject).to match(/^ci_unarchived_traces 10$/m) expect(subject).to match(/^ci_unarchived_traces 10.0$/m)
end end
end end
...@@ -349,34 +349,34 @@ describe GitLab::Exporter::Database do ...@@ -349,34 +349,34 @@ describe GitLab::Exporter::Database do
prober.write_to(writer) prober.write_to(writer)
output = writer.string output = writer.string
expect(output).to match(/^ci_stale_builds 0$/m) expect(output).to match(/^ci_stale_builds 0.0$/m)
end end
end end
end end
context "when executed on EE" do context "when executed on EE" do
let(:ci_created_builds_expected_lines) do let(:ci_created_builds_expected_lines) do
['ci_created_builds\{has_minutes="no",namespace="1",shared_runners="no"\} 10', ['ci_created_builds\{has_minutes="no",namespace="1",shared_runners="no"\} 10.0',
'ci_created_builds\{has_minutes="no",namespace="2",shared_runners="no"\} 20'] 'ci_created_builds\{has_minutes="no",namespace="2",shared_runners="no"\} 20.0']
end end
let(:ci_pending_builds_expected_lines) do let(:ci_pending_builds_expected_lines) do
['ci_pending_builds\{has_minutes="yes",namespace="1",shared_runners="yes"\} 30', ['ci_pending_builds\{has_minutes="yes",namespace="1",shared_runners="yes"\} 30.0',
'ci_pending_builds\{has_minutes="yes",namespace="2",shared_runners="yes"\} 50', 'ci_pending_builds\{has_minutes="yes",namespace="2",shared_runners="yes"\} 50.0',
'ci_pending_builds\{has_minutes="no",namespace="",shared_runners="yes"\} 1', 'ci_pending_builds\{has_minutes="no",namespace="",shared_runners="yes"\} 1.0',
'ci_pending_builds\{has_minutes="yes",namespace="",shared_runners="yes"\} 2', 'ci_pending_builds\{has_minutes="yes",namespace="",shared_runners="yes"\} 2.0',
'ci_pending_builds\{has_minutes="no",namespace="",shared_runners="no"\} 2'] 'ci_pending_builds\{has_minutes="no",namespace="",shared_runners="no"\} 2.0']
end end
let(:ci_running_builds_expected_lines) do let(:ci_running_builds_expected_lines) do
['ci_running_builds\{has_minutes="yes",mirror="no",mirror_trigger_builds="no",namespace="1",runner="1",scheduled="yes",shared_runner="yes",triggered="no"\} 15', ['ci_running_builds\{has_minutes="yes",mirror="no",mirror_trigger_builds="no",namespace="1",runner="1",scheduled="yes",shared_runner="yes",triggered="no"\} 15.0',
'ci_running_builds\{has_minutes="no",mirror="yes",mirror_trigger_builds="yes",namespace="",runner="2",scheduled="no",shared_runner="no",triggered="yes"\} 5', 'ci_running_builds\{has_minutes="no",mirror="yes",mirror_trigger_builds="yes",namespace="",runner="2",scheduled="no",shared_runner="no",triggered="yes"\} 5.0',
'ci_running_builds\{has_minutes="yes",mirror="yes",mirror_trigger_builds="yes",namespace="",runner="2",scheduled="no",shared_runner="no",triggered="yes"\} 5', 'ci_running_builds\{has_minutes="yes",mirror="yes",mirror_trigger_builds="yes",namespace="",runner="2",scheduled="no",shared_runner="no",triggered="yes"\} 5.0',
'ci_running_builds\{has_minutes="no",mirror="yes",mirror_trigger_builds="yes",namespace="",runner="3",scheduled="no",shared_runner="no",triggered="yes"\} 5'] 'ci_running_builds\{has_minutes="no",mirror="yes",mirror_trigger_builds="yes",namespace="",runner="3",scheduled="no",shared_runner="no",triggered="yes"\} 5.0']
end end
let(:ci_repeated_commands_builds_lines) do let(:ci_repeated_commands_builds_lines) do
['ci_repeated_commands_builds\{namespace="1",project="1",shared_runners="yes",status="pending",has_minutes="yes"\} 10', ['ci_repeated_commands_builds\{namespace="1",project="1",shared_runners="yes",status="pending",has_minutes="yes"\} 10.0',
'ci_repeated_commands_builds\{namespace="2",project="2",shared_runners="no",status="running",has_minutes="no"\} 20', 'ci_repeated_commands_builds\{namespace="2",project="2",shared_runners="no",status="running",has_minutes="no"\} 20.0',
'ci_repeated_commands_builds\{namespace="1",project="3",shared_runners="no",status="pending",has_minutes="yes"\} 30', 'ci_repeated_commands_builds\{namespace="1",project="3",shared_runners="no",status="pending",has_minutes="yes"\} 30.0',
'ci_repeated_commands_builds\{namespace="2",project="4",shared_runners="yes",status="running",has_minutes="no"\} 40'] 'ci_repeated_commands_builds\{namespace="2",project="4",shared_runners="yes",status="running",has_minutes="no"\} 40.0']
end end
let(:namespace_out_of_limit) { 2 } let(:namespace_out_of_limit) { 2 }
...@@ -389,25 +389,25 @@ describe GitLab::Exporter::Database do ...@@ -389,25 +389,25 @@ describe GitLab::Exporter::Database do
context "when executed on CE" do context "when executed on CE" do
let(:ci_created_builds_expected_lines) do let(:ci_created_builds_expected_lines) do
['ci_created_builds\{namespace="1",shared_runners="no"\} 10', ['ci_created_builds\{namespace="1",shared_runners="no"\} 10.0',
'ci_created_builds\{namespace="2",shared_runners="no"\} 20'] 'ci_created_builds\{namespace="2",shared_runners="no"\} 20.0']
end end
let(:ci_pending_builds_expected_lines) do let(:ci_pending_builds_expected_lines) do
['ci_pending_builds\{namespace="1",shared_runners="yes"\} 30', ['ci_pending_builds\{namespace="1",shared_runners="yes"\} 30.0',
'ci_pending_builds\{namespace="2",shared_runners="yes"\} 50', 'ci_pending_builds\{namespace="2",shared_runners="yes"\} 50.0',
'ci_pending_builds\{namespace="",shared_runners="yes"\} 3', 'ci_pending_builds\{namespace="",shared_runners="yes"\} 3.0',
'ci_pending_builds\{namespace="",shared_runners="no"\} 2'] 'ci_pending_builds\{namespace="",shared_runners="no"\} 2.0']
end end
let(:ci_running_builds_expected_lines) do let(:ci_running_builds_expected_lines) do
['ci_running_builds\{namespace="1",runner="1",scheduled="yes",shared_runner="yes",triggered="no"\} 15', ['ci_running_builds\{namespace="1",runner="1",scheduled="yes",shared_runner="yes",triggered="no"\} 15.0',
'ci_running_builds\{namespace="",runner="2",scheduled="no",shared_runner="no",triggered="yes"\} 10', 'ci_running_builds\{namespace="",runner="2",scheduled="no",shared_runner="no",triggered="yes"\} 10.0',
'ci_running_builds\{namespace="",runner="3",scheduled="no",shared_runner="no",triggered="yes"\} 5'] 'ci_running_builds\{namespace="",runner="3",scheduled="no",shared_runner="no",triggered="yes"\} 5.0']
end end
let(:ci_repeated_commands_builds_lines) do let(:ci_repeated_commands_builds_lines) do
['ci_repeated_commands_builds\{namespace="1",project="1",shared_runners="yes",status="pending"\} 10', ['ci_repeated_commands_builds\{namespace="1",project="1",shared_runners="yes",status="pending"\} 10.0',
'ci_repeated_commands_builds\{namespace="2",project="2",shared_runners="no",status="running"\} 20', 'ci_repeated_commands_builds\{namespace="2",project="2",shared_runners="no",status="running"\} 20.0',
'ci_repeated_commands_builds\{namespace="1",project="3",shared_runners="no",status="pending"\} 30', 'ci_repeated_commands_builds\{namespace="1",project="3",shared_runners="no",status="pending"\} 30.0',
'ci_repeated_commands_builds\{namespace="2",project="4",shared_runners="yes",status="running"\} 40'] 'ci_repeated_commands_builds\{namespace="2",project="4",shared_runners="yes",status="running"\} 40.0']
end end
let(:namespace_out_of_limit) { 0 } let(:namespace_out_of_limit) { 0 }
......
...@@ -14,4 +14,13 @@ describe GitLab::Exporter::PrometheusMetrics do ...@@ -14,4 +14,13 @@ describe GitLab::Exporter::PrometheusMetrics do
/mymetric{mylabel="x",myotherlabel="y"} 1.3 \d*$/ /mymetric{mylabel="x",myotherlabel="y"} 1.3 \d*$/
) )
end end
it "fails to add a non-numeric metric value" do
expect {
subject.add("mymetric", "1.4", mylabel: "x", myotherlabel: "y").to_s
}.to raise_error(RuntimeError)
expect {
subject.add("mymetric", "invalid", mylabel: "x", myotherlabel: "y").to_s
}.to raise_error(RuntimeError)
end
end end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment