[autotest] Compute perf dashboard point IDs before sending.
Currently, this point ID is constructed on the perf dashboard side
when the data is received.
I believe that it's more ideal if the Chrome-OS-specific logic
is moved here:
- This will simplify the dashboard code and make it more consistent.
- This also means that if we want to send Chrome OS data with
different kinds of point IDs, that's also possible.
This also adds validation for the version numbers, so that when a
version number that doesn't match the expected format is used,
a clearer error message should be printed.
BUG=chromium:476188
TEST=Test case test_get_id_from_version and test_get_version_numbers added.
Change-Id: I193dc05d9621d08f1add2a4c0aebf63b01afc3b1
Reviewed-on: https://chromium-review.googlesource.com/265726
Reviewed-by: Fang Deng <fdeng@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Trybot-Ready: Quinten Yearsley <qyearsley@chromium.org>
Tested-by: Quinten Yearsley <qyearsley@chromium.org>
diff --git a/tko/perf_upload/perf_uploader.py b/tko/perf_upload/perf_uploader.py
index e26bd45..24b55eb 100644
--- a/tko/perf_upload/perf_uploader.py
+++ b/tko/perf_upload/perf_uploader.py
@@ -13,7 +13,7 @@
"""
-import httplib, json, math, os, urllib, urllib2
+import httplib, json, math, os, re, urllib, urllib2
import common
from autotest_lib.client.cros import constants
@@ -24,6 +24,8 @@
_ROOT_DIR, 'perf_dashboard_config.json')
_DASHBOARD_UPLOAD_URL = 'https://chromeperf.appspot.com/add_point'
+# Format for Chrome and Chrome OS version strings.
+VERSION_REGEXP = r'^(\d+)\.(\d+)\.(\d+)\.(\d+)$'
class PerfUploadingError(Exception):
"""Exception raised in perf_uploader"""
@@ -138,6 +140,8 @@
@return A dictionary containing presentation information extracted from
|config_data| for the given autotest name.
+
+ @raises PerfUploadingError if some required data is missing.
"""
if not test_name in config_data:
raise PerfUploadingError(
@@ -211,9 +215,11 @@
'error': data['stddev'],
'units': data['units'],
'higher_is_better': data['higher_is_better'],
+ 'revision': _get_id_from_version(chrome_version, cros_version),
'supplemental_columns': {
'r_cros_version': cros_version,
'r_chrome_version': chrome_version,
+ 'a_default_rev': 'r_chrome_version',
'a_hardware_identifier': hardware_id,
'a_hardware_hostname': hardware_hostname,
}
@@ -225,6 +231,93 @@
return {'data': json_string}
+def _get_version_numbers(test_attributes):
+ """Gets the version numbers from the test attributes and validates them.
+
+ @param test_attributes: The attributes property (which is a dict) of an
+ autotest tko.models.test object.
+
+ @return A pair of strings (Chrome OS version, Chrome version).
+
+ @raises PerfUploadingError if a version isn't formatted as expected.
+ """
+ chrome_version = test_attributes.get('CHROME_VERSION', '')
+ cros_version = test_attributes.get('CHROMEOS_RELEASE_VERSION', '')
+ # Prefix the ChromeOS version number with the Chrome milestone.
+ cros_version = chrome_version[:chrome_version.find('.') + 1] + cros_version
+ if not re.match(VERSION_REGEXP, cros_version):
+ raise PerfUploadingError('CrOS version "%s" does not match expected '
+ 'format.' % cros_version)
+ if not re.match(VERSION_REGEXP, chrome_version):
+ raise PerfUploadingError('Chrome version "%s" does not match expected '
+ 'format.' % chrome_version)
+ return (cros_version, chrome_version)
+
+
+def _get_id_from_version(chrome_version, cros_version):
+ """Computes the point ID to use, from Chrome and ChromeOS version numbers.
+
+ For ChromeOS row data, data values are associated with both a Chrome
+ version number and a ChromeOS version number (unlike for Chrome row data
+ that is associated with a single revision number). This function takes
+ both version numbers as input, then computes a single, unique integer ID
+ from them, which serves as a 'fake' revision number that can uniquely
+ identify each ChromeOS data point, and which will allow ChromeOS data points
+ to be sorted by Chrome version number, with ties broken by ChromeOS version
+ number.
+
+ To compute the integer ID, we take the portions of each version number that
+ serve as the shortest unambiguous names for each (as described here:
+ http://www.chromium.org/developers/version-numbers). We then force each
+ component of each portion to be a fixed width (padded by zeros if needed),
+ concatenate all digits together (with those coming from the Chrome version
+ number first), and convert the entire string of digits into an integer.
+ We ensure that the total number of digits does not exceed that which is
+ allowed by AppEngine NDB for an integer (64-bit signed value).
+
+ For example:
+ Chrome version: 27.0.1452.2 (shortest unambiguous name: 1452.2)
+ ChromeOS version: 27.3906.0.0 (shortest unambiguous name: 3906.0.0)
+ concatenated together with padding for fixed-width columns:
+ ('01452' + '002') + ('03906' + '000' + '00') = '014520020390600000'
+ Final integer ID: 14520020390600000
+
+ @param chrome_ver: The Chrome version number as a string.
+ @param cros_ver: The ChromeOS version number as a string.
+
+ @return A unique integer ID associated with the two given version numbers.
+
+ """
+
+ # Number of digits to use from each part of the version string for Chrome
+ # and Chrome OS versions when building a point ID out of these two versions.
+ chrome_version_col_widths = [0, 0, 5, 3]
+ cros_version_col_widths = [0, 5, 3, 2]
+
+ def get_digits_from_version(version_num, column_widths):
+ if re.match(VERSION_REGEXP, version_num):
+ computed_string = ''
+ version_parts = version_num.split('.')
+ for i, version_part in enumerate(version_parts):
+ if column_widths[i]:
+ computed_string += version_part.zfill(column_widths[i])
+ return computed_string
+ else:
+ return None
+
+ chrome_digits = get_digits_from_version(
+ chrome_version, chrome_version_col_widths)
+ cros_digits = get_digits_from_version(
+ cros_version, cros_version_col_widths)
+ if not chrome_digits or not cros_digits:
+ return None
+ result_digits = chrome_digits + cros_digits
+ max_digits = sum(chrome_version_col_widths + cros_version_col_widths)
+ if len(result_digits) > max_digits:
+ return None
+ return int(result_digits)
+
+
def _send_to_dashboard(data_obj):
"""Sends formatted perf data to the perf dashboard.
@@ -274,17 +367,12 @@
# Format the perf data for the upload, then upload it.
test_name = test.testname
platform_name = job.machine_group
- cros_version = test.attributes.get('CHROMEOS_RELEASE_VERSION', '')
- chrome_version = test.attributes.get('CHROME_VERSION', '')
hardware_id = test.attributes.get('hwid', '')
hardware_hostname = test.machine
variant_name = test.attributes.get(constants.VARIANT_KEY, None)
- # Prefix the chromeOS version number with the chrome milestone.
- # TODO(dennisjeffrey): Modify the dashboard to accept the chromeOS version
- # number *without* the milestone attached.
- cros_version = chrome_version[:chrome_version.find('.') + 1] + cros_version
config_data = _parse_config_file()
try:
+ cros_version, chrome_version = _get_version_numbers(test.attributes)
presentation_info = _gather_presentation_info(config_data, test_name)
formatted_data = _format_for_upload(
platform_name, cros_version, chrome_version, hardware_id,
diff --git a/tko/perf_upload/perf_uploader_unittest.py b/tko/perf_upload/perf_uploader_unittest.py
index d441f7f..edb0ca6 100644
--- a/tko/perf_upload/perf_uploader_unittest.py
+++ b/tko/perf_upload/perf_uploader_unittest.py
@@ -232,6 +232,70 @@
self._PRESENT_INFO_MISSING_MASTER, 'test_name')
+class test_get_id_from_version(unittest.TestCase):
+ """Tests for the _get_id_from_version function."""
+
+ def test_correctly_formatted_versions(self):
+ """Verifies that the expected ID is returned when input is OK."""
+ chrome_version = '27.0.1452.2'
+ cros_version = '27.3906.0.0'
+ # 1452.2 + 3906.0.0
+ # --> 01452 + 002 + 03906 + 000 + 00
+ # --> 14520020390600000
+ self.assertEqual(
+ 14520020390600000,
+ perf_uploader._get_id_from_version(
+ chrome_version, cros_version))
+
+ chrome_version = '25.10.1000.0'
+ cros_version = '25.1200.0.0'
+ # 1000.0 + 1200.0.0
+ # --> 01000 + 000 + 01200 + 000 + 00
+ # --> 10000000120000000
+ self.assertEqual(
+ 10000000120000000,
+ perf_uploader._get_id_from_version(
+ chrome_version, cros_version))
+
+ def test_returns_none_when_given_invalid_input(self):
+ """Checks the return value when invalid input is given."""
+ chrome_version = '27.0'
+ cros_version = '27.3906.0.0'
+ self.assertIsNone(perf_uploader._get_id_from_version(
+ chrome_version, cros_version))
+
+
+class test_get_version_numbers(unittest.TestCase):
+ """Tests for the _get_version_numbers function."""
+
+ def test_with_valid_versions(self):
+ """Checks the version numbers used when data is formatted as expected."""
+ self.assertEqual(
+ ('34.5678.9.0', '34.5.678.9'),
+ perf_uploader._get_version_numbers(
+ {
+ 'CHROME_VERSION': '34.5.678.9',
+ 'CHROMEOS_RELEASE_VERSION': '5678.9.0',
+ }))
+
+ def test_with_missing_version_raises_error(self):
+ """Checks that an error is raised when a version is missing."""
+ with self.assertRaises(perf_uploader.PerfUploadingError):
+ perf_uploader._get_version_numbers(
+ {
+ 'CHROMEOS_RELEASE_VERSION': '5678.9.0',
+ })
+
+ def test_with_unexpected_version_format_raises_error(self):
+ """Checks that an error is raised when there's a rN suffix."""
+ with self.assertRaises(perf_uploader.PerfUploadingError):
+ perf_uploader._get_version_numbers(
+ {
+ 'CHROME_VERSION': '34.5.678.9',
+ 'CHROMEOS_RELEASE_VERSION': '5678.9.0r1',
+ })
+
+
class test_format_for_upload(unittest.TestCase):
"""Tests for the format_for_upload function."""
@@ -299,6 +363,10 @@
expected[idx]['supplemental_columns']['r_chrome_version'],
msg=fail_msg)
self.assertEqual(
+ actual[idx]['supplemental_columns']['a_default_rev'],
+ expected[idx]['supplemental_columns']['a_default_rev'],
+ msg=fail_msg)
+ self.assertEqual(
actual[idx]['supplemental_columns']['a_hardware_identifier'],
expected[idx]['supplemental_columns']['a_hardware_identifier'],
msg=fail_msg)
@@ -308,6 +376,8 @@
msg=fail_msg)
self.assertEqual(
actual[idx]['bot'], expected[idx]['bot'], msg=fail_msg)
+ self.assertEqual(
+ actual[idx]['revision'], expected[idx]['revision'], msg=fail_msg)
self.assertAlmostEqual(
actual[idx]['value'], expected[idx]['value'], 4,
msg=fail_msg)
@@ -329,21 +399,25 @@
def test_format_for_upload(self):
"""Verifies format_for_upload generates correct json data."""
result = perf_uploader._format_for_upload(
- 'platform', '1200.0.0', '25.10.0.0', 'WINKY E2A-F2K-Q35',
+ 'platform', '25.1200.0.0', '25.10.1000.0', 'WINKY E2A-F2K-Q35',
'i7', 'test_machine', self._perf_data, self._PRESENT_INFO)
expected_result_string = (
- '[{"supplemental_columns": {"r_cros_version": "1200.0.0", '
+ '[{"supplemental_columns": {"r_cros_version": "25.1200.0.0", '
+ '"a_default_rev" : "r_chrome_version",'
'"a_hardware_identifier" : "WINKY E2A-F2K-Q35",'
'"a_hardware_hostname" : "test_machine",'
- '"r_chrome_version": "25.10.0.0"}, "bot": "cros-platform-i7", '
+ '"r_chrome_version": "25.10.1000.0"}, "bot": "cros-platform-i7", '
'"higher_is_better": false, "value": 2.7, '
+ '"revision": 10000000120000000, '
'"units": "msec", "master": "new_master_name", '
'"error": 0.2, "test": "new_test_name/graph_name/metric1"}, '
- '{"supplemental_columns": {"r_cros_version": "1200.0.0", '
+ '{"supplemental_columns": {"r_cros_version": "25.1200.0.0", '
+ '"a_default_rev" : "r_chrome_version",'
'"a_hardware_identifier" : "WINKY E2A-F2K-Q35",'
'"a_hardware_hostname" : "test_machine",'
- '"r_chrome_version": "25.10.0.0"}, "bot": "cros-platform-i7", '
+ '"r_chrome_version": "25.10.1000.0"}, "bot": "cros-platform-i7", '
'"higher_is_better": true, "value": 101.35, '
+ '"revision": 10000000120000000, '
'"units": "frames_per_sec", "master": "new_master_name", '
'"error": 5.78, "test": "new_test_name/metric2"}]')