New issue
Advanced search Search tips

Issue 911660 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

isolate server: migrate to pRPC

Project Member Reported by maruel@google.com, Dec 4

Issue description

I'll start with adding the stats retriever with it, and if it pans out, we can start redoing everything, including the .isolated file format as proto.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/7b83a13ada570fdbed158f1cfdf9416805e20290

commit 7b83a13ada570fdbed158f1cfdf9416805e20290
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Wed Dec 05 13:51:12 2018

[isolate] Add new prpc service to retrieve statistics

- This adds back a way to extract statistics from the Isolated server. This had
  to be removed years ago due to security issues.
- This should enable re-creating a Web UI for the statistics dashboard.
- I decided to use prpc to see if it can be used fine in case I want to switch
  the Swarming server to prpc. Using protos is generally better for readability
  for users than 'protorpc', which is completely unmaintained. I decided to do
  it here because the code is very simple, yet the same flow could be used for
  Swarming Web UI.
- Tweak stats_framework to be nicer to use.

Bug: 911660

Change-Id: Ib9cc6d5e547430c34577379999d1150c4d7db585
Reviewed-on: https://chromium-review.googlesource.com/c/1360132
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/components/components/stats_framework/__init__.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/README.md
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/app.yaml
[add] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/handlers_prpc.py
[add] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/handlers_prpc_test.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/handlers_test.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/main_frontend.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/proto/__init__.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/proto/config_pb2.py
[add] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/proto/isolated.proto
[add] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/proto/isolated_pb2.py
[add] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/proto/isolated_prpc_pb2.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/stats.py
[modify] https://crrev.com/7b83a13ada570fdbed158f1cfdf9416805e20290/appengine/isolate/stats_test.py

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/3300f819dc653f519c90faef9ba9d14e5a027e71

commit 3300f819dc653f519c90faef9ba9d14e5a027e71
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Wed Dec 05 15:51:19 2018

[components] Fix regression in 7b83a13ada570fdbed158f1cfdf9416805e20290

I missed three calls when I switched to the timestamp property.

TBR=qyearsley@chromium.org
Bug: 911660
Change-Id: I107a7c7e570e7e3e993303825fd1daaebdbdc274
Reviewed-on: https://chromium-review.googlesource.com/c/1363530
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/3300f819dc653f519c90faef9ba9d14e5a027e71/appengine/components/components/stats_framework/__init__.py

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 11

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/51735cb506e641790b97161873ec2ca3b56b9b90

commit 51735cb506e641790b97161873ec2ca3b56b9b90
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Tue Dec 11 17:44:44 2018

[isolate] call out that I know the API needs to be completed

So it doesn't look like an half-hearted attempt.

R=qyearsley@chromium.org

Bug: 911660
Change-Id: I7775814bd8b230798c8883d77d0993181c824d15
Reviewed-on: https://chromium-review.googlesource.com/c/1372144
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>

[modify] https://crrev.com/51735cb506e641790b97161873ec2ca3b56b9b90/appengine/isolate/proto/isolated.proto
[modify] https://crrev.com/51735cb506e641790b97161873ec2ca3b56b9b90/appengine/isolate/proto/isolated_prpc_pb2.py

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/0928df20bfca97416719fc5fd60dbbb27dea0173

commit 0928df20bfca97416719fc5fd60dbbb27dea0173
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Fri Dec 14 20:00:53 2018

[isolate] Tweak isolated.proto

- Add version number to the package, I hadn't realized it was possible.
  This will help future-proofing the service.
- Since this is a breaking change, use this occasion to tweak some of
  the protos members.
- Add documentation to the proto.
- Backport improvements from ../swarming/setup_bigquery.sh. Eventually
  the script will be generalized, but not yet. :)

Deployment of this code will be annoying, as I need to recreate the BigQuery
tables from scratch.

Usage is now:

  echo '{"resolution":"MINUTE","page_size":20}' | \
    prpc call <host> isolated.v1.Isolated.Stats

Bug: 911660
Change-Id: I2b4f22f5f69ac3164682fecd5c4a61d80f644fc8
Reviewed-on: https://chromium-review.googlesource.com/c/1377914
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/handlers_prpc.py
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/handlers_prpc_test.py
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/proto/isolated.proto
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/proto/isolated_pb2.py
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/proto/isolated_prpc_pb2.py
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/setup_bigquery.sh
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/stats.py
[modify] https://crrev.com/0928df20bfca97416719fc5fd60dbbb27dea0173/appengine/isolate/stats_test.py

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/1492a840f2d79137d0513d737b0bdcdf2a6febc6

commit 1492a840f2d79137d0513d737b0bdcdf2a6febc6
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Fri Dec 14 20:11:53 2018

[isolate] Temporarily disable send_to_bq cron job

Will be reverted right away. It's just a workaround to not trigger
tainted version alert.

TBR=qyearsley@chromium.org

Bug: 911660
Change-Id: I56a3a2c59d87297b6c1995430d1c2ad0b0967a13
Reviewed-on: https://chromium-review.googlesource.com/c/1378706
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/1492a840f2d79137d0513d737b0bdcdf2a6febc6/appengine/isolate/cron.yaml

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 14

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/326070b23d198146fa7121d5c54161389d79489c

commit 326070b23d198146fa7121d5c54161389d79489c
Author: Marc-Antoine Ruel <maruel@chromium.org>
Date: Fri Dec 14 21:11:46 2018

Revert "[isolate] Temporarily disable send_to_bq cron job"

This reverts commit 1492a840f2d79137d0513d737b0bdcdf2a6febc6.

Reason for revert: Instance cleanup is completed

- I uploaded the version to all instances, disabling the cron job
- I made it the default, ensuring the default version had good code
- I deleted the entity BqStateStats
- I cleared memcache (and tweaked settings for some instances)

Original change's description:
> [isolate] Temporarily disable send_to_bq cron job
> 
> Will be reverted right away. It's just a workaround to not trigger
> tainted version alert.
> 
> TBR=qyearsley@chromium.org
> 
> Bug: 911660
> Change-Id: I56a3a2c59d87297b6c1995430d1c2ad0b0967a13
> Reviewed-on: https://chromium-review.googlesource.com/c/1378706
> Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
> Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

TBR=maruel@chromium.org,qyearsley@chromium.org

Change-Id: Ifc644e1b3aa0a1de507833ab94b49b654e855b92
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 911660
Reviewed-on: https://chromium-review.googlesource.com/c/1378569
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/326070b23d198146fa7121d5c54161389d79489c/appengine/isolate/cron.yaml

Sign in to add a comment