milo: Enable Swarming LogDog log loading.

milo: Enable Swarming LogDog log loading.

Enable Swarming builds to load directly from Milo annotation streams
when the build contains a LogDog "log_location" tag.

To do this, Swarming parses the tags when it fetches a task result. If
the tags include a "log_location" tag, and that location is LogDog, it will
terminate any log fetching (if applicable) and, rather than parse
Swarming logs through Annotee, load the annotation protobuf instead.

To enable this, we introduce the concept of a LogDog URL:
logdog://host/project/prefix/+/name

This URL is handled in the LogDog "types" package.

BUG=chromium:692220
TEST=unit

Review-Url: https://codereview.chromium.org/2695383002
Committed: https://github.com/luci/luci-go/commit/0118a73d6c8f7cb94001978d89424a82b0b37d37

[dnj, 2017-02-16 00:50:17]

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

[dnj, 2017-02-16 00:50:18]

PTAL

[hinoka, 2017-02-16 02:08:21]

lgtm % comments and names

https://codereview.chromium.org/2695383002/diff/1/logdog/common/types/url.go
File logdog/common/types/url.go (right):

https://codereview.chromium.org/2695383002/diff/1/logdog/common/types/url.go#...
logdog/common/types/url.go:18: type Stream struct {
StreamAddr

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
File milo/appengine/swarming/build.go (right):

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:121: // the swarmingFetch of the fetch result
(if enabled). It will contain the
invoked after fetching the swarm fetchRes call?

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:126: taskTagCallback func(map[string]string)
bool
This is funky, but I don't have any better ideas :)

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:471: type buildLoader struct {
Comment on why this exists (testing)

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:479: func (bl *buildLoader)
newAnnotationStream(c context.Context, stream *types.Stream)
(*logdog.AnnotationStream, error) {
newEmptyAnnotationStream might be more appropriate?  To make it clear that it's
not a populated one.

At some point we probably want to go through and do a refactor and rename things
to make things easier to read.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:509: // Cancel if LogDog annotation strem
parameters are present in the tag set.
stream

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:528: // If the LogDog stream is available, load
the step from that.
Add to comment: Explanation on why we're preferring one over the other.  ie
tasks with log_location will probably not contain annotated swarming logs.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:536: if s, err = as.Load(c); err != nil {
Fetch seems like a more appropriate name

[dnj, 2017-02-16 02:20:52]

https://codereview.chromium.org/2695383002/diff/1/logdog/common/types/url.go
File logdog/common/types/url.go (right):

https://codereview.chromium.org/2695383002/diff/1/logdog/common/types/url.go#...
logdog/common/types/url.go:18: type Stream struct {
On 2017/02/16 02:08:21, hinoka wrote:
> StreamAddr

Done.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
File milo/appengine/swarming/build.go (right):

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:121: // the swarmingFetch of the fetch result
(if enabled). It will contain the
On 2017/02/16 02:08:21, hinoka wrote:
> invoked after fetching the swarm fetchRes call?

Done.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:126: taskTagCallback func(map[string]string)
bool
On 2017/02/16 02:08:21, hinoka wrote:
> This is funky, but I don't have any better ideas :)

Acknowledged.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:471: type buildLoader struct {
On 2017/02/16 02:08:21, hinoka wrote:
> Comment on why this exists (testing)

Done.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:479: func (bl *buildLoader)
newAnnotationStream(c context.Context, stream *types.Stream)
(*logdog.AnnotationStream, error) {
On 2017/02/16 02:08:21, hinoka wrote:
> newEmptyAnnotationStream might be more appropriate?  To make it clear that
it's
> not a populated one.
> 
> At some point we probably want to go through and do a refactor and rename
things
> to make things easier to read.

Done.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:509: // Cancel if LogDog annotation strem
parameters are present in the tag set.
On 2017/02/16 02:08:21, hinoka wrote:
> stream

Done.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:528: // If the LogDog stream is available, load
the step from that.
On 2017/02/16 02:08:21, hinoka wrote:
> Add to comment: Explanation on why we're preferring one over the other.  ie
> tasks with log_location will probably not contain annotated swarming logs.

Done.

https://codereview.chromium.org/2695383002/diff/1/milo/appengine/swarming/bui...
milo/appengine/swarming/build.go:536: if s, err = as.Load(c); err != nil {
On 2017/02/16 02:08:21, hinoka wrote:
> Fetch seems like a more appropriate name

Done.

[dnj, 2017-02-16 02:22:05]

The CQ bit was checked by dnj@chromium.org

[dnj, 2017-02-16 02:22:06]

The patchset sent to the CQ was uploaded after l-g-t-m from hinoka@chromium.org
Link to the patchset: https://codereview.chromium.org/2695383002/#ps20001
(title: "Comments, fix links.")

[commit-bot: I haz the power, 2017-02-16 02:22:17]

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-16 03:04:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487211725726140,
"parent_rev": "ddca4ffe013b628ca436a6c7a3df7d565ba2e8b0", "commit_rev":
"0118a73d6c8f7cb94001978d89424a82b0b37d37"}

[commit-bot: I haz the power, 2017-02-16 03:04:06]

Description was changed from

==========
milo: Enable Swarming LogDog log loading.

Enable Swarming builds to load directly from Milo annotation streams
when the build contains a LogDog "log_location" tag.

To do this, Swarming parses the tags when it fetches a task result. If
the tags include a "log_location" tag, and that location is LogDog, it will
terminate any log fetching (if applicable) and, rather than parse
Swarming logs through Annotee, load the annotation protobuf instead.

To enable this, we introduce the concept of a LogDog URL:
logdog://host/project/prefix/+/name

This URL is handled in the LogDog "types" package.

BUG=chromium:692220
TEST=unit
==========

to

==========
milo: Enable Swarming LogDog log loading.

Enable Swarming builds to load directly from Milo annotation streams
when the build contains a LogDog "log_location" tag.

To do this, Swarming parses the tags when it fetches a task result. If
the tags include a "log_location" tag, and that location is LogDog, it will
terminate any log fetching (if applicable) and, rather than parse
Swarming logs through Annotee, load the annotation protobuf instead.

To enable this, we introduce the concept of a LogDog URL:
logdog://host/project/prefix/+/name

This URL is handled in the LogDog "types" package.

BUG=chromium:692220
TEST=unit

Review-Url: https://codereview.chromium.org/2695383002
Committed:
https://github.com/luci/luci-go/commit/0118a73d6c8f7cb94001978d89424a82b0b37d37
==========

[commit-bot: I haz the power, 2017-02-16 03:04:07]

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