Log uses of isolate archive and exparchive when 'chromium' build tag is set.

Log uses of isolate archive and exparchive when 'chromium' build tag is set.

This can be disabled by setting eventlog-endpoint to none. The purpose
of this change is to facilitate tracking down uses of isolate
{,batch_}archive so that they can be migrated to exp_archive.

Note: batch_archive is not included in this CL because it does not yet log at all. But it will behave in the same way as {,exp_}archive when logging is added to it in a followup CL.

BUG=727985

[mcgreevy, 2017-06-05 06:16:16]

Description was changed from

==========
Log uses of isolate archive and exparchive when prod isolateserver is used.

This can be disabled by setting eventlog-endpoint to none. The purpose
of this change is to facilitate tracking down uses of isolate
{,batch_}archive so that they can be migrated to exp_archive.

BUG=727985
==========

to

==========
Log uses of isolate archive and exparchive when prod isolateserver is used.

This can be disabled by setting eventlog-endpoint to none. The purpose
of this change is to facilitate tracking down uses of isolate
{,batch_}archive so that they can be migrated to exp_archive.

Note: batch_archive is not included in this CL because it does not yet log at
all. But it will behave in the same way as {,exp_}archive when logging is added
to it in a followup CL.

BUG=727985
==========

[mcgreevy, 2017-06-05 06:16:17]

mcgreevy@chromium.org changed reviewers:
+ maruel@chromium.org, tansell@chromium.org

[mithro, 2017-06-06 00:56:44]

lgtm

[tandrii(chromium), 2017-06-06 07:14:47]

tandrii@chromium.org changed reviewers:
+ tandrii@chromium.org

[tandrii(chromium), 2017-06-06 07:14:48]

https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/comm...
File client/cmd/isolate/common.go (right):

https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/comm...
client/cmd/isolate/common.go:145: f.StringVar(&lf.EventlogEndpoint,
"eventlog-endpoint", "auto", eventlogEndpointHelp)
IMO, this should default to None, while recipe invocation on our bots should
explicitly state auto or maybe even just list prod.

This will be inline with other tooling that developers may directly or
indirectly invoke on their workstations.

[tandrii(chromium), 2017-06-06 07:15:06]

On 2017/06/06 07:14:48, tandrii(chromium) wrote:
>
https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/comm...
> File client/cmd/isolate/common.go (right):
> 
>
https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/comm...
> client/cmd/isolate/common.go:145: f.StringVar(&lf.EventlogEndpoint,
> "eventlog-endpoint", "auto", eventlogEndpointHelp)
> IMO, this should default to None, while recipe invocation on our bots should
> explicitly state auto or maybe even just list prod.
> 
> This will be inline with other tooling that developers may directly or
> indirectly invoke on their workstations.

but other than that, this is really cool change :)

[mcgreevy, 2017-06-06 23:56:39]

On 2017/06/06 07:14:48, tandrii(chromium) wrote:
>
https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/comm...
> File client/cmd/isolate/common.go (right):
> 
>
https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/comm...
> client/cmd/isolate/common.go:145: f.StringVar(&lf.EventlogEndpoint,
> "eventlog-endpoint", "auto", eventlogEndpointHelp)
> IMO, this should default to None, while recipe invocation on our bots should
> explicitly state auto or maybe even just list prod.
> 
> This will be inline with other tooling that developers may directly or
> indirectly invoke on their workstations.

That is what I would normally do, but the point of this change is explicitly to
discover any places that we use these commands that I am not aware of, so that I
can plan their deprecation.  One other option that I considered to limit the
situations in which logging occurs was to only log automatically if all of these
environment variables are set:
BUILDBOT_MASTERNAME, BUILDBOT_BUILDERNAME, BUILDBOT_BUILDNUMBER,
BUILDBOT_SLAVENAME, but I was worried that these might not be set in some cases
where we are using these tools in prod. Do you think I should reintroduce that
restriction?

[mcgreevy, 2017-06-06 23:57:09]

@maruel, would you mind commenting on this CL? I would appreciate your
perspective.

[mithro, 2017-06-07 00:14:54]

On 2017/06/06 23:57:09, mcgreevy wrote:
> @maruel, would you mind commenting on this CL? I would appreciate your
> perspective.

We originally were going to use the environment variables, but decided that if
you set the production endpoint for uploading files we wanted to know about the
upload.

[M-A Ruel, 2017-06-08 14:06:06]

https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/isol...
File client/cmd/isolate/isolate_event_logger.go (right):

https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/isol...
client/cmd/isolate/isolate_event_logger.go:19: // When --eventlog-endpoint=auto,
we decide to send eventlogs based on whether --isolate-server matches
prodIsolateServer
I don't understand why we don't want to log everywhere? Also there's many more
"prod" instances and I'd prefer not have to hardcode them here.

[mcgreevy, 2017-06-08 23:19:01]

https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/isol...
File client/cmd/isolate/isolate_event_logger.go (right):

https://codereview.chromium.org/2922153002/diff/40001/client/cmd/isolate/isol...
client/cmd/isolate/isolate_event_logger.go:19: // When --eventlog-endpoint=auto,
we decide to send eventlogs based on whether --isolate-server matches
prodIsolateServer
On 2017/06/08 14:06:05, M-A Ruel wrote:
> I don't understand why we don't want to log everywhere? Also there's many more
> "prod" instances and I'd prefer not have to hardcode them here.

The reason is that luci-go is an open source project, and I don't want to log
usages of it that are unrelated to the chromium project. Here's another idea,
though: rather than checking which isolateserver is being used, I could switch
logging on via a build tag, and enable that build tag when building the isolate
binary that we upload to GCS. That way the binary that we build for our own
purposes will automatically log, whereas when it's built by anyone else, it
wont.  WDYT?

[mcgreevy, 2017-06-09 01:55:13]

Description was changed from

==========
Log uses of isolate archive and exparchive when prod isolateserver is used.

This can be disabled by setting eventlog-endpoint to none. The purpose
of this change is to facilitate tracking down uses of isolate
{,batch_}archive so that they can be migrated to exp_archive.

Note: batch_archive is not included in this CL because it does not yet log at
all. But it will behave in the same way as {,exp_}archive when logging is added
to it in a followup CL.

BUG=727985
==========

to

==========
Log uses of isolate archive and exparchive when 'chromium' build tag is set.

This can be disabled by setting eventlog-endpoint to none. The purpose
of this change is to facilitate tracking down uses of isolate
{,batch_}archive so that they can be migrated to exp_archive.

Note: batch_archive is not included in this CL because it does not yet log at
all. But it will behave in the same way as {,exp_}archive when logging is added
to it in a followup CL.

BUG=727985
==========

[mcgreevy, 2017-06-09 01:56:53]

OK, I've switched this to use a build tag instead of looking at which isolate
server is being used.

The corresponding recipe change to make use of this is at:
https://chromium-review.googlesource.com/c/527778/