Ned and Emily,
Does this plan seem reasonable to you? Think it's safe to remove json_output_formatter.py? Or would this be a send an email to telemetry-announce now and wait two weeks before doing that step?
Thanks!

We should always send email to telemetry-announce@ upon changing API (even when we are 99% sure that no one use that API).

+1 from me for removing json_output_formatter.py You will want to talk to people who maintain https://cs.chromium.org/chromium/src/tools/perf/run_and_compare_benchmarks as well since this script depends on json output format (contacts are mgiuca@, benwells@)

Ethan: FYI

+1 to removing json_output_formatter, but be aware that --output-format=json has been propagated to folks as the hack to use to get Telemetry to output traces for each story run. This is just an artifact of the way json_output_formatter works (it treats TraceValues specially), and probably should be replaced by an appropriate, more purposeful flag.

What do we plan to tell people to do instead, if they want Telemetry to emit traces?

So far it sounds like steps 1-3 are okay to do but 4-6 are going to hit some snags. I'll reach out to mguica@ and benwells@ about run_and_compare_benchmarks about step 4.

With regards to steps 5 & 6 and generating traces, interesting point! I did not know that the flag was being used for that. So we should either keep the json flag around, or better yet, create a flag specifically for generating trace values. Or we can generate the trace files as a part of the updated json-test-results flag but it would be nice to have things cleaned up and make sense. I'll ping you to chat about this, Ethan!

Okay so plan w.r.t. tracing is to leave json_output_formatter.py for now. I'm gonna work to come up with a plan to refactor the tracing output in Q4 so we remove hacks instead of replacing them with more hacks. For now, I'm going to focus on steps 1-4. (yay for overall code cleanup planning!)

Thanks Ashley! You always amaze me by how fast you detect these architecture issues & opportunities for making things better.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/d3a7a573c7f6f881ac2354ea3fa990616a7d68b0

commit d3a7a573c7f6f881ac2354ea3fa990616a7d68b0
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Wed Jul 26 20:24:21 2017

Pass through upload_test_results parameter for swarming

SwarmingIsolatedScriptTest has an optional parameter
upload_test_results. By passing this param from the swarming config, we
allow Telemetry (and other users of generate_isolated_script) to
delineate which test results should be uploaded to the server. We
default this to True if not defined since that is the current default
in SwarmingIsolatedScriptTest.__init__.

BUG=chromium:748638

Change-Id: Ia8fa52a3d9bf2f72a65d28ae383953775305b579
Reviewed-on: https://chromium-review.googlesource.com/586972
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/d3a7a573c7f6f881ac2354ea3fa990616a7d68b0/scripts/slave/recipe_modules/chromium_tests/steps.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/88b7285575ddbe56cc1f35f2408d6cdffbad7519

commit 88b7285575ddbe56cc1f35f2408d6cdffbad7519
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Thu Jul 27 14:08:43 2017

Only upload test results for specific benchmarks

BUG=chromium:748638
BUG= chromium:717394 

NOTRY=true

Change-Id: I881e4e33d1803837ed373b863ae6a0c0d33762c0
Reviewed-on: https://chromium-review.googlesource.com/587092
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#489913}
[modify] https://crrev.com/88b7285575ddbe56cc1f35f2408d6cdffbad7519/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/88b7285575ddbe56cc1f35f2408d6cdffbad7519/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/88b7285575ddbe56cc1f35f2408d6cdffbad7519/tools/perf/core/perf_data_generator.py
[modify] https://crrev.com/88b7285575ddbe56cc1f35f2408d6cdffbad7519/tools/perf/core/perf_data_generator_unittest.py

Thanks for your supportive words Ned! :)

Steps 1-3 are submitted at this point
Step 4 remains
Step 5-6 will be punted to Q4

Wow so exciting that you are cleaning up some of this!  

Full disclosure, even before leave I wasn't as familiar with the flakiness dashboard efforts.  The only thing I can think of that might be unrelated, but worth a look is the runtest.py script that seems to be run by everyone and everything: https://cs.chromium.org/chromium/build/scripts/slave/runtest.py. 

I am looping martiniss@ in on this because he might know better if this is used in our perf workflow anymore.  Since everything is swarmed, I *think* we bypass most of this now, but I vaguely remember this script relying on the basic success/failure json and I am not sure if the new format would break that.  Again, for perf purposes we shouldn't be using this anymore, but I think there are more consumers of this script.  

For Step 4, looks like run_and_compare_benchmarks actually depends on chartjson output and html output, not json output
https://cs.chromium.org/chromium/src/tools/perf/run_and_compare_benchmarks?l=46
So we should actually be good to go through with step 4 from that perspective

#14: good catch! So I think you're right & we can make an announcement now about deprecating json results . Somehow I misremember this.

Thanks Emily! If that script depends on the basic success/failure json, then these changes could break it. I'll definitely spend some time reading through it today :)

I'm looking at slave/runtest.py and am only seeing references to gtestjson and chartjson, not the simplified format I'm trying to remove in Q4. It's definitely possible I'm missing it though since this file is 1778 lines long :/
At the very least, it doesn't seem like that will affect step 4

Thanks for checking, you are probably right!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4147da786b8ecb0c27e539cfccae51aade36bb0c

commit 4147da786b8ecb0c27e539cfccae51aade36bb0c
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Tue Aug 01 14:18:45 2017

Adding --output-format=json-test-results for all Telemetry benchmarks

Now that all the benchmarks are set not to upload results until their
individual flags are flipped, we can change all the benchmarks to output
json of the json-test-results format instead of the old simplified json
format.

BUG=chromium:748638

Change-Id: I10ad5029f29052aec44a0476fe17db74215f4786
Reviewed-on: https://chromium-review.googlesource.com/589820
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490988}
[modify] https://crrev.com/4147da786b8ecb0c27e539cfccae51aade36bb0c/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/4147da786b8ecb0c27e539cfccae51aade36bb0c/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/4147da786b8ecb0c27e539cfccae51aade36bb0c/testing/scripts/run_telemetry_benchmark_as_googletest.py
[modify] https://crrev.com/4147da786b8ecb0c27e539cfccae51aade36bb0c/tools/perf/core/perf_data_generator.py
[modify] https://crrev.com/4147da786b8ecb0c27e539cfccae51aade36bb0c/tools/perf/core/perf_data_generator_unittest.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8148f5fdc47e7f1a4c62c4df85770d1ec2594896

commit 8148f5fdc47e7f1a4c62c4df85770d1ec2594896
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Wed Aug 02 03:08:57 2017

Updating run_telemetry_benchmark_as_googletest to resolve crash.

Previously, run_telemetry_benchmark_as_googletest would check the
validity of the benchmark run by opening results.json. This file
was generated using the --output-format=json flag. By removing
that flag in https://chromium-review.googlesource.com/c/589820,
I broke all the Telemetry benchmarks on the perf waterfall o.O my
bad. This change should fix the failing tests by checking the
validity of the benchmark run by opening test-results.json,
generated using the --output-format=json-test-results flag now
being used for all Telemetry benchmark runs on the waterfall.

BUG= 751346 , 748638

NOTRY=true  # This script is not run on trybot/CQ, hence should be safe to notry

Change-Id: I75e0abb5801f1cbaa480e65ade1827335a1195fd
Reviewed-on: https://chromium-review.googlesource.com/597428
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#491242}
[modify] https://crrev.com/8148f5fdc47e7f1a4c62c4df85770d1ec2594896/testing/scripts/run_telemetry_benchmark_as_googletest.py