diff --git a/samcli/lib/telemetry/telemetry.py b/samcli/lib/telemetry/telemetry.py index 2daae87513..95a0f70663 100644 --- a/samcli/lib/telemetry/telemetry.py +++ b/samcli/lib/telemetry/telemetry.py @@ -78,7 +78,7 @@ def _send(self, metric, wait_for_response=False): payload = {"metrics": [metric]} LOG.debug("Sending Telemetry: %s", payload) - timeout_ms = 2000 if wait_for_response else 1 # 2 seconds to wait for response or 1ms + timeout_ms = 2000 if wait_for_response else 100 # 2 seconds to wait for response or 100ms timeout = (2, # connection timeout. Always set to 2 seconds timeout_ms / 1000.0 # Read timeout. Tweaked based on input. @@ -86,8 +86,9 @@ def _send(self, metric, wait_for_response=False): try: r = requests.post(self._url, json=payload, timeout=timeout) LOG.debug("Telemetry response: %d", r.status_code) - except requests.exceptions.Timeout as ex: - # Expected if request times out. Just print debug log and ignore the exception. + except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as ex: + # Expected if request times out OR cannot connect to the backend (offline). + # Just print debug log and ignore the exception. LOG.debug(str(ex)) def _add_common_metric_attributes(self, attrs): diff --git a/tests/integration/telemetry/integ_base.py b/tests/integration/telemetry/integ_base.py index c7098eb18d..4acb1b661c 100644 --- a/tests/integration/telemetry/integ_base.py +++ b/tests/integration/telemetry/integ_base.py @@ -54,14 +54,17 @@ def base_command(cls): return command - def run_cmd(self, stdin_data=""): + def run_cmd(self, stdin_data="", optout_envvar_value=None): # Any command will work for this test suite cmd_list = [self.cmd, "local", "generate-event", "s3", "put"] env = os.environ.copy() - # remove the envvar which usually is set in Travis. This interferes with tests. + # remove the envvar which usually is set in Travis. This interferes with tests env.pop("SAM_CLI_TELEMETRY", None) + if optout_envvar_value: + # But if the caller explicitly asked us to opt-out via EnvVar, then set it here + env["SAM_CLI_TELEMETRY"] = optout_envvar_value env["__SAM_CLI_APP_DIR"] = self.config_dir env["__SAM_CLI_TELEMETRY_ENDPOINT_URL"] = "{}/metrics".format(TELEMETRY_ENDPOINT_URL) diff --git a/tests/integration/telemetry/test_telemetry_contract.py b/tests/integration/telemetry/test_telemetry_contract.py new file mode 100644 index 0000000000..f42f01effd --- /dev/null +++ b/tests/integration/telemetry/test_telemetry_contract.py @@ -0,0 +1,73 @@ + +from .integ_base import IntegBase, TelemetryServer + + +class TestTelemetryContract(IntegBase): + """ + Validates the basic tenets/contract Telemetry module needs to adhere to + """ + + def test_must_not_send_metrics_if_disabled_using_envvar(self): + """ + No metrics should be sent if "Enabled via Config file but Disabled via Envvar" + """ + # Enable it via configuration file + self.set_config(telemetry_enabled=True) + + with TelemetryServer() as server: + # Start the CLI, but opt-out of Telemetry using env var + process = self.run_cmd(optout_envvar_value="0") + (_, stderrdata) = process.communicate() + retcode = process.poll() + self.assertEquals(retcode, 0, "Command should successfully complete") + all_requests = server.get_all_requests() + self.assertEquals(0, len(all_requests), "No metrics should be sent") + + # Now run again without the Env Var Opt out + process = self.run_cmd() + (_, stderrdata) = process.communicate() + retcode = process.poll() + self.assertEquals(retcode, 0, "Command should successfully complete") + all_requests = server.get_all_requests() + self.assertEquals(1, len(all_requests), "Command run metric should be sent") + + def test_must_send_metrics_if_enabled_via_envvar(self): + """ + Metrics should be sent if "Disabled via config file but Enabled via Envvar" + """ + # Disable it via configuration file + self.set_config(telemetry_enabled=False) + + with TelemetryServer() as server: + # Run without any envvar.Should not publish metrics + process = self.run_cmd() + (_, stderrdata) = process.communicate() + retcode = process.poll() + self.assertEquals(retcode, 0, "Command should successfully complete") + all_requests = server.get_all_requests() + self.assertEquals(0, len(all_requests), "No metric should be sent") + + # Opt-in via env var + process = self.run_cmd(optout_envvar_value="1") + (_, stderrdata) = process.communicate() + retcode = process.poll() + self.assertEquals(retcode, 0, "Command should successfully complete") + all_requests = server.get_all_requests() + self.assertEquals(1, len(all_requests), "Command run metric must be sent") + + def test_must_not_crash_when_offline(self): + """ + Must not crash the process if internet is not available + """ + self.set_config(telemetry_enabled=True) + + # DO NOT START Telemetry Server here. + # Try to run the command without it. + + # Start the CLI + process = self.run_cmd() + + (_, stderrdata) = process.communicate() + + retcode = process.poll() + self.assertEquals(retcode, 0, "Command should successfully complete") diff --git a/tests/unit/lib/telemetry/test_telemetry.py b/tests/unit/lib/telemetry/test_telemetry.py index 3777bd1636..46c32ed13a 100644 --- a/tests/unit/lib/telemetry/test_telemetry.py +++ b/tests/unit/lib/telemetry/test_telemetry.py @@ -100,7 +100,7 @@ def test_default_request_should_be_fire_and_forget(self, requests_mock): telemetry = Telemetry(url=self.url) telemetry.emit("metric_name", {}) - requests_mock.post.assert_called_once_with(ANY, json=ANY, timeout=(2, 0.001)) # 1ms response timeout + requests_mock.post.assert_called_once_with(ANY, json=ANY, timeout=(2, 0.1)) # 100ms response timeout @patch("samcli.lib.telemetry.telemetry.requests") def test_request_must_wait_for_2_seconds_for_response(self, requests_mock): @@ -121,15 +121,27 @@ def test_must_swallow_timeout_exception(self, requests_mock): # requests_mock.exceptions.Timeout = requests.exceptions.Timeout + requests_mock.exceptions.ConnectionError = requests.exceptions.ConnectionError requests_mock.post.side_effect = requests.exceptions.Timeout() telemetry.emit("metric_name", {}) + @patch("samcli.lib.telemetry.telemetry.requests") + def test_must_swallow_connection_error_exception(self, requests_mock): + telemetry = Telemetry(url=self.url) + + requests_mock.exceptions.Timeout = requests.exceptions.Timeout + requests_mock.exceptions.ConnectionError = requests.exceptions.ConnectionError + requests_mock.post.side_effect = requests.exceptions.ConnectionError() + + telemetry.emit("metric_name", {}) + @patch("samcli.lib.telemetry.telemetry.requests") def test_must_raise_on_other_requests_exception(self, requests_mock): telemetry = Telemetry(url=self.url) requests_mock.exceptions.Timeout = requests.exceptions.Timeout + requests_mock.exceptions.ConnectionError = requests.exceptions.ConnectionError requests_mock.post.side_effect = IOError() with self.assertRaises(IOError):