From ec4c557f84b33776b6660f6319c524a0f3442cc8 Mon Sep 17 00:00:00 2001 From: Saman Vaisipour Date: Fri, 28 Jun 2019 12:33:04 -0400 Subject: [PATCH 1/5] Add metrics module to DeepVariant Runner This PR contains the implementation of a decorator to collect metrics from DeepVariant Runner. A second PR will utilize the newly defined decorator to collect usage metrics. --- metrics.py | 184 ++++++++++++++++++++++++++++++++++++++++++ metrics_test.py | 210 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 394 insertions(+) create mode 100644 metrics.py create mode 100644 metrics_test.py diff --git a/metrics.py b/metrics.py new file mode 100644 index 0000000..050946b --- /dev/null +++ b/metrics.py @@ -0,0 +1,184 @@ +# Copyright 2019 Google LLC. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from this +# software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +r"""Used to collect anonymous DeepVariant metrics.""" + +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function + +import atexit +import json +import logging +import time +import uuid +import requests + +_CLEARCUT_ENDPOINT = 'https://play.googleapis.com/log' +_CLOUD_HCLS = 'CLOUD_HCLS' +_CONCORD = 'CONCORD' +_DEEP_VARIANT_RUN = 'DeepVariantRun' +_HTTP_REQUEST_TIMEOUT_SEC = 10 +_PYTHON = 'PYTHON' +_VIRTUAL_CHC_DEEPVARIANT = 'virtual.chc.deepvariant' + + +def capture_exceptions(func): + """Function decorator to capture and log any exceptions.""" + + def wrapper(*args, **kwds): + try: + return func(*args, **kwds) + # pylint:disable=broad-except + except Exception as e: + logging.error('Exception captured in %s : %s', func.__name__, e) + + return wrapper + + +class _ConcordEvent(object): + """Encapsulates information representing a Concord event.""" + + def __init__(self, + event_name, + event_type, + project_number, + console_type, + page_hostname, + event_metadata=None): + self._event_name = event_name + self._event_type = event_type + self._project_number = project_number + self._console_type = console_type + self._page_hostname = page_hostname + self._event_metadata = event_metadata or {} + + def to_json(self, **kwargs): + """Encodes data in json.""" + event_dict = { + 'project_number': self._project_number, + 'event_name': self._event_name, + 'event_type': self._event_type, + 'console_type': self._console_type, + 'page_hostname': self._page_hostname, + 'event_metadata': self._event_metadata_as_kv(), + } + return json.dumps(event_dict, **kwargs) + + def _event_metadata_as_kv(self): + # pylint:disable=g-complex-comprehension + return [{ + 'key': k, + 'value': str(v) + } for k, v in sorted(self._event_metadata.items())] + + +class _MetricsCollector(object): + """Singleton class that collects and submits metrics. + + Instances of this class share the same internal state, and thus behave + the same all the time. + """ + _shared_events = [] + _shared_session_identifier = uuid.uuid4().hex + + def __init__(self): + self._events = self._shared_events + self._session_identifier = self._shared_session_identifier + + def add_metrics(self, project_number, metrics_name, **metrics_kw): + concord_event = _ConcordEvent( + event_name=metrics_name, + event_type=_DEEP_VARIANT_RUN, + project_number=project_number, + console_type=_CLOUD_HCLS, + page_hostname=_VIRTUAL_CHC_DEEPVARIANT, + event_metadata={k: v for k, v in metrics_kw.items()}) + self._events.append(concord_event) + + def submit_metrics(self): + """Submits all the collected metrics to Concord endpoint. + + Raises: + HTTPError if http request doesn't succeed (status code != 200). + """ + request_data = json.dumps(self._clearcut_request(), sort_keys=True) + requests.post( + url=_CLEARCUT_ENDPOINT, + data=request_data, + headers=None, + timeout=_HTTP_REQUEST_TIMEOUT_SEC).raise_for_status() + + def _clearcut_request(self): + # We dont have (or want to have) any cookies. So, using a random ID for + # zwieback_cookie is ok for tracking purposes. + return { + 'client_info': { + 'client_type': _PYTHON, + }, + 'log_source_name': + _CONCORD, + 'zwieback_cookie': + self._session_identifier, + 'request_time_ms': + _now_ms(), + 'log_event': [{ + 'source_extension_json': e.to_json(sort_keys=True) + } for e in self._events] + } + + +def _now_ms(): + """Returns current time in milliseconds.""" + return int(round(time.time() * 1000)) + + +def add(project_number, metrics_name, **metrics_kw): + """Adds the given metric to the metrics to be submitted to Concord. + + Note: All metrics are submitted at exit. + Note: Do not rely on thread safety of this method. + + Args: + project_number(int): GCP project number. + metrics_name(str): metrics name. + **metrics_kw: key-values of metrics. For example, for + metrics_name="MakeExamplesSuccess", metrics_kw can be + duration_seconds=1000, wait_duration_seconds=100. + """ + metrics_collector = _MetricsCollector() + metrics_collector.add_metrics(project_number, metrics_name, **metrics_kw) + + +# Exceptions are captured and logged to avoid crashing callers. +@capture_exceptions +@atexit.register +def shutdown(): + """Reports all metrics that were collected.""" + metrics_collector = _MetricsCollector() + metrics_collector.submit_metrics() diff --git a/metrics_test.py b/metrics_test.py new file mode 100644 index 0000000..e062a44 --- /dev/null +++ b/metrics_test.py @@ -0,0 +1,210 @@ +# Copyright 2019 Google LLC. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from this +# software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +"""Tests for metrics.py. + +To run the tests, first activate virtualenv and install required packages: +$ virtualenv venv +$ . venv/bin/activate +$ pip install mock requests + +Then run: +$ python metrics_test.py +""" +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function + +import json +import unittest +from metrics import _CLEARCUT_ENDPOINT as CLEARCUT_ENDPOINT +from metrics import _MetricsCollector as MetricsCollector +import mock + + +# This is to test if all metrics collector instances share same session +# identifier. Mocks '_shared_session_identifier' on import. +@mock.patch('metrics._MetricsCollector._shared_session_identifier', 'abcd') +class MetricsCollectorTest(unittest.TestCase): + """Tests for MetricsCollector class.""" + + def _clear_metrics_collector(self): + # 'metrics_collector' is a singleton. Remove any shared state before + # starting next test. + MetricsCollector()._events[:] = [] + + @mock.patch('requests.post') + @mock.patch('time.time', return_value=1234) + def test_submit_metrics(self, unused_mock_time, mock_requests_post): + self._clear_metrics_collector() + metrics_collector = MetricsCollector() + + metrics_collector.add_metrics( + 123, + 'test-metrics-1', + attribute_1=1, + attribute_2='string-1', + attribute_3=True) + metrics_collector.add_metrics( + 123, + 'test-metrics-2', + attribute_1=2, + attribute_2='string-2', + attribute_3=True) + metrics_collector.submit_metrics() + + mock_requests_post.assert_called_with( + data=json.dumps( + { + 'zwieback_cookie': 'abcd', + 'request_time_ms': 1234000, + 'log_source_name': 'CONCORD', + 'log_event': [ + { + 'source_extension_json': + json.dumps( + { + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_1', + 'value': '1' + }, + { + 'key': 'attribute_2', + 'value': 'string-1' + }, + { + 'key': 'attribute_3', + 'value': 'True' + }], + 'event_name': 'test-metrics-1', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + }, + { + 'source_extension_json': + json.dumps({ + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_1', + 'value': '2' + }, { + 'key': 'attribute_2', + 'value': 'string-2' + }, { + 'key': 'attribute_3', + 'value': 'True' + }], + 'event_name': 'test-metrics-2', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + } + ], + 'client_info': { + 'client_type': 'PYTHON' + } + }, + sort_keys=True), + headers=None, + timeout=10, + url=CLEARCUT_ENDPOINT) + + @mock.patch('requests.post') + @mock.patch('time.time', side_effect=(1234, 1235)) + def test_two_metrics_collector(self, unused_mock_time, mock_requests_post): + self._clear_metrics_collector() + first_metric_collector = MetricsCollector() + second_metric_collector = MetricsCollector() + + first_metric_collector.add_metrics(123, 'test-metrics-1', attribute_1=1) + second_metric_collector.add_metrics(123, 'test-metrics-2', attribute_2=2) + + def expected_post_data(request_time_ms): + template = { + 'zwieback_cookie': 'abcd', + 'log_source_name': 'CONCORD', + 'log_event': [{ + 'source_extension_json': + json.dumps({ + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_1', + 'value': '1' + }], + 'event_name': 'test-metrics-1', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + }, + { + 'source_extension_json': + json.dumps({ + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_2', + 'value': '2' + }], + 'event_name': 'test-metrics-2', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + }], + 'client_info': { + 'client_type': 'PYTHON' + } + } + template.update({'request_time_ms': request_time_ms}) + return json.dumps(template, sort_keys=True) + + first_metric_collector.submit_metrics() + mock_requests_post.assert_called_with( + data=expected_post_data(1234000), + headers=None, + timeout=10, + url=CLEARCUT_ENDPOINT) + + second_metric_collector.submit_metrics() + mock_requests_post.assert_called_with( + data=expected_post_data(1235000), + headers=None, + timeout=10, + url=CLEARCUT_ENDPOINT) + + +if __name__ == '__main__': + unittest.main() From ae775d0738042dc135a5fa5db46db7e49937d0a0 Mon Sep 17 00:00:00 2001 From: Saman Vaisipour Date: Tue, 2 Jul 2019 11:56:41 -0400 Subject: [PATCH 2/5] First round of comments (Metrics) --- metrics.py | 49 +++++++++++++++++++++++++------------------------ metrics_test.py | 2 +- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/metrics.py b/metrics.py index 050946b..3f14e45 100644 --- a/metrics.py +++ b/metrics.py @@ -26,18 +26,21 @@ # CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. -r"""Used to collect anonymous DeepVariant metrics.""" +"""Used to collect anonymous DeepVariant metrics.""" from __future__ import absolute_import from __future__ import division from __future__ import print_function import atexit +import functools import json import logging import time import uuid + import requests +from typing import Dict, Optional _CLEARCUT_ENDPOINT = 'https://play.googleapis.com/log' _CLOUD_HCLS = 'CLOUD_HCLS' @@ -51,6 +54,7 @@ def capture_exceptions(func): """Function decorator to capture and log any exceptions.""" + @functools.wraps(func) def wrapper(*args, **kwds): try: return func(*args, **kwds) @@ -65,12 +69,12 @@ class _ConcordEvent(object): """Encapsulates information representing a Concord event.""" def __init__(self, - event_name, - event_type, - project_number, - console_type, - page_hostname, - event_metadata=None): + event_name: str, + event_type: str, + project_number: int, + console_type: str, + page_hostname: str, + event_metadata: Optional[Dict[str, str]] = None) -> None: self._event_name = event_name self._event_type = event_type self._project_number = project_number @@ -91,27 +95,24 @@ def to_json(self, **kwargs): return json.dumps(event_dict, **kwargs) def _event_metadata_as_kv(self): - # pylint:disable=g-complex-comprehension - return [{ - 'key': k, - 'value': str(v) - } for k, v in sorted(self._event_metadata.items())] + kv_list = [] + for k, v in sorted(self._event_metadata.items()): + kv_list.append({'key': k, 'value': str(v)}) + + return kv_list class _MetricsCollector(object): - """Singleton class that collects and submits metrics. + """A class that collects and submits metrics. - Instances of this class share the same internal state, and thus behave - the same all the time. + Instances of this class share the same internal state, and thus behave the + same all the time. """ _shared_events = [] _shared_session_identifier = uuid.uuid4().hex - def __init__(self): - self._events = self._shared_events - self._session_identifier = self._shared_session_identifier - - def add_metrics(self, project_number, metrics_name, **metrics_kw): + def add_metrics(self, project_number: int, + metrics_name: str, **metrics_kw: str) -> None: concord_event = _ConcordEvent( event_name=metrics_name, event_type=_DEEP_VARIANT_RUN, @@ -119,7 +120,7 @@ def add_metrics(self, project_number, metrics_name, **metrics_kw): console_type=_CLOUD_HCLS, page_hostname=_VIRTUAL_CHC_DEEPVARIANT, event_metadata={k: v for k, v in metrics_kw.items()}) - self._events.append(concord_event) + self._shared_events.append(concord_event) def submit_metrics(self): """Submits all the collected metrics to Concord endpoint. @@ -144,12 +145,12 @@ def _clearcut_request(self): 'log_source_name': _CONCORD, 'zwieback_cookie': - self._session_identifier, + self._shared_session_identifier, 'request_time_ms': _now_ms(), 'log_event': [{ 'source_extension_json': e.to_json(sort_keys=True) - } for e in self._events] + } for e in self._shared_events] } @@ -158,7 +159,7 @@ def _now_ms(): return int(round(time.time() * 1000)) -def add(project_number, metrics_name, **metrics_kw): +def add(project_number: int, metrics_name: str, **metrics_kw: str) -> None: """Adds the given metric to the metrics to be submitted to Concord. Note: All metrics are submitted at exit. diff --git a/metrics_test.py b/metrics_test.py index e062a44..17d1d52 100644 --- a/metrics_test.py +++ b/metrics_test.py @@ -56,7 +56,7 @@ class MetricsCollectorTest(unittest.TestCase): def _clear_metrics_collector(self): # 'metrics_collector' is a singleton. Remove any shared state before # starting next test. - MetricsCollector()._events[:] = [] + MetricsCollector()._shared_events[:] = [] @mock.patch('requests.post') @mock.patch('time.time', return_value=1234) From 2aaddc94abe28699196588d8dbd1d8c19da22bf0 Mon Sep 17 00:00:00 2001 From: Saman Vaisipour Date: Mon, 8 Jul 2019 11:35:17 -0400 Subject: [PATCH 3/5] Second round of comments (Metrics) --- metrics.py | 26 +++++------ metrics_test.py | 114 +++++++++++++++++++++++++++--------------------- 2 files changed, 77 insertions(+), 63 deletions(-) diff --git a/metrics.py b/metrics.py index 3f14e45..64ca0d4 100644 --- a/metrics.py +++ b/metrics.py @@ -40,7 +40,7 @@ import uuid import requests -from typing import Dict, Optional +from typing import Dict, Optional, Text _CLEARCUT_ENDPOINT = 'https://play.googleapis.com/log' _CLOUD_HCLS = 'CLOUD_HCLS' @@ -69,12 +69,12 @@ class _ConcordEvent(object): """Encapsulates information representing a Concord event.""" def __init__(self, - event_name: str, - event_type: str, + event_name: Text, + event_type: Text, project_number: int, - console_type: str, - page_hostname: str, - event_metadata: Optional[Dict[str, str]] = None) -> None: + console_type: Text, + page_hostname: Text, + event_metadata: Optional[Dict[Text, Text]] = None) -> None: self._event_name = event_name self._event_type = event_type self._project_number = project_number @@ -108,11 +108,11 @@ class _MetricsCollector(object): Instances of this class share the same internal state, and thus behave the same all the time. """ - _shared_events = [] - _shared_session_identifier = uuid.uuid4().hex + _events = [] + _session_identifier = uuid.uuid4().hex def add_metrics(self, project_number: int, - metrics_name: str, **metrics_kw: str) -> None: + metrics_name: Text, **metrics_kw: Text) -> None: concord_event = _ConcordEvent( event_name=metrics_name, event_type=_DEEP_VARIANT_RUN, @@ -120,7 +120,7 @@ def add_metrics(self, project_number: int, console_type=_CLOUD_HCLS, page_hostname=_VIRTUAL_CHC_DEEPVARIANT, event_metadata={k: v for k, v in metrics_kw.items()}) - self._shared_events.append(concord_event) + self._events.append(concord_event) def submit_metrics(self): """Submits all the collected metrics to Concord endpoint. @@ -145,12 +145,12 @@ def _clearcut_request(self): 'log_source_name': _CONCORD, 'zwieback_cookie': - self._shared_session_identifier, + self._session_identifier, 'request_time_ms': _now_ms(), 'log_event': [{ 'source_extension_json': e.to_json(sort_keys=True) - } for e in self._shared_events] + } for e in self._events] } @@ -159,7 +159,7 @@ def _now_ms(): return int(round(time.time() * 1000)) -def add(project_number: int, metrics_name: str, **metrics_kw: str) -> None: +def add(project_number: int, metrics_name: Text, **metrics_kw: Text) -> None: """Adds the given metric to the metrics to be submitted to Concord. Note: All metrics are submitted at exit. diff --git a/metrics_test.py b/metrics_test.py index 17d1d52..3c1fe8c 100644 --- a/metrics_test.py +++ b/metrics_test.py @@ -42,41 +42,38 @@ import json import unittest -from metrics import _CLEARCUT_ENDPOINT as CLEARCUT_ENDPOINT -from metrics import _MetricsCollector as MetricsCollector + +import metrics import mock # This is to test if all metrics collector instances share same session -# identifier. Mocks '_shared_session_identifier' on import. -@mock.patch('metrics._MetricsCollector._shared_session_identifier', 'abcd') +# identifier. Mocks '_session_identifier' on import. +@mock.patch('metrics._MetricsCollector._session_identifier', 'abcd') class MetricsCollectorTest(unittest.TestCase): """Tests for MetricsCollector class.""" - def _clear_metrics_collector(self): - # 'metrics_collector' is a singleton. Remove any shared state before - # starting next test. - MetricsCollector()._shared_events[:] = [] + def setUp(self): + super(MetricsCollectorTest, self).setUp() + # 'metrics_collector' has class attributes, clear them before each test. + metrics._MetricsCollector()._events[:] = [] @mock.patch('requests.post') @mock.patch('time.time', return_value=1234) def test_submit_metrics(self, unused_mock_time, mock_requests_post): - self._clear_metrics_collector() - metrics_collector = MetricsCollector() - - metrics_collector.add_metrics( + metrics.add( 123, 'test-metrics-1', attribute_1=1, attribute_2='string-1', attribute_3=True) - metrics_collector.add_metrics( + metrics.add( 123, 'test-metrics-2', attribute_1=2, attribute_2='string-2', attribute_3=True) - metrics_collector.submit_metrics() + metrics._MetricsCollector().submit_metrics() mock_requests_post.assert_called_with( data=json.dumps( @@ -138,52 +135,69 @@ def test_submit_metrics(self, unused_mock_time, mock_requests_post): sort_keys=True), headers=None, timeout=10, - url=CLEARCUT_ENDPOINT) + url=metrics._CLEARCUT_ENDPOINT) @mock.patch('requests.post') @mock.patch('time.time', side_effect=(1234, 1235)) def test_two_metrics_collector(self, unused_mock_time, mock_requests_post): - self._clear_metrics_collector() - first_metric_collector = MetricsCollector() - second_metric_collector = MetricsCollector() + first_metric_collector = metrics._MetricsCollector() + second_metric_collector = metrics._MetricsCollector() first_metric_collector.add_metrics(123, 'test-metrics-1', attribute_1=1) second_metric_collector.add_metrics(123, 'test-metrics-2', attribute_2=2) + metrics.add(123, 'test-metrics-3', attribute_3=3) def expected_post_data(request_time_ms): template = { 'zwieback_cookie': 'abcd', 'log_source_name': 'CONCORD', - 'log_event': [{ - 'source_extension_json': - json.dumps({ - 'console_type': 'CLOUD_HCLS', - 'event_metadata': [{ - 'key': 'attribute_1', - 'value': '1' - }], - 'event_name': 'test-metrics-1', - 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 - }, - sort_keys=True) - }, - { - 'source_extension_json': - json.dumps({ - 'console_type': 'CLOUD_HCLS', - 'event_metadata': [{ - 'key': 'attribute_2', - 'value': '2' - }], - 'event_name': 'test-metrics-2', - 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 - }, - sort_keys=True) - }], + 'log_event': [ + { + 'source_extension_json': + json.dumps({ + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_1', + 'value': '1' + }], + 'event_name': 'test-metrics-1', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + }, + { + 'source_extension_json': + json.dumps({ + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_2', + 'value': '2' + }], + 'event_name': 'test-metrics-2', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + }, + { + 'source_extension_json': + json.dumps({ + 'console_type': 'CLOUD_HCLS', + 'event_metadata': [{ + 'key': 'attribute_3', + 'value': '3' + }], + 'event_name': 'test-metrics-3', + 'event_type': 'DeepVariantRun', + 'page_hostname': 'virtual.chc.deepvariant', + 'project_number': 123 + }, + sort_keys=True) + } + ], 'client_info': { 'client_type': 'PYTHON' } @@ -196,14 +210,14 @@ def expected_post_data(request_time_ms): data=expected_post_data(1234000), headers=None, timeout=10, - url=CLEARCUT_ENDPOINT) + url=metrics._CLEARCUT_ENDPOINT) second_metric_collector.submit_metrics() mock_requests_post.assert_called_with( data=expected_post_data(1235000), headers=None, timeout=10, - url=CLEARCUT_ENDPOINT) + url=metrics._CLEARCUT_ENDPOINT) if __name__ == '__main__': From fcc34269ebf967e865afc41eafac1f82936f9d66 Mon Sep 17 00:00:00 2001 From: Saman Vaisipour Date: Thu, 11 Jul 2019 16:02:22 -0400 Subject: [PATCH 4/5] Third round of comments (Metrics) --- Dockerfile | 1 + metrics.py | 6 ++--- metrics_test.py | 70 +++++++++++++++++++++++++++---------------------- 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/Dockerfile b/Dockerfile index 0bbd611..33d290a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,6 +30,7 @@ RUN curl -L -o /usr/bin/kubectl https://storage.googleapis.com/kubernetes-releas ADD LICENSE / ADD gcp_deepvariant_runner.py /opt/deepvariant_runner/src/ ADD gke_cluster.py /opt/deepvariant_runner/src/ +ADD metrics.py /opt/deepvariant_runner/src/ ADD process_util.py /opt/deepvariant_runner/src/ ADD run_and_verify.sh /opt/deepvariant_runner/bin/ ADD cancel /opt/deepvariant_runner/bin/ diff --git a/metrics.py b/metrics.py index 64ca0d4..0d131a5 100644 --- a/metrics.py +++ b/metrics.py @@ -48,7 +48,7 @@ _DEEP_VARIANT_RUN = 'DeepVariantRun' _HTTP_REQUEST_TIMEOUT_SEC = 10 _PYTHON = 'PYTHON' -_VIRTUAL_CHC_DEEPVARIANT = 'virtual.chc.deepvariant' +_VIRTUAL_HCLS_DEEPVARIANT = 'virtual.hcls.deepvariant' def capture_exceptions(func): @@ -85,7 +85,7 @@ def __init__(self, def to_json(self, **kwargs): """Encodes data in json.""" event_dict = { - 'project_number': self._project_number, + 'project_number': str(self._project_number), 'event_name': self._event_name, 'event_type': self._event_type, 'console_type': self._console_type, @@ -118,7 +118,7 @@ def add_metrics(self, project_number: int, event_type=_DEEP_VARIANT_RUN, project_number=project_number, console_type=_CLOUD_HCLS, - page_hostname=_VIRTUAL_CHC_DEEPVARIANT, + page_hostname=_VIRTUAL_HCLS_DEEPVARIANT, event_metadata={k: v for k, v in metrics_kw.items()}) self._events.append(concord_event) diff --git a/metrics_test.py b/metrics_test.py index 3c1fe8c..6178576 100644 --- a/metrics_test.py +++ b/metrics_test.py @@ -87,22 +87,24 @@ def test_submit_metrics(self, unused_mock_time, mock_requests_post): json.dumps( { 'console_type': 'CLOUD_HCLS', - 'event_metadata': [{ - 'key': 'attribute_1', - 'value': '1' - }, - { - 'key': 'attribute_2', - 'value': 'string-1' - }, - { - 'key': 'attribute_3', - 'value': 'True' - }], + 'event_metadata': [ + { + 'key': 'attribute_1', + 'value': '1' + }, + { + 'key': 'attribute_2', + 'value': 'string-1' + }, + { + 'key': 'attribute_3', + 'value': 'True' + } + ], 'event_name': 'test-metrics-1', 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 + 'page_hostname': 'virtual.hcls.deepvariant', + 'project_number': '123' }, sort_keys=True) }, @@ -110,20 +112,24 @@ def test_submit_metrics(self, unused_mock_time, mock_requests_post): 'source_extension_json': json.dumps({ 'console_type': 'CLOUD_HCLS', - 'event_metadata': [{ - 'key': 'attribute_1', - 'value': '2' - }, { - 'key': 'attribute_2', - 'value': 'string-2' - }, { - 'key': 'attribute_3', - 'value': 'True' - }], + 'event_metadata': [ + { + 'key': 'attribute_1', + 'value': '2' + }, + { + 'key': 'attribute_2', + 'value': 'string-2' + }, + { + 'key': 'attribute_3', + 'value': 'True' + } + ], 'event_name': 'test-metrics-2', 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 + 'page_hostname': 'virtual.hcls.deepvariant', + 'project_number': '123' }, sort_keys=True) } @@ -162,8 +168,8 @@ def expected_post_data(request_time_ms): }], 'event_name': 'test-metrics-1', 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 + 'page_hostname': 'virtual.hcls.deepvariant', + 'project_number': '123' }, sort_keys=True) }, @@ -177,8 +183,8 @@ def expected_post_data(request_time_ms): }], 'event_name': 'test-metrics-2', 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 + 'page_hostname': 'virtual.hcls.deepvariant', + 'project_number': '123' }, sort_keys=True) }, @@ -192,8 +198,8 @@ def expected_post_data(request_time_ms): }], 'event_name': 'test-metrics-3', 'event_type': 'DeepVariantRun', - 'page_hostname': 'virtual.chc.deepvariant', - 'project_number': 123 + 'page_hostname': 'virtual.hcls.deepvariant', + 'project_number': '123' }, sort_keys=True) } From 9c6c1102669138cb165e02fece6f8022f23aea3b Mon Sep 17 00:00:00 2001 From: Saman Vaisipour Date: Thu, 18 Jul 2019 15:13:23 -0400 Subject: [PATCH 5/5] modify console_type to CLOUD_HCLS_OSS --- metrics.py | 4 ++-- metrics_test.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/metrics.py b/metrics.py index 0d131a5..559191b 100644 --- a/metrics.py +++ b/metrics.py @@ -43,7 +43,7 @@ from typing import Dict, Optional, Text _CLEARCUT_ENDPOINT = 'https://play.googleapis.com/log' -_CLOUD_HCLS = 'CLOUD_HCLS' +_CLOUD_HCLS_OSS = 'CLOUD_HCLS_OSS' _CONCORD = 'CONCORD' _DEEP_VARIANT_RUN = 'DeepVariantRun' _HTTP_REQUEST_TIMEOUT_SEC = 10 @@ -117,7 +117,7 @@ def add_metrics(self, project_number: int, event_name=metrics_name, event_type=_DEEP_VARIANT_RUN, project_number=project_number, - console_type=_CLOUD_HCLS, + console_type=_CLOUD_HCLS_OSS, page_hostname=_VIRTUAL_HCLS_DEEPVARIANT, event_metadata={k: v for k, v in metrics_kw.items()}) self._events.append(concord_event) diff --git a/metrics_test.py b/metrics_test.py index 6178576..8c7b2bd 100644 --- a/metrics_test.py +++ b/metrics_test.py @@ -86,7 +86,7 @@ def test_submit_metrics(self, unused_mock_time, mock_requests_post): 'source_extension_json': json.dumps( { - 'console_type': 'CLOUD_HCLS', + 'console_type': 'CLOUD_HCLS_OSS', 'event_metadata': [ { 'key': 'attribute_1', @@ -111,7 +111,7 @@ def test_submit_metrics(self, unused_mock_time, mock_requests_post): { 'source_extension_json': json.dumps({ - 'console_type': 'CLOUD_HCLS', + 'console_type': 'CLOUD_HCLS_OSS', 'event_metadata': [ { 'key': 'attribute_1', @@ -161,7 +161,7 @@ def expected_post_data(request_time_ms): { 'source_extension_json': json.dumps({ - 'console_type': 'CLOUD_HCLS', + 'console_type': 'CLOUD_HCLS_OSS', 'event_metadata': [{ 'key': 'attribute_1', 'value': '1' @@ -176,7 +176,7 @@ def expected_post_data(request_time_ms): { 'source_extension_json': json.dumps({ - 'console_type': 'CLOUD_HCLS', + 'console_type': 'CLOUD_HCLS_OSS', 'event_metadata': [{ 'key': 'attribute_2', 'value': '2' @@ -191,7 +191,7 @@ def expected_post_data(request_time_ms): { 'source_extension_json': json.dumps({ - 'console_type': 'CLOUD_HCLS', + 'console_type': 'CLOUD_HCLS_OSS', 'event_metadata': [{ 'key': 'attribute_3', 'value': '3'