Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2418063002:
Milo: Add ts_mon metrics for master json datastore success (Closed)
|
Created:
4 years, 2 months ago by Ryan Tseng Modified:
4 years, 1 month ago CC:
chromium-reviews, infra-reviews+luci-go_chromium.org, andrew.wang, todd, tandrii+luci-go_chromium.org, M-A Ruel Target Ref:
refs/heads/master Project:
luci-go Visibility:
Public. |
DescriptionMilo: Add ts_mon metrics for master json datastore success
BUG=655863
Committed: https://github.com/luci/luci-go/commit/7d92eccc01a9b20b65de213753701e35658ae57f
Patch Set 1 #
Total comments: 7
Patch Set 2 : Review #Patch Set 3 : GoImports #Patch Set 4 : Rebase #Patch Set 5 : fix #Messages
Total messages: 50 (34 generated)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hinoka@chromium.org changed reviewers: + dsansome@chromium.org, hinoka@chromium.org
ptal
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... File milo/appengine/buildbot/pubsub.go (right): https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:52: // Status can be one of 2 options. "Success", "Failure". Convention is for these field values to be lower-case: "success", "failure" https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:147: // getOSInfo fetches the os family and version of hte build from the While I'm here, s/hte/the/ https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:147: // getOSInfo fetches the os family and version of hte build from the And should it be "of the slave" rather than "of the build"? https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:306: masterCounter.Add(c, 1, internal, master.Name, "Failure") Does master.Name have a "master." prefix? If not it might be difficult to join with other metrics.
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by hinoka@chromium.org
The CQ bit was checked by hinoka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dsansome@chromium.org Link to the patchset: https://codereview.chromium.org/2418063002/#ps20001 (title: "Review")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Linux Precise 32 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/31ed29d892159110)
GoImports
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by hinoka@chromium.org
The CQ bit was checked by hinoka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dsansome@chromium.org Link to the patchset: https://codereview.chromium.org/2418063002/#ps40001 (title: "GoImports")
https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... File milo/appengine/buildbot/pubsub.go (right): https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:52: // Status can be one of 2 options. "Success", "Failure". On 2016/10/16 23:45:36, dsansome wrote: > Convention is for these field values to be lower-case: "success", "failure" Done. https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:147: // getOSInfo fetches the os family and version of hte build from the On 2016/10/16 23:45:36, dsansome wrote: > While I'm here, s/hte/the/ Done. https://codereview.chromium.org/2418063002/diff/1/milo/appengine/buildbot/pub... milo/appengine/buildbot/pubsub.go:306: masterCounter.Add(c, 1, internal, master.Name, "Failure") On 2016/10/16 23:45:36, dsansome wrote: > Does master.Name have a "master." prefix? If not it might be difficult to join > with other metrics. It does not, I assume it'd be better to normalize it by pre-pending "master."?
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Win Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/31ed3129c4d2dc10)
The CQ bit was checked by hinoka@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Win Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/31eda62d1931c610)
The CQ bit was checked by hinoka@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/326dd172b4330c10) Luci-go Mac Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/326dd17158fc0610) Luci-go Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/326dd170d5e09a10)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/327da5dfa573d410) Luci-go Mac Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/327da5dd54fa4710)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hinoka@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dsansome@chromium.org Link to the patchset: https://codereview.chromium.org/2418063002/#ps80001 (title: "fix")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/329257cf97742710)
The CQ bit was checked by hinoka@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Milo: Add ts_mon metrics for master json datastore success BUG=655863 ========== to ========== Milo: Add ts_mon metrics for master json datastore success BUG=655863 Committed: https://github.com/luci/luci-go/commit/7d92eccc01a9b20b65de213753701e35658ae57f ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://github.com/luci/luci-go/commit/7d92eccc01a9b20b65de213753701e35658ae57f |
