milo: Add BuildInfo implementation for BuildBot.

milo: Add BuildInfo implementation for BuildBot.

This adds a BuildInfo endpoint implementation for BuildBot builds. These
builds will load the Milo annotation protobuf from LogDog, merge it with
the BuildBot build information, and export that via pRPC endpoint.

BUG=chromium:687625
TEST=unit

Review-Url: https://codereview.chromium.org/2674513003
Committed: https://github.com/luci/luci-go/commit/cf475ef7566a1fb123eae4993888614c6a121532

[dnj, 2017-02-02 02:57:33]

dnj@chromium.org changed reviewers:
+ hinoka@chromium.org

[dnj, 2017-02-02 02:57:33]

PTAL. This set of CLs adds a pRPC service to Milo called "BuildInfo". This
service offers a one-stop shop for users to query Milo about a specific build,
referencing it using familiar terms, and receive canonical build data (mostly
LogDog annotation protobufs) and metadata. It is envisioned as an entry point
for services that wish to learn about builds or view their logs.

CL chain:
https://codereview.chromium.org/2671633002 - (prep) Export host in LogDog
client.
https://codereview.chromium.org/2674603002 - (prep) Milo logdog module exports
AnnotationStream loading.
https://codereview.chromium.org/2675493003 - (prep) Refactor Swarming a bit, add
parallel fetcher.
https://codereview.chromium.org/2667353002 - Barebones (Unimplemented) BuildInfo
service addition.
https://codereview.chromium.org/2676503002 - (prep) Use strict for BuildBot
properties instead of []interface{}
https://codereview.chromium.org/2674513003 - BuildInfo implementation for
BuildBot builds.
https://codereview.chromium.org/2667363002 - BuildInfo implementation for
Swarming Kitchen builds.

[hinoka, 2017-02-03 02:16:44]

code lg, comments mostly revolve around readability.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
File milo/appengine/buildbot/buildinfo.go (right):

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:47: // GetBuildInfo resolves a Milo
protobuf Step for a given BuildBot build.
Add to comment:
1. Fetches the buildbot build json from datastore
2. Fetches the logdog stream from the logdog server.
3. Extract the milo step proto from the logdog stream
4. Merge some information from the buildbot build json (what info? Just
properties?) into the final milo step proto.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:80: return nil, err
We should resolve 404s and give a better error message if Milo knows about a
build but Logdog doesn't.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:122: func getLogDogAnnotations(c
context.Context, client *coordinator.Client, build *buildbotBuild, projectHint
cfgtypes.ProjectName) (
Try to fit this in under 100char

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:139: if projectHint == "" {
I thought this was optional, will buildbot fetches fail if this isn't provided?

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:173: logging.Debugf(c, "Resolved
path/project via tag query.")
Resolved path/project into %s .....

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:207: func mergeBuildIntoAnnotation(c
context.Context, step *miloProto.Step, build *buildbotBuild) error {
Add comment.  This looks like it just extract build properties from the buildbot
build struct and adds it to the step.  Given the function name I expected it to
do more.

[dnj, 2017-02-03 23:53:39]

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
File milo/appengine/buildbot/buildinfo.go (right):

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:80: return nil, err
On 2017/02/03 02:16:44, hinoka wrote:
> We should resolve 404s and give a better error message if Milo knows about a
> build but Logdog doesn't.

This here will error if, given a build, we can't figure out the LogDog
annotation proto. I'll accompany the failures with some more info, though.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:122: func getLogDogAnnotations(c
context.Context, client *coordinator.Client, build *buildbotBuild, projectHint
cfgtypes.ProjectName) (
On 2017/02/03 02:16:44, hinoka wrote:
> Try to fit this in under 100char

Done.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:139: if projectHint == "" {
On 2017/02/03 02:16:44, hinoka wrote:
> I thought this was optional, will buildbot fetches fail if this isn't
provided?

It is optional, but some resolution paths require a hint to succeed.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:173: logging.Debugf(c, "Resolved
path/project via tag query.")
On 2017/02/03 02:16:44, hinoka wrote:
> Resolved path/project into %s .....

The calling site for getLogDogAnnotations follows up a successful return value
with this information.

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:207: func mergeBuildIntoAnnotation(c
context.Context, step *miloProto.Step, build *buildbotBuild) error {
On 2017/02/03 02:16:44, hinoka wrote:
> Add comment.  This looks like it just extract build properties from the
buildbot
> build struct and adds it to the step.  Given the function name I expected it
to
> do more.

I was thinking that this might be expanded in the future if there is additional
data that is identified that we want to merge; the function name reflects this.

I updated the comment to be more detailed.

[dnj, 2017-02-03 23:54:04]

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
File milo/appengine/buildbot/buildinfo.go (right):

https://codereview.chromium.org/2674513003/diff/1/milo/appengine/buildbot/bui...
milo/appengine/buildbot/buildinfo.go:47: // GetBuildInfo resolves a Milo
protobuf Step for a given BuildBot build.
On 2017/02/03 02:16:44, hinoka wrote:
> Add to comment:
> 1. Fetches the buildbot build json from datastore
> 2. Fetches the logdog stream from the logdog server.
> 3. Extract the milo step proto from the logdog stream
> 4. Merge some information from the buildbot build json (what info? Just
> properties?) into the final milo step proto.

Done.

[hinoka, 2017-02-03 23:56:13]

lgtm

[dnj, 2017-02-03 23:59:38]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2017-02-03 23:59:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-04 00:06:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486166378459280,
"parent_rev": "96163a8561d90a9784ae79e12683d7440a457f04", "commit_rev":
"cf475ef7566a1fb123eae4993888614c6a121532"}

[commit-bot: I haz the power, 2017-02-04 00:06:49]

Description was changed from

==========
milo: Add BuildInfo implementation for BuildBot.

This adds a BuildInfo endpoint implementation for BuildBot builds. These
builds will load the Milo annotation protobuf from LogDog, merge it with
the BuildBot build information, and export that via pRPC endpoint.

BUG=chromium:687625
TEST=unit
==========

to

==========
milo: Add BuildInfo implementation for BuildBot.

This adds a BuildInfo endpoint implementation for BuildBot builds. These
builds will load the Milo annotation protobuf from LogDog, merge it with
the BuildBot build information, and export that via pRPC endpoint.

BUG=chromium:687625
TEST=unit

Review-Url: https://codereview.chromium.org/2674513003
Committed:
https://github.com/luci/luci-go/commit/cf475ef7566a1fb123eae4993888614c6a121532
==========

[commit-bot: I haz the power, 2017-02-04 00:06:53]

Committed patchset #2 (id:20001) as
https://github.com/luci/luci-go/commit/cf475ef7566a1fb123eae4993888614c6a121532