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

Issue 646370 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 646343



Sign in to add a comment

Annotee creating "duplicate stream name" for some nested steps.

Project Member Reported by d...@chromium.org, Sep 13 2016

Issue description

Example error output:

[E2016-09-09T14:33:11.182170-07:00 6096 0 butler.go:319] Failed to add stream.                       {"error":"a stream has already been registered with name \"recipes/steps/ensure_goma.install_cipd/0/stdout\"", "streamServer":"*streamserver.listenerStreamServer(\\\\.\\pipe\\LUCILogDogButler)"}
[E2016-09-09T14:33:11.182170-07:00 5408 0 processor.go:281] Failed to send line to LogDog.              {"error":"The pipe is being closed."}
[E2016-09-09T14:33:11.182170-07:00 5408 0 processor.go:211] Failed to ingest line.                      {"error":"The pipe is being closed.", "line":" name: ensure_goma.install cipd", "stream":"stdout"}
[E2016-09-09T14:33:11.182170-07:00 5408 0 processor.go:281] Failed to send line to LogDog.              {"error":"The pipe is being closed."}
[E2016-09-09T14:33:11.182170-07:00 5408 0 processor.go:211] Failed to ingest line.                      {"error":"The pipe is being closed.", "line":" step_test_data: <lambda>(...)", "stream":"stdout"}

The problem is that Butler is (rightfully) rejecting Annotee's attempts to create multiple streams with the same name, which Annotee should not be doing. This likely changed when nested step support was added to Annotee.
 

Comment 1 by d...@chromium.org, Sep 13 2016

Blocking: 646343
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/ee7b181c8d08b2b518be4659421a03f7a2afacce

commit ee7b181c8d08b2b518be4659421a03f7a2afacce
Author: dnj <dnj@chromium.org>
Date: Tue Sep 13 21:05:04 2016

Fix Annotee stream name generation.

Annotee was encountering two major problems related to nesting. Nesting
is, unfortunately, implemented as two independent annotation events, one
to select/seed the step and one to change its nest level from the
default, 0.

The first problem arose when Annotee is configured to echo annotations.
It will be created as "A", a root step, and echo that annotation to a
stream named after "A". Then its nest level will change, and it will
echo the nest level annotation to "B", its new name. If "A" equals "B",
we get a stream name conflict. Also, the "A" stream is totally junk.

To address this, we now write ALL annotations to the root stream, not the
stream in which they occurred. This does not affect the tee'd output, which
will still receive the annotations in the order in which they are emitted.

The second problem is that the "canonical name" was implemented very
lazily (originally for logging), but used for real things. This caused any
number of opportunities where two steps could have the same canonical name.
We remove this altogether, tracking steps directly and using their log name
base as a disambiguating factor. This means that even if two steps with the
same name exist, they will be given differnt log names.

BUG= chromium:646370 
TEST=local

Review-Url: https://codereview.chromium.org/2328023003

[modify] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/annotation.go
[modify] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/annotation_test.go
[modify] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_data/coverage.annotations.txt
[modify] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_data/nested.annotations.txt
[rename] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/coverage_base.proto.txt
[add] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/coverage_base_steps_baz_qux_0_logs_content_0.txt
[rename] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/coverage_base_steps_foo_0_steps_bar_0_logs_logging_json_0.txt
[rename] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/coverage_base_steps_foo_0_steps_bar_0_logs_lorem_txt_0.txt
[rename] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/default_base.proto.txt
[rename] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/nested_base.proto.txt
[add] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/nested_base_steps_nesting_parent_0_steps_nesting_child0_0_steps_grandchild_0_logs_content_0.txt
[rename] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/annotation/test_expectations/timestamps_base.proto.txt
[modify] https://crrev.com/ee7b181c8d08b2b518be4659421a03f7a2afacce/logdog/client/annotee/processor.go

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 13 2016

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

commit 0fc860e7abeb3c4f71962278cd72507da613f53f
Author: Dan Jacques <dnj@chromium.org>
Date: Tue Sep 13 21:24:27 2016

Roll luci-go.

infra/go/src/github.com/luci/luci-go:
ee7b181 Fix Annotee stream name generation.
cba650f scheduler: Fetch configs more frequently.
3e45a26 Rename 'cron' to 'scheduler'.
09de807 Milo: Show hidden tweaks
659a305 LogDog/Web: Write text content directly.
118ceeb LogDog Web: Use "div" instead of table view.
bff2bc7 LogDog/Web: Fix query streaming.
6676c8e Create log container elements in JavaScript for faster loading
e1a482a LogDog/Archivist: Conditionally render data.
dfbbe36 Allow COUNTER_WRITER ACLs to be listed and edited.
f8c0913 Milo: Add task expired as a failure status
0c06a57 LogDog: Remove use of AckID.

TBR=iannucci@chromium.org
BUG= chromium:646370 
TEST=None

Change-Id: I5262310efc1bfdeac9d0e75c9d9f16c29518037e
Reviewed-on: https://chromium-review.googlesource.com/385157
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Daniel Jacques <dnj@chromium.org>

[modify] https://crrev.com/0fc860e7abeb3c4f71962278cd72507da613f53f/DEPS

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 13 2016

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

commit 0fc860e7abeb3c4f71962278cd72507da613f53f
Author: Dan Jacques <dnj@chromium.org>
Date: Tue Sep 13 21:24:27 2016

Roll luci-go.

infra/go/src/github.com/luci/luci-go:
ee7b181 Fix Annotee stream name generation.
cba650f scheduler: Fetch configs more frequently.
3e45a26 Rename 'cron' to 'scheduler'.
09de807 Milo: Show hidden tweaks
659a305 LogDog/Web: Write text content directly.
118ceeb LogDog Web: Use "div" instead of table view.
bff2bc7 LogDog/Web: Fix query streaming.
6676c8e Create log container elements in JavaScript for faster loading
e1a482a LogDog/Archivist: Conditionally render data.
dfbbe36 Allow COUNTER_WRITER ACLs to be listed and edited.
f8c0913 Milo: Add task expired as a failure status
0c06a57 LogDog: Remove use of AckID.

TBR=iannucci@chromium.org
BUG= chromium:646370 
TEST=None

Change-Id: I5262310efc1bfdeac9d0e75c9d9f16c29518037e
Reviewed-on: https://chromium-review.googlesource.com/385157
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Daniel Jacques <dnj@chromium.org>

[modify] https://crrev.com/0fc860e7abeb3c4f71962278cd72507da613f53f/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 13 2016

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

commit 0fc860e7abeb3c4f71962278cd72507da613f53f
Author: Dan Jacques <dnj@chromium.org>
Date: Tue Sep 13 21:24:27 2016

Roll luci-go.

infra/go/src/github.com/luci/luci-go:
ee7b181 Fix Annotee stream name generation.
cba650f scheduler: Fetch configs more frequently.
3e45a26 Rename 'cron' to 'scheduler'.
09de807 Milo: Show hidden tweaks
659a305 LogDog/Web: Write text content directly.
118ceeb LogDog Web: Use "div" instead of table view.
bff2bc7 LogDog/Web: Fix query streaming.
6676c8e Create log container elements in JavaScript for faster loading
e1a482a LogDog/Archivist: Conditionally render data.
dfbbe36 Allow COUNTER_WRITER ACLs to be listed and edited.
f8c0913 Milo: Add task expired as a failure status
0c06a57 LogDog: Remove use of AckID.

TBR=iannucci@chromium.org
BUG= chromium:646370 
TEST=None

Change-Id: I5262310efc1bfdeac9d0e75c9d9f16c29518037e
Reviewed-on: https://chromium-review.googlesource.com/385157
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Daniel Jacques <dnj@chromium.org>

[modify] https://crrev.com/0fc860e7abeb3c4f71962278cd72507da613f53f/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 14 2016

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 14 2016

Comment 8 by d...@chromium.org, Sep 14 2016

Status: Fixed (was: Untriaged)
This is believe to be fixed.
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 14 2016

Project Member

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

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

commit 2e4f26a02858c01ad39da12ea0a675f4013783d7
Author: recipe-roller <recipe-roller@chromium.org>
Date: Fri Sep 16 21:50:47 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/0fc6b2e65ce4651c7b70239c4b7cc36489fa0c97 Use api.chromium.compile() in client.nacl.sdk.recipe_autogen.py (sbc@chromium.org)
  https://crrev.com/f683608de4fe82d85451dff456b9228bab7e4090 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/6fd7041208f42a8c4592d701d79d11a943327790 chromium.android: Enable swarming on Marshmallow 64 bit Tester (bpastene@chromium.org)
  https://crrev.com/b46e0087c2dbf12ecd9e3a1452e781d6d6153e9c Cleanup NaCl SDK recipes (sbc@chromium.org)
  https://crrev.com/15faceb9c15002050acc274c7773cb8537b91706 Merge recipes in client.nacl.sdk.recipe_autogen (sbc@chromium.org)
  https://crrev.com/c3e75a171aae1f581d8664044aa3e0ded2f40800 LogDog: bump canary BuildBot CIPD pin. (dnj@chromium.org)
  https://crrev.com/50d0e85c51db4fa62b1b5bb4e0b8529027edb620 Enable LogDog on chromium.gatekeeper. (dnj@chromium.org)
  https://crrev.com/5d9a4bb87b9ee698554ac2a76c77eec5ee0ba074 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/6e947e576ddf9e45034fbc2b2c4275f3bad252d3 Bump LogDog pin for fleet. (dnj@chromium.org)
  https://crrev.com/efcfa093c953d2e690e6c1493def054f8ad00bea When retrying a failing swarming gtest without the patch, don't pass on the original --test-launcher-filter-file argument. (jam@chromium.org)
  https://crrev.com/ac3894c8293b42e21b891722965f982118bf6a5a libyuv: Add Android Testers (kjellander@chromium.org)
  https://crrev.com/741d9d458adf84901bbdfecb492276b98201c29a V8: Fix gn args for non-MB arm builders (machenbach@chromium.org)
  https://crrev.com/9c621ef2f19500a63fd2503eb9f73a71443563a0 adding attributes to builders that build chromium for swarming bots. (eyaich@chromium.org)
  https://crrev.com/6d930dea23eeab6d0d2b30e71e4b4b84ff01c70d Disable goma on 'ThinLTO Linux ToT'. (krasin@chromium.org)
  https://crrev.com/470dc77a00270b6227facfdc781a468efb538769 Enable LogDog for dart. (dnj@chromium.org)
  https://crrev.com/9ef12eb44ae227823398dc0604213fadae4e1655 Enable the ninja up-to-date check for Android builders (agrieve@chromium.org)
  https://crrev.com/a30a2b57591752fd9b05589209a88b20d7114248 Windows 64-bit builder for chromium.perf.fyi. (dtu@chromium.org)
  https://crrev.com/c97a211b5b7815d9d601760f6119a3156c7e458d Revert of Enable the ninja up-to-date check for Android builders (patchset #1 id:1 of https://codereview.chromium.org/2338203004/ ) (agrieve@chromium.org)
depot_tools:
  https://crrev.com/65cc5b1918d61bbb7a26c78364414fdf264198fe Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/33061f78a10a7e44537a4c0384c5dab68d61f1ec Add recipe-roller as an OWNER of recipe modules. (martiniss@chromium.org)
recipe_engine:
  https://crrev.com/ca75ca80dcfcc2a2dc38096ba0522065ebd347ec Revert of Require recipe tryjob for CQ. (patchset #1 id:1 of https://codereview.chromium.org/2153303002/ ) (martiniss@chromium.org)

TBR=martiniss@chromium.org,phajdan.jr@chromium.org
BUG= chromium:474921 ,646165, chromium:646370 , 643144 , 645295 , 628801 , chromium:643226 ,chromium:633253, 646185 , 587527 ,chromium:646343,589180,644212,644609,633253

Recipe-Tryjob-Bypass-Reason: Autoroller
Bugdroid-Send-Email: False
Review-Url: https://codereview.chromium.org/2345303003

[modify] https://crrev.com/2e4f26a02858c01ad39da12ea0a675f4013783d7/infra/config/recipes.cfg

Sign in to add a comment