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

Alerting: Improve performance of matching captures #71828

Merged

Conversation

grobinson-grafana
Copy link
Contributor

@grobinson-grafana grobinson-grafana commented Jul 18, 2023

What is this feature?

This commit updates eval.go to improve the performance of matching captures in the general case. In some cases we have reduced the runtime of the function from 10s of minutes to a couple 100ms. In the case where no capture matches the exact labels, we revert to the current subset/superset match, but with a reduced search space due to grouping captures.

Benchmarks

All benchmarks are show as before and after benchmarks.

100 alerts

go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	   35728	    997875 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	46.346s
go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	  199930	    180568 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	38.337s

500 alerts

go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	    1627	  22169211 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	38.737s
go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	   40276	    911248 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	46.170s

5000 alerts

go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	      16	2133231219 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	36.677s
go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	    3922	   9225236 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	37.556s

10,000 alerts

go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	       4	8555365688 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	68.640s
go test -bench=. -benchtime=30s
goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana/pkg/services/ngalert/eval
BenchmarkEvaluate-8   	    1882	  18879993 ns/op
PASS
ok  	github.com/grafana/grafana/pkg/services/ngalert/eval	37.919s

Why do we need this feature?

[Add a description of the problem the feature is trying to solve.]

Who is this feature for?

[Add information on what kind of user the feature is for.]

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@grobinson-grafana grobinson-grafana added the area/alerting Grafana Alerting label Jul 18, 2023
@grobinson-grafana grobinson-grafana added this to the 10.0.x milestone Jul 18, 2023
@grobinson-grafana grobinson-grafana requested review from a team, rwwiv and JacobsonMT and removed request for a team July 18, 2023 09:47
@grobinson-grafana grobinson-grafana self-assigned this Jul 18, 2023
@grobinson-grafana grobinson-grafana requested review from yuri-tceretian, a team, gotjosh, JohnnyQQQQ, alexweav, santihernandezc, stevesg and dylanherbig and removed request for a team July 18, 2023 09:48
@ephemeral-instances-bot
Copy link

👋 comment /deploy-to-hg to deploy this PR to the hosted Grafana dev environment.

@grafana-delivery-bot
Copy link
Contributor

Hello @grobinson-grafana!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@grafana-delivery-bot
Copy link
Contributor

Hello @grobinson-grafana!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

if theseLabels.Equals(cap.Labels) || theseLabels.Contains(cap.Labels) || cap.Labels.Contains(theseLabels) {
if frame.Meta.Custom == nil {
frame.Meta.Custom = []NumberValueCapture{}
fp := theseLabels.Fingerprint()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@yuri-tceretian yuri-tceretian left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@stevesg stevesg left a comment

Choose a reason for hiding this comment

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

Nice work. PTAL at my comments but they are non-blocking so approving to unblock you.

pkg/services/ngalert/eval/eval.go Outdated Show resolved Hide resolved
pkg/services/ngalert/eval/eval.go Outdated Show resolved Hide resolved
This commit updates eval.go to improve the performance of matching
captures in the general case. In some cases we have reduced the
runtime of the function from 10s of minutes to a couple 100ms.
In the case where no capture matches the exact labels, we revert to
the current subset/superset match, but with a reduced search space
due to grouping captures.
@grobinson-grafana grobinson-grafana force-pushed the grobinson/improve-performance-matching-captures branch from d31c8e0 to cef8a5c Compare July 19, 2023 11:58
@grobinson-grafana grobinson-grafana merged commit 8dd3eb8 into main Jul 20, 2023
11 checks passed
@grobinson-grafana grobinson-grafana deleted the grobinson/improve-performance-matching-captures branch July 20, 2023 08:07
@grafana-delivery-bot grafana-delivery-bot bot modified the milestones: 10.0.x, 10.1.x Jul 20, 2023
grafana-delivery-bot bot pushed a commit that referenced this pull request Jul 20, 2023
This commit updates eval.go to improve the performance of matching
captures in the general case. In some cases we have reduced the
runtime of the function from 10s of minutes to a couple 100ms.
In the case where no capture matches the exact labels, we revert to
the current subset/superset match, but with a reduced search space
due to grouping captures.

(cherry picked from commit 8dd3eb8)
grafana-delivery-bot bot pushed a commit that referenced this pull request Jul 20, 2023
This commit updates eval.go to improve the performance of matching
captures in the general case. In some cases we have reduced the
runtime of the function from 10s of minutes to a couple 100ms.
In the case where no capture matches the exact labels, we revert to
the current subset/superset match, but with a reduced search space
due to grouping captures.

(cherry picked from commit 8dd3eb8)
linoman pushed a commit that referenced this pull request Jul 24, 2023
This commit updates eval.go to improve the performance of matching
captures in the general case. In some cases we have reduced the
runtime of the function from 10s of minutes to a couple 100ms.
In the case where no capture matches the exact labels, we revert to
the current subset/superset match, but with a reduced search space
due to grouping captures.
armandgrillet pushed a commit that referenced this pull request Jul 24, 2023
* Alerting: Improve performance of matching captures (#71828)

This commit updates eval.go to improve the performance of matching
captures in the general case. In some cases we have reduced the
runtime of the function from 10s of minutes to a couple 100ms.
In the case where no capture matches the exact labels, we revert to
the current subset/superset match, but with a reduced search space
due to grouping captures.

(cherry picked from commit 8dd3eb8)

* Add label fingerprints from grafana-plugin-sdk-go

* Remove unsafe.StringData as we use Go 1.19

* Fix lint

---------

Co-authored-by: George Robinson <george.robinson@grafana.com>
armandgrillet pushed a commit that referenced this pull request Jul 24, 2023
* Alerting: Improve performance of matching captures (#71828)

This commit updates eval.go to improve the performance of matching
captures in the general case. In some cases we have reduced the
runtime of the function from 10s of minutes to a couple 100ms.
In the case where no capture matches the exact labels, we revert to
the current subset/superset match, but with a reduced search space
due to grouping captures.

(cherry picked from commit 8dd3eb8)

* Add label fingerprints from grafana-plugin-sdk-go

* Remove unsafe.StringData as we use Go 1.19

* Fix lint

---------

Co-authored-by: George Robinson <george.robinson@grafana.com>
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants