From 9fe0298fc0f8b4fbc12a8897140eb7c8c2a7b316 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 19:17:21 +0200 Subject: [PATCH 1/8] Handle server info for envoy <= 1.8 --- envoy/datadog_checks/envoy/envoy.py | 21 +++++++++++++- envoy/tests/common.py | 4 +-- envoy/tests/fixtures/server_info | 1 + envoy/tests/fixtures/server_info_before_1_9 | 1 + envoy/tests/fixtures/server_info_invalid | 1 + envoy/tests/test_envoy.py | 32 +++++++++++++++++++-- 6 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 envoy/tests/fixtures/server_info create mode 100644 envoy/tests/fixtures/server_info_before_1_9 create mode 100644 envoy/tests/fixtures/server_info_invalid diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index 54f989097537f..21b7b0b31bcbc 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -1,6 +1,7 @@ # (C) Datadog, Inc. 2018-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +import json import re from collections import defaultdict @@ -12,6 +13,8 @@ from .errors import UnknownMetric, UnknownTags from .parser import parse_histogram, parse_metric +LEGACY_VERSION_RE = re.compile(r'/(\d.\d.\d)/') + class Envoy(AgentCheck): HTTP_CONFIG_REMAPPER = {'verify_ssl': {'name': 'tls_verify'}} @@ -159,7 +162,23 @@ def _collect_metadata(self, stats_url): # "state": "LIVE", # ... # } - raw_version = response.json()["version"].split('/')[1] + try: + raw_version = response.json()["version"].split('/')[1] + except json.decoder.JSONDecodeError as e: + self.log.debug('Error decoding json for with url=`%s`. Error: %s', server_info_url, str(e)) + + if raw_version is None: + # Search version in server info for Envoy version <= 1.8 + # Example: + # envoy 5d25f466c3410c0dfa735d7d4358beb76b2da507/1.8.0/Clean/RELEASE live 581130 581130 0 + content = response.content.decode() + found = LEGACY_VERSION_RE.search(content) + if found: + raw_version = found.group(1) + else: + self.log.warning('Version not matched. content=%s', content) + return + except requests.exceptions.Timeout: self.log.warning( 'Envoy endpoint `%s` timed out after %s seconds', server_info_url, self.http.options['timeout'] diff --git a/envoy/tests/common.py b/envoy/tests/common.py index b44c6b39ddb4b..90c5fd01b5533 100644 --- a/envoy/tests/common.py +++ b/envoy/tests/common.py @@ -1,3 +1,4 @@ +import json import os from datadog_checks.base.utils.common import get_docker_hostname @@ -36,8 +37,7 @@ def __init__(self, content, status_code): self.status_code = status_code def json(self): - # Metadata - return SERVER_INFO + return json.loads(self.content) @lru_cache(maxsize=None) diff --git a/envoy/tests/fixtures/server_info b/envoy/tests/fixtures/server_info new file mode 100644 index 0000000000000..cf995695dd5c4 --- /dev/null +++ b/envoy/tests/fixtures/server_info @@ -0,0 +1 @@ +{"version": "222aaacccfff888/1.14.1/Clean/RELEASE/BoringSSL", "state": "LIVE"} diff --git a/envoy/tests/fixtures/server_info_before_1_9 b/envoy/tests/fixtures/server_info_before_1_9 new file mode 100644 index 0000000000000..d8d6576eaa334 --- /dev/null +++ b/envoy/tests/fixtures/server_info_before_1_9 @@ -0,0 +1 @@ +envoy 5d25f466c3410c0dfa735d7d4358beb76b2da507/1.8.0/Clean/RELEASE live 581130 581130 0 diff --git a/envoy/tests/fixtures/server_info_invalid b/envoy/tests/fixtures/server_info_invalid new file mode 100644 index 0000000000000..5da849b5c6f00 --- /dev/null +++ b/envoy/tests/fixtures/server_info_invalid @@ -0,0 +1 @@ +ABC diff --git a/envoy/tests/test_envoy.py b/envoy/tests/test_envoy.py index e83206bf91107..d316a835c3ace 100644 --- a/envoy/tests/test_envoy.py +++ b/envoy/tests/test_envoy.py @@ -123,7 +123,7 @@ def test_config(test_case, extra_config, expected_http_kwargs): r.get.assert_called_with('http://{}:8001/stats'.format(HOST), **http_wargs) -def test_metadata(datadog_agent): +def test_metadata_unit(datadog_agent): instance = INSTANCES['main'] check = Envoy(CHECK_NAME, {}, [instance]) check.check_id = 'test:123' @@ -136,6 +136,7 @@ def test_metadata(datadog_agent): 'Envoy endpoint `%s` timed out after %s seconds', 'http://localhost:8001/server_info', (10.0, 10.0) ) + datadog_agent.reset() with mock.patch('requests.get', side_effect=IndexError()): check._collect_metadata(instance['stats_url']) datadog_agent.assert_metadata_count(0) @@ -143,6 +144,7 @@ def test_metadata(datadog_agent): 'Error collecting Envoy version with url=`%s`. Error: %s', 'http://localhost:8001/server_info', '' ) + datadog_agent.reset() with mock.patch('requests.get', side_effect=requests.exceptions.RequestException('Req Exception')): check._collect_metadata(instance['stats_url']) datadog_agent.assert_metadata_count(0) @@ -152,7 +154,8 @@ def test_metadata(datadog_agent): 'Req Exception', ) - with mock.patch('requests.get', return_value=response('multiple_services')): + datadog_agent.reset() + with mock.patch('requests.get', return_value=response('server_info')): check._collect_metadata(instance['stats_url']) major, minor, patch = ENVOY_VERSION.split('.') @@ -167,6 +170,31 @@ def test_metadata(datadog_agent): datadog_agent.assert_metadata('test:123', version_metadata) datadog_agent.assert_metadata_count(len(version_metadata)) + datadog_agent.reset() + with mock.patch('requests.get', return_value=response('server_info_before_1_9')): + check._collect_metadata(instance['stats_url']) + + expected_version = '1.8.0' + major, minor, patch = expected_version.split('.') + version_metadata = { + 'version.scheme': 'semver', + 'version.major': major, + 'version.minor': minor, + 'version.patch': patch, + 'version.raw': expected_version, + } + + datadog_agent.assert_metadata('test:123', version_metadata) + datadog_agent.assert_metadata_count(len(version_metadata)) + + datadog_agent.reset() + with mock.patch('requests.get', return_value=response('server_info_invalid')): + check._collect_metadata(instance['stats_url']) + + datadog_agent.assert_metadata('test:123', {}) + datadog_agent.assert_metadata_count(0) + check.log.warning.assert_called_with('Version not matched. content=%s', 'ABC\n') + @pytest.mark.usefixtures('dd_environment') def test_metadata_integration(aggregator, datadog_agent): From c22377ddcce148064fda4eacf3d875f86b87b4d0 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 19:19:47 +0200 Subject: [PATCH 2/8] Fix test --- envoy/datadog_checks/envoy/envoy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index 21b7b0b31bcbc..8731971981f7b 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -148,6 +148,7 @@ def whitelisted_metric(self, metric): def _collect_metadata(self, stats_url): # From http://domain/thing/stats to http://domain/thing/server_info server_info_url = urljoin(stats_url, 'server_info') + raw_version = None try: response = self.http.get(server_info_url) From 2852718fb15592012ceaff3b47a6644a0f56b0e9 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 19:35:46 +0200 Subject: [PATCH 3/8] use debug --- envoy/datadog_checks/envoy/envoy.py | 5 +++-- envoy/tests/test_envoy.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index 8731971981f7b..69c8191751298 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -177,7 +177,7 @@ def _collect_metadata(self, stats_url): if found: raw_version = found.group(1) else: - self.log.warning('Version not matched. content=%s', content) + self.log.debug('Version not matched. content=%s', content) return except requests.exceptions.Timeout: @@ -189,4 +189,5 @@ def _collect_metadata(self, stats_url): self.log.warning('Error collecting Envoy version with url=`%s`. Error: %s', server_info_url, str(e)) return - self.set_metadata('version', raw_version) + if raw_version: + self.set_metadata('version', raw_version) diff --git a/envoy/tests/test_envoy.py b/envoy/tests/test_envoy.py index d316a835c3ace..76fd1432c590e 100644 --- a/envoy/tests/test_envoy.py +++ b/envoy/tests/test_envoy.py @@ -193,7 +193,7 @@ def test_metadata_unit(datadog_agent): datadog_agent.assert_metadata('test:123', {}) datadog_agent.assert_metadata_count(0) - check.log.warning.assert_called_with('Version not matched. content=%s', 'ABC\n') + check.log.debug.assert_called_with('Version not matched. content=%s', 'ABC\n') @pytest.mark.usefixtures('dd_environment') From ce9dec9b48bcd9ac0a13b2d2581d7e72f13ba15e Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 19:36:48 +0200 Subject: [PATCH 4/8] fix typo --- envoy/datadog_checks/envoy/envoy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index 69c8191751298..e1abb82cd91bb 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -166,7 +166,7 @@ def _collect_metadata(self, stats_url): try: raw_version = response.json()["version"].split('/')[1] except json.decoder.JSONDecodeError as e: - self.log.debug('Error decoding json for with url=`%s`. Error: %s', server_info_url, str(e)) + self.log.debug('Error decoding json for url=`%s`. Error: %s', server_info_url, str(e)) if raw_version is None: # Search version in server info for Envoy version <= 1.8 From 249165ca71dd1d5eb0d530e0ca21ac4d7ae2e75d Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 19:41:38 +0200 Subject: [PATCH 5/8] update test --- envoy/datadog_checks/envoy/envoy.py | 3 ++- envoy/tests/test_envoy.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index e1abb82cd91bb..afa8a91e7d458 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -174,10 +174,11 @@ def _collect_metadata(self, stats_url): # envoy 5d25f466c3410c0dfa735d7d4358beb76b2da507/1.8.0/Clean/RELEASE live 581130 581130 0 content = response.content.decode() found = LEGACY_VERSION_RE.search(content) + self.log.debug('Looking for version in content: %s', content) if found: raw_version = found.group(1) else: - self.log.debug('Version not matched. content=%s', content) + self.log.debug('Version not matched.') return except requests.exceptions.Timeout: diff --git a/envoy/tests/test_envoy.py b/envoy/tests/test_envoy.py index 76fd1432c590e..afc23befbb254 100644 --- a/envoy/tests/test_envoy.py +++ b/envoy/tests/test_envoy.py @@ -193,7 +193,7 @@ def test_metadata_unit(datadog_agent): datadog_agent.assert_metadata('test:123', {}) datadog_agent.assert_metadata_count(0) - check.log.debug.assert_called_with('Version not matched. content=%s', 'ABC\n') + check.log.debug.assert_called_with('Version not matched.') @pytest.mark.usefixtures('dd_environment') From 4e36fca3a2d08ca4b0f7224dca57aeb36cee0d80 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 19:44:58 +0200 Subject: [PATCH 6/8] clean up --- envoy/tests/common.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/envoy/tests/common.py b/envoy/tests/common.py index 90c5fd01b5533..d0e43df1825ee 100644 --- a/envoy/tests/common.py +++ b/envoy/tests/common.py @@ -24,10 +24,6 @@ 'metric_blacklist': [r'envoy\.cluster\.out\.'], }, } -SERVER_INFO = { - "version": "222aaacccfff888/1.14.1/Clean/RELEASE/BoringSSL", - "state": "LIVE", -} ENVOY_VERSION = os.getenv('ENVOY_VERSION') From e3e9c696d381558d9537daf6ecf5355d11beea2e Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 20:08:35 +0200 Subject: [PATCH 7/8] Fix exception handling --- envoy/datadog_checks/envoy/envoy.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index afa8a91e7d458..84289667610ff 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -1,7 +1,6 @@ # (C) Datadog, Inc. 2018-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -import json import re from collections import defaultdict @@ -165,7 +164,7 @@ def _collect_metadata(self, stats_url): # } try: raw_version = response.json()["version"].split('/')[1] - except json.decoder.JSONDecodeError as e: + except Exception as e: self.log.debug('Error decoding json for url=`%s`. Error: %s', server_info_url, str(e)) if raw_version is None: From efba207be02e7f63ec440e51c4e0c6207add0e58 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Tue, 26 May 2020 20:18:56 +0200 Subject: [PATCH 8/8] Fix regex --- envoy/datadog_checks/envoy/envoy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envoy/datadog_checks/envoy/envoy.py b/envoy/datadog_checks/envoy/envoy.py index 84289667610ff..27355417615e3 100644 --- a/envoy/datadog_checks/envoy/envoy.py +++ b/envoy/datadog_checks/envoy/envoy.py @@ -12,7 +12,7 @@ from .errors import UnknownMetric, UnknownTags from .parser import parse_histogram, parse_metric -LEGACY_VERSION_RE = re.compile(r'/(\d.\d.\d)/') +LEGACY_VERSION_RE = re.compile(r'/(\d\.\d\.\d)/') class Envoy(AgentCheck):