Skip to content

Commit

Permalink
Report the stats version pants is using to the server. (pantsbuild#8086)
Browse files Browse the repository at this point in the history
### Problem

Pants doesn't report which version of stats it using when sending build stats data.

### Solution

Put the stats version information in the HTTP request header.

### Result

The server processing the build data doesn't have to infer which version of the stats it is getting.
The server can just read this information from the HTTP request headers.
  • Loading branch information
asherf authored and stuhood committed Jul 23, 2019
1 parent 54d0d4e commit 570fab8
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 11 deletions.
25 changes: 17 additions & 8 deletions src/python/pants/goal/run_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class RunTracker(Subsystem):

# The name of the tracking root for the background worker threads.
BACKGROUND_ROOT_NAME = 'background'
SUPPORTED_STATS_VERSIONS = [1, 2]

@classmethod
def subsystem_dependencies(cls):
Expand All @@ -87,7 +88,7 @@ def register_options(cls, register):
'auth provider name is only used to provide a more helpful error message.')
register('--stats-upload-timeout', advanced=True, type=int, default=2,
help='Wait at most this many seconds for the stats upload to complete.')
register('--stats-version', advanced=True, type=int, default=1, choices=[1, 2],
register('--stats-version', advanced=True, type=int, default=1, choices=cls.SUPPORTED_STATS_VERSIONS,
help='Format of stats JSON for uploads and local json file.')
register('--num-foreground-workers', advanced=True, type=int,
default=multiprocessing.cpu_count(),
Expand Down Expand Up @@ -253,7 +254,7 @@ def start(self, report, run_start_time=None):
self.report = report

# Set up the JsonReporter for V2 stats.
if self.get_options().stats_version == 2:
if self._stats_version == 2:
json_reporter_settings = JsonReporter.Settings(log_level=Report.INFO)
self.json_reporter = JsonReporter(self, json_reporter_settings)
report.add_reporter('json', self.json_reporter)
Expand Down Expand Up @@ -348,16 +349,22 @@ def new_workunit_under_parent(self, name, parent, labels=None, cmd='', log_confi
workunit.set_outcome(outcome)
self.end_workunit(workunit)

@property
def _stats_version(self) -> int:
return self.get_options().stats_version

def log(self, level, *msg_elements):
"""Log a message against the current workunit."""
self.report.log(self._threadlocal.current_workunit, level, *msg_elements)

@classmethod
def _get_headers(cls) -> Dict[str, str]:
return {'User-Agent': f"pants/v{VERSION}"}
def _get_headers(cls, stats_version: int) -> Dict[str, str]:
return {'User-Agent': f"pants/v{VERSION}",
'X-Pants-Stats-Version': str(stats_version),
}

@classmethod
def post_stats(cls, stats_url, stats, timeout=2, auth_provider=None):
def post_stats(cls, stats_url, stats, timeout=2, auth_provider=None, stats_version=1):
"""POST stats to the given url.
:return: True if upload was successful, False otherwise.
Expand All @@ -367,12 +374,14 @@ def error(msg):
print(f'WARNING: Failed to upload stats to {stats_url} due to {msg}', file=sys.stderr)
return False

if stats_version not in cls.SUPPORTED_STATS_VERSIONS:
raise ValueError("Invalid stats version")
# TODO(benjy): The upload protocol currently requires separate top-level params, with JSON
# values. Probably better for there to be one top-level JSON value, namely json.dumps(stats).
# But this will first require changing the upload receiver at every shop that uses this.
params = {k: cls._json_dump_options(v) for (k, v) in stats.items()}
cookies = Cookies.global_instance()
headers = cls._get_headers()
headers = cls._get_headers(stats_version=stats_version)
auth_provider = auth_provider or '<provider>'

# We can't simply let requests handle redirects, as we only allow them for specific codes:
Expand Down Expand Up @@ -425,7 +434,7 @@ def run_information(self):
return run_information

def _stats(self):
if self.get_options().stats_version == 2:
if self._stats_version == 2:
return {
'run_info': self.run_information(),
'artifact_cache_stats': self.artifact_cache_stats.get_all(),
Expand Down Expand Up @@ -457,7 +466,7 @@ def store_stats(self):
stats_upload_urls = copy.copy(self.get_options().stats_upload_urls)
timeout = self.get_options().stats_upload_timeout
for stats_url, auth_provider in stats_upload_urls.items():
self.post_stats(stats_url, stats, timeout=timeout, auth_provider=auth_provider)
self.post_stats(stats_url, stats, timeout=timeout, auth_provider=auth_provider, stats_version=self._stats_version)

_log_levels = [Report.ERROR, Report.ERROR, Report.WARN, Report.INFO, Report.INFO]

Expand Down
22 changes: 19 additions & 3 deletions tests/python/pants_test/goal/test_run_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def do_POST(handler):
decoded_post_data = {k: json.loads(v[0]) for k, v in post_data.items()}
self.assertEqual(stats, decoded_post_data)
self.assertEqual(handler.headers['User-Agent'], f"pants/v{VERSION}")
self.assertIn(handler.headers['X-Pants-Stats-Version'], {"1", "2"})
handler.send_response(200)
handler.end_headers()
except Exception:
Expand All @@ -51,13 +52,28 @@ def mk_url(path):
server_thread.start()

self.context(for_subsystems=[Cookies])
self.assertTrue(RunTracker.post_stats(mk_url('/upload'), stats))
self.assertTrue(RunTracker.post_stats(mk_url('/redirect307'), stats))
self.assertFalse(RunTracker.post_stats(mk_url('/redirect302'), stats))
self.assertTrue(RunTracker.post_stats(mk_url('/upload'), stats, stats_version=1))
self.assertTrue(RunTracker.post_stats(mk_url('/redirect307'), stats, stats_version=1))
self.assertFalse(RunTracker.post_stats(mk_url('/redirect302'), stats, stats_version=2))

server.shutdown()
server.server_close()

def test_invalid_stats_version(self):
stats = {'stats': {'foo': 'bar', 'baz': 42}}
url = 'http://example.com/upload/'
with self.assertRaises(ValueError):
RunTracker.post_stats(url, stats, stats_version=0)

with self.assertRaises(ValueError):
RunTracker.post_stats(url, stats, stats_version=None)

with self.assertRaises(ValueError):
RunTracker.post_stats(url, stats, stats_version=9)

with self.assertRaises(ValueError):
RunTracker.post_stats(url, stats, stats_version="not a number")

def test_write_stats_to_json_file(self):
# Set up
stats = {'stats': {'foo': 'bar', 'baz': 42}}
Expand Down

0 comments on commit 570fab8

Please sign in to comment.