Milo: Add ts_mon metrics for master json datastore success

milo/appengine/buildbot/pubsub.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package buildbot
 
 import (
         "bytes"
         "compress/gzip"
         "compress/zlib"
@@ skipping 24 matching lines @@
         buildCounter = metric.NewCounter(
                 "luci/milo/buildbot_pubsub/builds",
                 "The number of buildbot builds received by Milo from PubSub",
                 nil,
                 field.Bool("internal"),
                 field.String("master"),
                 field.String("builder"),
                 field.Bool("finished"),
                 // Status can be one of 3 options.  "New", "Replaced", "Rejected".
                 field.String("status"))
+
+        masterCounter = metric.NewCounter(
+                "luci/milo/buildbot_pubsub/masters",
+                "The number of buildbot master jsons received by Milo from PubSub",
+                nil,
+                field.Bool("internal"),
+                field.String("master"),
+                // Status can be one of 2 options.  "Success", "Failure".

[dsansome, 2016/10/16 23:45:36]

Convention is for these field values to be lower-case: "success", "failure"

[hinoka, 2016/10/17 21:35:13]

Done.

+                field.String("status"))
 )
 
 type pubSubMessage struct {
         Attributes map[string]string `json:"attributes"`
         Data       string            `json:"data"`
         MessageID  string            `json:"message_id"`
 }
 
 type pubSubSubscription struct {
         Message      pubSubMessage `json:"message"`
@@ skipping 73 matching lines @@
                                 bm.Builds = append(bm.Builds, build)
                                 slave.RunningbuildsMap[build.Buildername] = append(
                                         slave.RunningbuildsMap[build.Buildername], build.Number)
                         }
                         slave.Runningbuilds = nil
                 }
         }
         return bm.Builds, bm.Master, nil
 }
 
 // getOSInfo fetches the os family and version of hte build from the

[dsansome, 2016/10/16 23:45:36]

And should it be "of the slave" rather than "of the build"?

[dsansome, 2016/10/16 23:45:36]

While I'm here, s/hte/the/

[hinoka, 2016/10/17 21:35:13]

Done.

 // master json on a best-effort basis.
 func getOSInfo(c context.Context, b *buildbotBuild, m *buildbotMaster) (
         family, version string) {
         // Fetch the master info from datastore if not provided.
         if m.Name == "" {
                 logging.Infof(c, "Fetching info for master %s", b.Master)
                 entry := buildbotMasterEntry{Name: b.Master}
                 err := ds.Get(c, &entry)
                 if err != nil {
                         logging.WithError(err).Errorf(
@@ skipping 138 matching lines @@
                         buildCounter.Add(
                                 c, 1, false, build.Master, build.Buildername, build.Finished, "New")
                 }
 
         }
         if master != nil {
                 err = putDSMasterJSON(c, master, internal)
                 if err != nil {
                         logging.WithError(err).Errorf(
                                 c, "Could not save master in datastore %s", err)
+                        masterCounter.Add(c, 1, internal, master.Name, "Failure")

[dsansome, 2016/10/16 23:45:36]

Does master.Name have a "master." prefix?  If not it might be difficult to join
with other metrics.

[hinoka, 2016/10/17 21:35:13]

It does not, I assume it'd be better to normalize it by pre-pending "master."?

                         // This is transient, we do want PubSub to retry.
                         h.WriteHeader(500)
                         return
                 }
+                masterCounter.Add(c, 1, internal, master.Name, "Success")
         }
         h.WriteHeader(200)
 }