Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support id lookup on slo timeseries dashboard widgets #312

Merged
merged 13 commits into from
May 14, 2024
1 change: 1 addition & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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" }`|
smo921 marked this conversation as resolved.
Show resolved Hide resolved
|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"]`|
Expand Down
12 changes: 12 additions & 0 deletions lib/kennel/models/dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,18 @@ 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|
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
end
smo921 marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
Expand Down
1 change: 1 addition & 0 deletions template/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"]`|
Expand Down
35 changes: 35 additions & 0 deletions test/kennel/models/dashboard_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ 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
smo921 marked this conversation as resolved.
Show resolved Hide resolved
it "ignores unknown widget types" do
definition[:type] = "unknown"
resolve.keys.must_equal [:requests, :type, :title]
end

describe "uptime" do
before { definition[:type] = "uptime" }

Expand Down Expand Up @@ -232,6 +239,34 @@ 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: "metrics", query: "avg:system.cpu.user{*}" },
{ data_source: "slo", slo_id: nil }
]
}
end

it "ignores missing ids" do
assert_nil resolve[:requests].last[:queries].last[:slo_id]
smo921 marked this conversation as resolved.
Show resolved Hide resolved
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 "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")
id_map.set("slo", "#{project.kennel_id}:b", "123")
resolved = resolve
resolved[:requests].last[:queries].last[:slo_id].must_equal "123"
end
Comment on lines +260 to +266
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice test!

end
end

describe "#diff" do
Expand Down
Loading