New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 630455 link

Starred by 4 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 632085
issue 632086



Sign in to add a comment

PRR for Sheriff-o-Matic

Project Member Reported by seanmccullough@google.com, Jul 21 2016

Issue description

Here's the information we have so far, when it's complete we'll add the PRR label and start the review.

========
Name of application:

Sheriff-o-Matic

========
Name of cloud project:

sheriff-o-matic

========
Source location(s):

https://cs.chromium.org/chromium/infra/go/src/infra/appengine/sheriff-o-matic/

========
Url(s) at which app is/will be deployed:

https://sheriff-o-matic.appspot.com
https://sheriff-o-matic-staging.appspot.com


========
Documentation location(s):

SoM: https://chromium.googlesource.com/infra/infra/+/master/go/src/infra/appengine/sheriff-o-matic/

Alerts dispatcher: https://chromium.googlesource.com/infra/infra/+/master/go/src/infra/monitoring/dispatcher/

========
Viceroy dashboard:

SoM Staging: https://viceroy.corp.google.com/chrome_infra/Appengine/sheriff_o_matic_staging
TODO: SoM Prod
alerts-dispatcher: https://viceroy.corp.google.com/chrome_infra/Services/alerts_dispatcher

========
Bug component/queue:

Infra>Sheriffing>SheriffOMatic

========
Bug triage rotation members:

https://chrome-internal.googlesource.com/infra/infra_internal.git/+/master/infra_internal/services/sheriff/rotations/sheriff-o-matic.rotation.json

========
GAR Web Self-Assessment doc:

TODO

========
Security Questionnaire:

TODO



 
Cc: aga...@chromium.org
Project Member

Comment 2 by sheriffbot@chromium.org, Jul 22 2016

Labels: Hotlist-Google
Labels: Milestone-SoMNG
Blockedon: 632085
Blockedon: 632086
Labels: Type-Bug
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/9c71f8a2f9fe10580745745f1ccc3f2ba6201153

commit 9c71f8a2f9fe10580745745f1ccc3f2ba6201153
Author: Sean McCullough <seanmccullough@chromium.org>
Date: Thu Aug 04 18:30:49 2016

[som] improve README.md

BUG= 630455 

Change-Id: I2eec37e428a600d1ce35029c8064ca2cc143b7da
Reviewed-on: https://chromium-review.googlesource.com/366132
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Sean McCullough <seanmccullough@chromium.org>

[modify] https://crrev.com/9c71f8a2f9fe10580745745f1ccc3f2ba6201153/go/src/infra/appengine/sheriff-o-matic/README.md

Description: Show this description
Description: Show this description
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 15 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/6c83b255c1ac96acbf98b2bd690963c449a584b0

commit 6c83b255c1ac96acbf98b2bd690963c449a584b0
Author: seanmccullough <seanmccullough@google.com>
Date: Thu Sep 15 21:44:44 2016

Description: Show this description
agable@: Can we start this review now?
Labels: Infra-PRR
ping?
✓ Go (preferably)
✓ Tests in CQ/CI
✓ Source in canonical location
✓ Design Doc
✗ Test coverage > 80%
✓ Polymer for frontend framework
✗ Billing set up correctly
✓ chrome-troopers is an Owner
✓ Uses gae.py for deployment
✓ Staging instance
✗ Entry in go/chrome-infra-services
✗ Playbook entries
□ Documented APIs
✓ Top-level README
✓ Links to help and documentation
✓ Incorporate ts_mon
✓ Have a viceroy dashboard
✗ Alerts are "Level 3 Certified"
✓ CIT Feedback button
□ Security review
□ Privacy review
✓ Monorail component
✓ Bug triage rotation


So the things that need to be worked on are:
* Get SoM test coverage up from 65% to 80% (or change floor in .infra_testing, if it already is that high).
* Write tests for alerts-dispatcher
* Set up billing correctly (https://docs.google.com/document/d/18f7dLUjaeIYdzVD7rIaQLrNHfCAOgZtzs1Q5Ub0zWi0/edit)
* Update entry in go/chrome-infra-services to point to correct source location and give more details
* Set up alerts from the existing monitoring, or document those alerts in the trooper playbook if they already exist
* Confirm that SoM doesn't expose an API, or if it does, document it
Thanks for the update! Will get to work on the remainders.
Labels: -Milestone-SoMNG Milestone-Reliability
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/a290182dec73ed20a0fb4be49df363f697882e12

commit a290182dec73ed20a0fb4be49df363f697882e12
Author: seanmccullough <seanmccullough@google.com>
Date: Fri Oct 07 17:02:10 2016

Project Member

Comment 19 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/a290182dec73ed20a0fb4be49df363f697882e12

commit a290182dec73ed20a0fb4be49df363f697882e12
Author: seanmccullough <seanmccullough@google.com>
Date: Fri Oct 07 17:02:10 2016

Project Member

Comment 20 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/a290182dec73ed20a0fb4be49df363f697882e12

commit a290182dec73ed20a0fb4be49df363f697882e12
Author: seanmccullough <seanmccullough@google.com>
Date: Fri Oct 07 17:02:10 2016

Regarding the API docs: SoM doesn't expose an API to external (non-infra) users, but the endpoints that alerts-dispatcher POSTs to are documented in Frontend section of the design doc: https://docs.google.com/document/d/1FPgxMWgHN6tBPdeC7Xh8F8s3ozx5qx2Z71iJ3bbTLM0/edit#heading=h.my2jv2xnswb7 
I just finished setting up the billing account per the instructions in that doc.

Remaining steps for this PRR (both in progress):
* Write tests for alerts-dispatcher
* Set up alerts from the existing monitoring, or document those alerts in the trooper playbook if they already exist
Submitted https://critique.corp.google.com/#review/135818379 for the alerts-dispatcher ... alerts :)

So all that's left is the alerts-dispatcher code coverage update, still in review: https://chromium-review.googlesource.com/c/395571/
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/d383c52d708895a738d21fbd4013fab22570b248

commit d383c52d708895a738d21fbd4013fab22570b248
Author: Sean McCullough <seanmccullough@chromium.org>
Date: Wed Oct 12 23:38:45 2016

[a-d] add tests to dispatcher and increase coverage elsewhere

This actually adds more tests to files besides dispatcher.go.
dispatcher.go doesn't contain that much logic. Most of it is parsing
flags and configuration files, thus the low coverage for that package.

BUG= 630455 

Change-Id: I8e6c680fa2e027208faf4817fdc611a60953b02e
Reviewed-on: https://chromium-review.googlesource.com/395571
Reviewed-by: Aaron Gable <agable@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Sean McCullough <seanmccullough@chromium.org>

[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/analyzer/analyzer.go
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/analyzer/analyzer.infra_testing
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/analyzer/analyzer_test.go
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/analyzer/step/step.infra_testing
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/analyzer/step/test_step_test.go
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/analyzer/test/test.infra_testing
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/dispatcher/dispatcher.go
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/dispatcher/dispatcher.infra_testing
[add] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/dispatcher/dispatcher_test.go
[modify] https://crrev.com/d383c52d708895a738d21fbd4013fab22570b248/go/src/infra/monitoring/pubsubalerts/pubsubalerts.infra_testing

✓ Go (preferably)
✓ Tests in CQ/CI
✓ Source in canonical location
✓ Design Doc
✓ Test coverage > 80%
✓ Polymer for frontend framework
✓ Billing set up correctly
✓ chrome-troopers is an Owner
✓ Uses gae.py for deployment
✓ Staging instance
✓ Entry in go/chrome-infra-services
✗ Playbook entries
□ Documented APIs
✓ Top-level README
✓ Links to help and documentation
✓ Incorporate ts_mon
✓ Have a viceroy dashboard
✓ CIT Feedback button
□ Security review
□ Privacy review
✓ Monorail component
✓ Bug triage rotation

The only thing that I see still missing is a go/trooper playbook entry for the new SoM and alerts-dispatcher alerts. Can you make sure they're in there?
Project Member

Comment 26 by bugdroid1@chromium.org, Oct 14 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/916b424c1c35d6630e919c91595b83c56d48749c

commit 916b424c1c35d6630e919c91595b83c56d48749c
Author: seanmccullough <seanmccullough@google.com>
Date: Fri Oct 14 17:43:54 2016

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 14 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/916b424c1c35d6630e919c91595b83c56d48749c

commit 916b424c1c35d6630e919c91595b83c56d48749c
Author: seanmccullough <seanmccullough@google.com>
Date: Fri Oct 14 17:43:54 2016

Project Member

Comment 28 by bugdroid1@chromium.org, Oct 14 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/916b424c1c35d6630e919c91595b83c56d48749c

commit 916b424c1c35d6630e919c91595b83c56d48749c
Author: seanmccullough <seanmccullough@google.com>
Date: Fri Oct 14 17:43:54 2016

Status: Fixed (was: Started)
Cc: dsansome@chromium.org
Cc: jparent@chromium.org jem@chromium.org benhenry@chromium.org estaab@chromium.org
Owner: dsansome@chromium.org
Status: Assigned (was: Fixed)
One thing that's come up a few times recently is getting SRE signoff during PRR, and there's another item in the PRR that was added after this was closed:
□ SREs have done a successful deploy+roll-back cycle

Dave, any objections to using SoM as a guinea pig for signing off on a service at the end of PRR? Sean finished everything else already.
Owner: philwright@chromium.org
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/9659789af81e17c4f4c314e373db795cd0566b3b

commit 9659789af81e17c4f4c314e373db795cd0566b3b
Author: Phil Wright <philwright@chromium.org>
Date: Mon Dec 19 23:28:51 2016

Pushing latest version of SoM as part of PRR deploy and rollback test

BUG= 630455 

Change-Id: I11f2685cff33a8b1f06ffd9949bb8a20e35a1f98
Reviewed-on: https://chromium-review.googlesource.com/422607
Reviewed-by: Sean McCullough <seanmccullough@chromium.org>
Commit-Queue: Phil Wright <philwright@chromium.org>

[modify] https://crrev.com/9659789af81e17c4f4c314e373db795cd0566b3b/go/src/infra/appengine/sheriff-o-matic/RELNOTES.md

Status: Archived (was: Assigned)

Sign in to add a comment