From 68576db650d06e244d3f0195fc0944b2491d2375 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 12:31:18 -0400 Subject: [PATCH 01/13] support id lookup on slo timeseries dashboard widgets --- lib/kennel/models/dashboard.rb | 14 ++++++++++++++ test/kennel/models/dashboard_test.rb | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/kennel/models/dashboard.rb b/lib/kennel/models/dashboard.rb index 4f588e75..8ef6468a 100644 --- a/lib/kennel/models/dashboard.rb +++ b/lib/kennel/models/dashboard.rb @@ -195,6 +195,20 @@ def resolve_linked_tracking_ids!(id_map, **args) if id = definition[:slo_id] definition[:slo_id] = resolve(id, :slo, id_map, **args) || id end + when "timeseries" + if requests = definition[:requests] + requests.map do |request| + if queries = request[:queries] + queries.map do |query| + if query[:data_source] == 'slo' + if id = query[:slo_id] + query[:slo_id] = resolve(id, :slo, id_map, **args) || id + end + end + end + end + end + end end end end diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 4f1fa3b4..ac3a35e2 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -232,6 +232,27 @@ def resolve(force: false) resolved[:widgets][0][:definition][:slo_id].must_equal "123" end end + + describe "timeseries" do + before do + definition[:requests] << { + queries: [{ data_source: 'slo', slo_id: nil }] + } + end + + it "does not modify regular ids" do + definition[:requests].last[:queries].first[:slo_id] = "abcdef1234567" + resolve[:requests].last[:queries].first[:slo_id].must_equal "abcdef1234567" + end + + it "resolves the slo widget with full id" do + definition[:requests].last[:queries].first[:slo_id] = "#{project.kennel_id}:b" + id_map.set("slo", "a:c", "1") + id_map.set("slo", "#{project.kennel_id}:b", "123") + resolved = resolve + resolved[:requests].last[:queries].first[:slo_id].must_equal "123" + end + end end describe "#diff" do From 778d296cd3a9c0404f141b009f0e9abd99195562 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 12:47:06 -0400 Subject: [PATCH 02/13] fixup rubocop warnings --- lib/kennel/models/dashboard.rb | 12 +++++------- test/kennel/models/dashboard_test.rb | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/kennel/models/dashboard.rb b/lib/kennel/models/dashboard.rb index 8ef6468a..7458ca47 100644 --- a/lib/kennel/models/dashboard.rb +++ b/lib/kennel/models/dashboard.rb @@ -198,13 +198,11 @@ def resolve_linked_tracking_ids!(id_map, **args) when "timeseries" if requests = definition[:requests] requests.map do |request| - if queries = request[:queries] - queries.map do |query| - if query[:data_source] == 'slo' - if id = query[:slo_id] - query[:slo_id] = resolve(id, :slo, id_map, **args) || id - end - end + next unless queries = request[:queries] + queries.map do |query| + next unless query[:data_source] == "slo" + if id = query[:slo_id] + query[:slo_id] = resolve(id, :slo, id_map, **args) || id end end end diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index ac3a35e2..4092af27 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -236,7 +236,7 @@ def resolve(force: false) describe "timeseries" do before do definition[:requests] << { - queries: [{ data_source: 'slo', slo_id: nil }] + queries: [{ data_source: "slo", slo_id: nil }] } end From 2ef84ffa22a0f54c7dc686806d26b3572e3dd8cf Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 13:12:10 -0400 Subject: [PATCH 03/13] fixup tests --- test/kennel/models/dashboard_test.rb | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 4092af27..57c266d3 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -151,6 +151,11 @@ def resolve(force: false) resolve.must_be_nil end + it "ignores unknown widget types" do + definition[:type] = "unknown" + resolve.keys.must_equal [:requests, :type, :title] + end + describe "uptime" do before { definition[:type] = "uptime" } @@ -236,21 +241,32 @@ def resolve(force: false) describe "timeseries" do before do definition[:requests] << { - queries: [{ data_source: "slo", slo_id: nil }] + queries: [ + { data_source: "metrics", query: "avg:system.cpu.user{*}" }, + { data_source: "slo", slo_id: nil } + ] } end + it "does nothing for regular widgets" do + resolve.keys.must_equal [:requests, :type, :title] + end + it "does not modify regular ids" do - definition[:requests].last[:queries].first[:slo_id] = "abcdef1234567" - resolve[:requests].last[:queries].first[:slo_id].must_equal "abcdef1234567" + definition[:requests].last[:queries].last[:slo_id] = "abcdef1234567" + resolve[:requests].last[:queries].last[:slo_id].must_equal "abcdef1234567" + end + + it "ignores missing ids" do + assert_nil resolve[:requests].last[:queries].last[:slo_id] end it "resolves the slo widget with full id" do - definition[:requests].last[:queries].first[:slo_id] = "#{project.kennel_id}:b" + definition[:requests].last[:queries].last[:slo_id] = "#{project.kennel_id}:b" id_map.set("slo", "a:c", "1") id_map.set("slo", "#{project.kennel_id}:b", "123") resolved = resolve - resolved[:requests].last[:queries].first[:slo_id].must_equal "123" + resolved[:requests].last[:queries].last[:slo_id].must_equal "123" end end end From bcb809ec93dc8f45b8482a5c8430b104f527566d Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 13:14:32 -0400 Subject: [PATCH 04/13] remove unnecessary test case --- test/kennel/models/dashboard_test.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 57c266d3..3bc87938 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -248,10 +248,6 @@ def resolve(force: false) } end - it "does nothing for regular widgets" do - resolve.keys.must_equal [:requests, :type, :title] - end - it "does not modify regular ids" do definition[:requests].last[:queries].last[:slo_id] = "abcdef1234567" resolve[:requests].last[:queries].last[:slo_id].must_equal "abcdef1234567" From 0c2dbdcc27142998c490a25c0e5be4dfa1013236 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 13:15:24 -0400 Subject: [PATCH 05/13] reorder test cases --- test/kennel/models/dashboard_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 3bc87938..4b8fa545 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -248,15 +248,15 @@ def resolve(force: false) } end + it "ignores missing ids" do + assert_nil resolve[:requests].last[:queries].last[:slo_id] + end + it "does not modify regular ids" do definition[:requests].last[:queries].last[:slo_id] = "abcdef1234567" resolve[:requests].last[:queries].last[:slo_id].must_equal "abcdef1234567" end - it "ignores missing ids" do - assert_nil resolve[:requests].last[:queries].last[:slo_id] - end - it "resolves the slo widget with full id" do definition[:requests].last[:queries].last[:slo_id] = "#{project.kennel_id}:b" id_map.set("slo", "a:c", "1") From 4f85efe10b7840cf20b485861ca4801799dbffe5 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 14:40:12 -0400 Subject: [PATCH 06/13] add comment for new test case --- test/kennel/models/dashboard_test.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 4b8fa545..4893bca2 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -151,6 +151,8 @@ def resolve(force: false) resolve.must_be_nil end + # Test is necessary for code coverage after adding timeseries support, + # to ensure the case statement is tested for unmatched types it "ignores unknown widget types" do definition[:type] = "unknown" resolve.keys.must_equal [:requests, :type, :title] From dcc44433685b24be631f60496d129a54b1726571 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 16:29:41 -0400 Subject: [PATCH 07/13] update readme --- Readme.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Readme.md b/Readme.md index 50248bc9..7d3923be 100644 --- a/Readme.md +++ b/Readme.md @@ -342,6 +342,7 @@ so they can be created in a single update and can be re-created if any of them i |Dashboard|uptime|`monitor: {id: "foo:bar"}`| |Dashboard|alert_graph|`alert_id: "foo:bar"`| |Dashboard|slo|`slo_id: "foo:bar"`| +|Dashboard|timeseries|`slo_id: "foo:bar"`| |Monitor|composite|`query: -> { "%{foo:bar} && %{foo:baz}" }`| |Monitor|slo alert|`query: -> { "error_budget(\"%{foo:bar}\").over(\"7d\") > 123.0" }`| |Slo|monitor|`monitor_ids: -> ["foo:bar"]`| From 784ae769f105c85c74dce79072cab2e1b15b6a77 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 16:33:20 -0400 Subject: [PATCH 08/13] fixup readme fixup --- Readme.md | 1 - 1 file changed, 1 deletion(-) diff --git a/Readme.md b/Readme.md index 7d3923be..50248bc9 100644 --- a/Readme.md +++ b/Readme.md @@ -342,7 +342,6 @@ so they can be created in a single update and can be re-created if any of them i |Dashboard|uptime|`monitor: {id: "foo:bar"}`| |Dashboard|alert_graph|`alert_id: "foo:bar"`| |Dashboard|slo|`slo_id: "foo:bar"`| -|Dashboard|timeseries|`slo_id: "foo:bar"`| |Monitor|composite|`query: -> { "%{foo:bar} && %{foo:baz}" }`| |Monitor|slo alert|`query: -> { "error_budget(\"%{foo:bar}\").over(\"7d\") > 123.0" }`| |Slo|monitor|`monitor_ids: -> ["foo:bar"]`| From 59a0c5cd980fdbdeefe2b3890ab2a84e80a0c7c7 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Mon, 13 May 2024 16:36:23 -0400 Subject: [PATCH 09/13] fixup readme fixup --- Readme.md | 1 + template/Readme.md | 1 + 2 files changed, 2 insertions(+) diff --git a/Readme.md b/Readme.md index 50248bc9..aa7b32c2 100644 --- a/Readme.md +++ b/Readme.md @@ -342,6 +342,7 @@ so they can be created in a single update and can be re-created if any of them i |Dashboard|uptime|`monitor: {id: "foo:bar"}`| |Dashboard|alert_graph|`alert_id: "foo:bar"`| |Dashboard|slo|`slo_id: "foo:bar"`| +|Dashboard|timeseries|`query: -> { data_source: "slo", slo_id: "foo:bar" }`| |Monitor|composite|`query: -> { "%{foo:bar} && %{foo:baz}" }`| |Monitor|slo alert|`query: -> { "error_budget(\"%{foo:bar}\").over(\"7d\") > 123.0" }`| |Slo|monitor|`monitor_ids: -> ["foo:bar"]`| diff --git a/template/Readme.md b/template/Readme.md index 1b7fce83..7d6be77b 100644 --- a/template/Readme.md +++ b/template/Readme.md @@ -324,6 +324,7 @@ so they can be created in a single update and can be re-created if any of them i |Dashboard|uptime|`monitor: {id: "foo:bar"}`| |Dashboard|alert_graph|`alert_id: "foo:bar"`| |Dashboard|slo|`slo_id: "foo:bar"`| +|Dashboard|timeseries|`query: -> { data_source: "slo", slo_id: "foo:bar" }`| |Monitor|composite|`query: -> { "%{foo:bar} && %{foo:baz}" }`| |Monitor|slo alert|`query: -> { "error_budget(\"%{foo:bar}\").over(\"7d\") > 123.0" }`| |Slo|monitor|`monitor_ids: -> ["foo:bar"]`| From 139ae120460dca2cd6ab999e30f122e1e83362a8 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Tue, 14 May 2024 08:17:26 -0400 Subject: [PATCH 10/13] Update lib/kennel/models/dashboard.rb Co-authored-by: Michael Grosser --- lib/kennel/models/dashboard.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/kennel/models/dashboard.rb b/lib/kennel/models/dashboard.rb index 7458ca47..afac7295 100644 --- a/lib/kennel/models/dashboard.rb +++ b/lib/kennel/models/dashboard.rb @@ -196,14 +196,10 @@ def resolve_linked_tracking_ids!(id_map, **args) definition[:slo_id] = resolve(id, :slo, id_map, **args) || id end when "timeseries" - if requests = definition[:requests] - requests.map do |request| - next unless queries = request[:queries] - queries.map do |query| - next unless query[:data_source] == "slo" - if id = query[:slo_id] - query[:slo_id] = resolve(id, :slo, id_map, **args) || id - end + definition[:requests]&.each do |request| + request[:queries]&.each do |query| + if query[:data_source] == "slo" && (slo_id = query[:slo_id]) + query[:slo_id] = resolve(slo_id, :slo, id_map, **args) || slo_id end end end From 925dde534e0c9f24ac2026e99c5e2d784b40d58d Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Tue, 14 May 2024 08:18:19 -0400 Subject: [PATCH 11/13] Update test/kennel/models/dashboard_test.rb Co-authored-by: Michael Grosser --- test/kennel/models/dashboard_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 4893bca2..4b8fa545 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -151,8 +151,6 @@ def resolve(force: false) resolve.must_be_nil end - # Test is necessary for code coverage after adding timeseries support, - # to ensure the case statement is tested for unmatched types it "ignores unknown widget types" do definition[:type] = "unknown" resolve.keys.must_equal [:requests, :type, :title] From d7420a70ee8128a4030af33b7e4b06d6541997cd Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Tue, 14 May 2024 08:19:20 -0400 Subject: [PATCH 12/13] Update test/kennel/models/dashboard_test.rb Co-authored-by: Michael Grosser --- test/kennel/models/dashboard_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/kennel/models/dashboard_test.rb b/test/kennel/models/dashboard_test.rb index 4b8fa545..8fc2e85b 100644 --- a/test/kennel/models/dashboard_test.rb +++ b/test/kennel/models/dashboard_test.rb @@ -249,7 +249,7 @@ def resolve(force: false) end it "ignores missing ids" do - assert_nil resolve[:requests].last[:queries].last[:slo_id] + assert_nil resolve.dig(:requests, 0, :queries, 0, :slo_id) end it "does not modify regular ids" do From ea00dd3a8e67c078fe7680412419f2fdd5126048 Mon Sep 17 00:00:00 2001 From: Stephen Oberther Date: Tue, 14 May 2024 08:19:47 -0400 Subject: [PATCH 13/13] Update Readme.md Co-authored-by: Michael Grosser --- Readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Readme.md b/Readme.md index aa7b32c2..0838b69e 100644 --- a/Readme.md +++ b/Readme.md @@ -342,7 +342,7 @@ so they can be created in a single update and can be re-created if any of them i |Dashboard|uptime|`monitor: {id: "foo:bar"}`| |Dashboard|alert_graph|`alert_id: "foo:bar"`| |Dashboard|slo|`slo_id: "foo:bar"`| -|Dashboard|timeseries|`query: -> { data_source: "slo", slo_id: "foo:bar" }`| +|Dashboard|timeseries|`queries: [{ data_source: "slo", slo_id: "foo:bar" }]`| |Monitor|composite|`query: -> { "%{foo:bar} && %{foo:baz}" }`| |Monitor|slo alert|`query: -> { "error_budget(\"%{foo:bar}\").over(\"7d\") > 123.0" }`| |Slo|monitor|`monitor_ids: -> ["foo:bar"]`|