Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

monitoring: wrap alert_count records in min/max#11985

Merged
bobheadxi merged 3 commits into
masterfrom
distribution/provisioning-alerts-fix
Jul 7, 2020
Merged

monitoring: wrap alert_count records in min/max#11985
bobheadxi merged 3 commits into
masterfrom
distribution/provisioning-alerts-fix

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Jul 7, 2020

Copy link
Copy Markdown
Member

Wrap all alert_count records in a min/max operators, and removes "support" for per-instance since this eliminates per-container / per-replica / per-pod labels from the final alert_count rule calculation. This closes #11571, resolving a bug where per-instance support in provisioning alerts were causing them to not apply. See https://github.com/sourcegraph/sourcegraph/issues/11571#issuecomment-654571953

Adjust wording and descriptions on the provisioning alerts.

I tried this out with new gitserver rules via the steps I described here on k8s and provisioning alerts seem to be green now

image

@bobheadxi bobheadxi requested review from a team and emidoots July 7, 2020 07:31
@bobheadxi bobheadxi force-pushed the distribution/provisioning-alerts-fix branch from 1016a58 to 0fce11c Compare July 7, 2020 07:33
Comment thread monitoring/generator.go
Comment thread monitoring/generator.go

@emidoots emidoots left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great!

bobheadxi and others added 2 commits July 7, 2020 15:49
Co-authored-by: Stephen Gutekanst <stephen.gutekanst@gmail.com>
@codecov

codecov Bot commented Jul 7, 2020

Copy link
Copy Markdown

Codecov Report

Merging #11985 into master will increase coverage by 0.36%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master   #11985      +/-   ##
==========================================
+ Coverage   49.57%   49.94%   +0.36%     
==========================================
  Files        1505     1517      +12     
  Lines       88076    88478     +402     
  Branches     6789     6669     -120     
==========================================
+ Hits        43665    44186     +521     
+ Misses      40485    40363     -122     
- Partials     3926     3929       +3     
Flag Coverage Δ
#go 54.34% <58.26%> (+0.02%) ⬆️
#storybook 10.74% <16.66%> (?)
#typescript 36.58% <16.62%> (+1.42%) ⬆️
#unit 49.55% <39.88%> (-0.02%) ⬇️
Impacted Files Coverage Δ
browser/src/shared/code-hosts/bitbucket/api.ts 30.76% <0.00%> (ø)
browser/src/shared/code-hosts/gitlab/api.ts 24.00% <0.00%> (ø)
...wser/src/shared/code-hosts/phabricator/backend.tsx 65.40% <0.00%> (ø)
browser/src/shared/code-hosts/shared/codeHost.tsx 57.40% <0.00%> (-0.42%) ⬇️
browser/src/shared/tracking/eventLogger.tsx 2.32% <0.00%> (-0.24%) ⬇️
cmd/frontend/graphqlbackend/codemod.go 13.33% <0.00%> (ø)
cmd/frontend/graphqlbackend/git_commit.go 24.30% <0.00%> (ø)
cmd/frontend/graphqlbackend/git_revision.go 22.22% <0.00%> (ø)
cmd/frontend/graphqlbackend/hunk.go 0.00% <0.00%> (ø)
cmd/frontend/graphqlbackend/parse_search_query.go 0.00% <0.00%> (ø)
... and 155 more

@bobheadxi bobheadxi merged commit 82cee84 into master Jul 7, 2020
@bobheadxi bobheadxi deleted the distribution/provisioning-alerts-fix branch July 7, 2020 08:22
bobheadxi added a commit that referenced this pull request Jan 27, 2021
Re-introduce "flattening" of observables by wrapping queries in a min or max where appropriate (first added in #11985) to fix a regression introduced in #17599.

Also add monitoring to ensure we have some visibility into rule failures, and reorganize Prometheus panels for uniformity.

Co-authored-by: Gonzalo Peci <pecigonzalo@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

monitoring: provisioning alerts not consistently applied

2 participants