From c8024b18e934b86f1dafd9e825c32ab522761610 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 13:55:02 +0200 Subject: [PATCH 01/13] accept any iterable as tags --- datadog/dogstatsd/base.py | 14 +++++++++----- datadog/threadstats/base.py | 9 ++++----- datadog/threadstats/metrics.py | 1 + 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/datadog/dogstatsd/base.py b/datadog/dogstatsd/base.py index 8af35c48e..5c99aaf43 100644 --- a/datadog/dogstatsd/base.py +++ b/datadog/dogstatsd/base.py @@ -433,12 +433,16 @@ def service_check(self, check_name, status, tags=None, timestamp=None, self._send(string) def _add_constant_tags(self, tags): + result = [] + + if tags: + result.extend(tags) + if self.constant_tags: - if tags: - return tags + self.constant_tags - else: - return self.constant_tags - return tags + result.extend(self.constant_tags) + + return result + statsd = DogStatsd() diff --git a/datadog/threadstats/base.py b/datadog/threadstats/base.py index ec89c3e03..e2e4bb516 100644 --- a/datadog/threadstats/base.py +++ b/datadog/threadstats/base.py @@ -150,12 +150,11 @@ def event(self, title, text, alert_type=None, aggregation_key=None, """ if not self._disabled: # Append all client level tags to every event - event_tags = tags + event_tags = [] + if tags: + event_tags.extend(tags) if self.constant_tags: - if tags: - event_tags = tags + self.constant_tags - else: - event_tags = self.constant_tags + event_tags.extend(self.constant_tags) self._event_aggregator.add_event( title=title, text=text, alert_type=alert_type, aggregation_key=aggregation_key, diff --git a/datadog/threadstats/metrics.py b/datadog/threadstats/metrics.py index 00ff71e2c..09081b59e 100644 --- a/datadog/threadstats/metrics.py +++ b/datadog/threadstats/metrics.py @@ -163,6 +163,7 @@ def __init__(self, roll_up_interval=10): def add_point(self, metric, tags, timestamp, value, metric_class, sample_rate=1, host=None): # The sample rate is currently ignored for in process stuff interval = timestamp - timestamp % self._roll_up_interval + tags = list(tags) if tags else [] key = (metric, host, tuple(sorted(tags)) if tags else None) with self._lock: if key not in self._metrics[interval]: From f4d9ffbcd9cd85d83f595f39c24711a8418eac94 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 14:04:07 +0200 Subject: [PATCH 02/13] test any iterable metric --- tests/unit/dogstatsd/test_statsd.py | 6 ++++++ tests/unit/threadstats/test_threadstats.py | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/unit/dogstatsd/test_statsd.py b/tests/unit/dogstatsd/test_statsd.py index 367224a58..15a10f22e 100644 --- a/tests/unit/dogstatsd/test_statsd.py +++ b/tests/unit/dogstatsd/test_statsd.py @@ -302,6 +302,12 @@ def test_gauge_constant_tags_with_metric_level_tags_twice(self): self.statsd.gauge('gauge', 123.4, tags=metric_level_tag) assert self.recv() == 'gauge:123.4|g|#foo:bar,bar:baz' + def test_gauge_constant_tags_with_iterable_metric_tags(self): + metric_level_tag = iter(('foo:bar',)) + self.statsd.constant_tags = ['bar:baz'] + self.statsd.gauge('gauge', 123.4, tags=metric_level_tag) + assert self.recv() == 'gauge:123.4|g|#foo:bar,bar:baz' + @staticmethod def assert_almost_equal(a, b, delta): assert 0 <= abs(a - b) <= delta, "%s - %s not within %s" % (a, b, delta) diff --git a/tests/unit/threadstats/test_threadstats.py b/tests/unit/threadstats/test_threadstats.py index 918a3b2d4..36b554574 100644 --- a/tests/unit/threadstats/test_threadstats.py +++ b/tests/unit/threadstats/test_threadstats.py @@ -458,11 +458,11 @@ def test_tags(self): # Post the same metric with different tags. dog.gauge('gauge', 10, timestamp=100.0) dog.gauge('gauge', 15, timestamp=100.0, tags=['env:production', 'db']) - dog.gauge('gauge', 20, timestamp=100.0, tags=['env:staging']) + dog.gauge('gauge', 20, timestamp=100.0, tags=iter(['env:staging'])) dog.increment('counter', timestamp=100.0) dog.increment('counter', timestamp=100.0, tags=['env:production', 'db']) - dog.increment('counter', timestamp=100.0, tags=['env:staging']) + dog.increment('counter', timestamp=100.0, tags=set(['env:staging'])) dog.flush(200.0) @@ -497,11 +497,11 @@ def test_constant_tags(self): # Post the same metric with different tags. dog.gauge("gauge", 10, timestamp=100.0) dog.gauge("gauge", 15, timestamp=100.0, tags=["env:production", 'db']) - dog.gauge("gauge", 20, timestamp=100.0, tags=["env:staging"]) + dog.gauge("gauge", 20, timestamp=100.0, tags=iter(["env:staging"])) dog.increment("counter", timestamp=100.0) dog.increment("counter", timestamp=100.0, tags=["env:production", 'db']) - dog.increment("counter", timestamp=100.0, tags=["env:staging"]) + dog.increment("counter", timestamp=100.0, tags=set(["env:staging"])) dog.flush(200.0) From 0169953712536825342e960c238893af1e053754 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 14:22:20 +0200 Subject: [PATCH 03/13] Works, but just don't want to break tests --- datadog/threadstats/base.py | 2 ++ datadog/threadstats/metrics.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/datadog/threadstats/base.py b/datadog/threadstats/base.py index e2e4bb516..73fb3d25e 100644 --- a/datadog/threadstats/base.py +++ b/datadog/threadstats/base.py @@ -156,6 +156,8 @@ def event(self, title, text, alert_type=None, aggregation_key=None, if self.constant_tags: event_tags.extend(self.constant_tags) + event_tags = event_tags if event_tags else None + self._event_aggregator.add_event( title=title, text=text, alert_type=alert_type, aggregation_key=aggregation_key, source_type_name=source_type_name, date_happened=date_happened, priority=priority, diff --git a/datadog/threadstats/metrics.py b/datadog/threadstats/metrics.py index 09081b59e..557e63fc9 100644 --- a/datadog/threadstats/metrics.py +++ b/datadog/threadstats/metrics.py @@ -163,7 +163,7 @@ def __init__(self, roll_up_interval=10): def add_point(self, metric, tags, timestamp, value, metric_class, sample_rate=1, host=None): # The sample rate is currently ignored for in process stuff interval = timestamp - timestamp % self._roll_up_interval - tags = list(tags) if tags else [] + tags = list(tags) if tags else None key = (metric, host, tuple(sorted(tags)) if tags else None) with self._lock: if key not in self._metrics[interval]: From 04d7f198a9dfafd321bbb48ebe176de7b4947524 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 14:29:38 +0200 Subject: [PATCH 04/13] tests changed during PR --- tests/unit/dogstatsd/test_statsd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/dogstatsd/test_statsd.py b/tests/unit/dogstatsd/test_statsd.py index 1e43ad54e..0e4057b04 100644 --- a/tests/unit/dogstatsd/test_statsd.py +++ b/tests/unit/dogstatsd/test_statsd.py @@ -346,7 +346,7 @@ def test_gauge_constant_tags_with_iterable_metric_tags(self): metric_level_tag = iter(('foo:bar',)) self.statsd.constant_tags = ['bar:baz'] self.statsd.gauge('gauge', 123.4, tags=metric_level_tag) - assert self.recv() == 'gauge:123.4|g|#foo:bar,bar:baz' + assert_equal_telemetry('gauge:123.4|g|#foo:bar,bar:baz', self.recv(), telemetry=telemetry_metrics(tags="bar:baz")) @staticmethod def assert_almost_equal(a, b, delta): From 87c9a91e2818951979f39a112db38b10b5a15a90 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 14:33:05 +0200 Subject: [PATCH 05/13] Too many blank lines pep8 --- datadog/dogstatsd/base.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/datadog/dogstatsd/base.py b/datadog/dogstatsd/base.py index 6e3ca0e0e..39618a59f 100644 --- a/datadog/dogstatsd/base.py +++ b/datadog/dogstatsd/base.py @@ -513,15 +513,11 @@ def service_check(self, check_name, status, tags=None, timestamp=None, def _add_constant_tags(self, tags): result = [] - if tags: result.extend(tags) - if self.constant_tags: result.extend(self.constant_tags) - return result - statsd = DogStatsd() From 6ea67305e905144a38906bdd6e3a07f70f3b806e Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 21:39:39 +0200 Subject: [PATCH 06/13] Added iterable check for APIs --- datadog/api/api_client.py | 4 +++- datadog/api/monitors.py | 4 +++- datadog/api/synthetics.py | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/datadog/api/api_client.py b/datadog/api/api_client.py index bf94720b4..928e33697 100644 --- a/datadog/api/api_client.py +++ b/datadog/api/api_client.py @@ -7,6 +7,8 @@ import time import zlib +from collections.abc import Iterable + # datadog from datadog.api import _api_version, _max_timeouts, _backoff_period from datadog.api.exceptions import ( @@ -136,7 +138,7 @@ def submit(cls, method, path, api_version=None, body=None, attach_host_name=Fals body['host'] = _host_name # If defined, make sure tags are defined as a comma-separated string - if 'tags' in params and isinstance(params['tags'], list): + if 'tags' in params and isinstance(params['tags'], Iterable): tag_list = normalize_tags(params['tags']) params['tags'] = ','.join(tag_list) diff --git a/datadog/api/monitors.py b/datadog/api/monitors.py index 22b527e28..5bdcbe73f 100644 --- a/datadog/api/monitors.py +++ b/datadog/api/monitors.py @@ -1,6 +1,8 @@ # Unless explicitly stated otherwise all files in this repository are licensed under the BSD-3-Clause License. # This product includes software developed at Datadog (https://www.datadoghq.com/). # Copyright 2015-Present Datadog, Inc +from collections.abc import Iterable + from datadog.api.resources import GetableAPIResource, CreateableAPIResource, \ UpdatableAPIResource, ListableAPIResource, DeletableAPIResource, \ ActionAPIResource @@ -54,7 +56,7 @@ def get_all(cls, **params): :returns: Dictionary representing the API's JSON response """ for p in ['group_states', 'tags', 'monitor_tags']: - if p in params and isinstance(params[p], list): + if p in params and isinstance(params[p], Iterable): params[p] = ','.join(params[p]) return super(Monitor, cls).get_all(**params) diff --git a/datadog/api/synthetics.py b/datadog/api/synthetics.py index 134ac772d..6886fdbe1 100644 --- a/datadog/api/synthetics.py +++ b/datadog/api/synthetics.py @@ -1,6 +1,8 @@ # Unless explicitly stated otherwise all files in this repository are licensed under the BSD-3-Clause License. # This product includes software developed at Datadog (https://www.datadoghq.com/). # Copyright 2015-Present Datadog, Inc +from collections.abc import Iterable + from datadog.api.exceptions import ApiError from datadog.api.resources import ( CreateableAPIResource, @@ -55,7 +57,7 @@ def get_all_tests(cls, **params): """ for p in ["locations", "tags"]: - if p in params and isinstance(params[p], list): + if p in params and isinstance(params[p], Iterable): params[p] = ",".join(params[p]) # API path = "synthetics/tests" From f4b0cf17eea75b39df318f5dae9d7994d668d112 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 21:44:17 +0200 Subject: [PATCH 07/13] Py2 compat --- datadog/api/api_client.py | 5 ++++- datadog/api/monitors.py | 5 ++++- datadog/api/synthetics.py | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/datadog/api/api_client.py b/datadog/api/api_client.py index 928e33697..7b5d39db0 100644 --- a/datadog/api/api_client.py +++ b/datadog/api/api_client.py @@ -7,7 +7,10 @@ import time import zlib -from collections.abc import Iterable +try: + from collections.abc import Iterable +except ImportError: + from collections import Iterable # datadog from datadog.api import _api_version, _max_timeouts, _backoff_period diff --git a/datadog/api/monitors.py b/datadog/api/monitors.py index 5bdcbe73f..ff05e8aff 100644 --- a/datadog/api/monitors.py +++ b/datadog/api/monitors.py @@ -1,7 +1,10 @@ # Unless explicitly stated otherwise all files in this repository are licensed under the BSD-3-Clause License. # This product includes software developed at Datadog (https://www.datadoghq.com/). # Copyright 2015-Present Datadog, Inc -from collections.abc import Iterable +try: + from collections.abc import Iterable +except ImportError: + from collections import Iterable from datadog.api.resources import GetableAPIResource, CreateableAPIResource, \ UpdatableAPIResource, ListableAPIResource, DeletableAPIResource, \ diff --git a/datadog/api/synthetics.py b/datadog/api/synthetics.py index 6886fdbe1..163bc9e15 100644 --- a/datadog/api/synthetics.py +++ b/datadog/api/synthetics.py @@ -1,7 +1,10 @@ # Unless explicitly stated otherwise all files in this repository are licensed under the BSD-3-Clause License. # This product includes software developed at Datadog (https://www.datadoghq.com/). # Copyright 2015-Present Datadog, Inc -from collections.abc import Iterable +try: + from collections.abc import Iterable +except ImportError: + from collections import Iterable from datadog.api.exceptions import ApiError from datadog.api.resources import ( From ca3c6e63c5d04ecf55684d8ed063f9999d9f44b3 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 21:50:53 +0200 Subject: [PATCH 08/13] Add iterable tests --- tests/integration/api/test_api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/api/test_api.py b/tests/integration/api/test_api.py index 70cdc3128..8afff51f6 100644 --- a/tests/integration/api/test_api.py +++ b/tests/integration/api/test_api.py @@ -54,7 +54,8 @@ def test_tags(self, dog, get_with_retry, freezer): ) # The response from `update` can be flaky, so let's test that it work by getting the tags - dog.Tag.update(hostname, tags=["test_tag:3"], source="datadog") + # Testing Iterable tags as well + dog.Tag.update(hostname, tags=iter(["test_tag:3"]), source="datadog") get_with_retry( "Tag", hostname, @@ -114,7 +115,7 @@ def test_events(self, dog, get_with_retry, freezer): assert event["event"]["alert_type"] == "success" event = dog.Event.create( - title="test tags", text="test tags", tags=["test_tag:1", "test_tag:2"] + title="test tags", text="test tags", tags=set(["test_tag:1", "test_tag:2"]) ) assert "test_tag:1" in event["event"]["tags"] assert "test_tag:2" in event["event"]["tags"] From 1955c7dd1bcc08bd5b20129c66b340977132a731 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 22:11:59 +0200 Subject: [PATCH 09/13] Iterable body --- datadog/api/api_client.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/datadog/api/api_client.py b/datadog/api/api_client.py index 7b5d39db0..5920c89bc 100644 --- a/datadog/api/api_client.py +++ b/datadog/api/api_client.py @@ -145,6 +145,10 @@ def submit(cls, method, path, api_version=None, body=None, attach_host_name=Fals tag_list = normalize_tags(params['tags']) params['tags'] = ','.join(tag_list) + # If set, make sure tags are a list and not any other iterable. + if body and "tags" in body: + body["tags"] = list(body["tags"]) + # If defined, make sure monitor_ids are defined as a comma-separated string if 'monitor_ids' in params and isinstance(params['monitor_ids'], list): params['monitor_ids'] = ','.join(str(i) for i in params['monitor_ids']) From 4127442b456a6980a9b90fb6e50f2da4890826dc Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 22:22:38 +0200 Subject: [PATCH 10/13] Iterable body --- datadog/api/api_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datadog/api/api_client.py b/datadog/api/api_client.py index 5920c89bc..4f2cb0650 100644 --- a/datadog/api/api_client.py +++ b/datadog/api/api_client.py @@ -146,7 +146,7 @@ def submit(cls, method, path, api_version=None, body=None, attach_host_name=Fals params['tags'] = ','.join(tag_list) # If set, make sure tags are a list and not any other iterable. - if body and "tags" in body: + if body and isinstance(body.get("tags", None), Iterable): body["tags"] = list(body["tags"]) # If defined, make sure monitor_ids are defined as a comma-separated string From b5bb53402087b421f312cd71640326cdf838aeaf Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Wed, 4 Mar 2020 22:29:03 +0200 Subject: [PATCH 11/13] Re-run tests --- tests/integration/api/test_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/api/test_api.py b/tests/integration/api/test_api.py index 8afff51f6..291208c5d 100644 --- a/tests/integration/api/test_api.py +++ b/tests/integration/api/test_api.py @@ -115,7 +115,8 @@ def test_events(self, dog, get_with_retry, freezer): assert event["event"]["alert_type"] == "success" event = dog.Event.create( - title="test tags", text="test tags", tags=set(["test_tag:1", "test_tag:2"]) + title="test tags", text="test tags", + tags=frozenset(["test_tag:1", "test_tag:2"]) ) assert "test_tag:1" in event["event"]["tags"] assert "test_tag:2" in event["event"]["tags"] From 5beefac1251d86e43333619370965a1be1e93d21 Mon Sep 17 00:00:00 2001 From: "bar.harel" Date: Thu, 5 Mar 2020 13:09:50 +0200 Subject: [PATCH 12/13] Revert "Re-run tests" This reverts commit b5bb53402087b421f312cd71640326cdf838aeaf. --- tests/integration/api/test_api.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/api/test_api.py b/tests/integration/api/test_api.py index 291208c5d..8afff51f6 100644 --- a/tests/integration/api/test_api.py +++ b/tests/integration/api/test_api.py @@ -115,8 +115,7 @@ def test_events(self, dog, get_with_retry, freezer): assert event["event"]["alert_type"] == "success" event = dog.Event.create( - title="test tags", text="test tags", - tags=frozenset(["test_tag:1", "test_tag:2"]) + title="test tags", text="test tags", tags=set(["test_tag:1", "test_tag:2"]) ) assert "test_tag:1" in event["event"]["tags"] assert "test_tag:2" in event["event"]["tags"] From 50fc480f7e85ececefd99ff81d7f3825c0e8609b Mon Sep 17 00:00:00 2001 From: Hippolyte HENRY Date: Thu, 5 Mar 2020 18:26:08 +0100 Subject: [PATCH 13/13] add custom matcher to ignore tags order --- tests/integration/conftest.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9198fd654..0da5378c9 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -40,6 +40,30 @@ def api(): return api +@pytest.fixture(scope='module') +def vcr(vcr): + """Override VCR matcher.""" + from vcr.matchers import _get_transformer, _identity, read_body + + def normalize(obj, sort=False): + if isinstance(obj, dict): + return {k: normalize(v, k == "tags") for k, v in obj.items()} + if isinstance(obj, list): + x = (normalize(x) for x in obj) + return sorted(x) if sort else list(x) + return obj + + def body(r1, r2): + transformer = _get_transformer(r1) + r2_transformer = _get_transformer(r2) + if transformer != r2_transformer: + transformer = _identity + assert normalize(transformer(read_body(r1))) == normalize(transformer(read_body(r2))) + + vcr.matchers["body"] = body + return vcr + + @pytest.fixture(scope='module') def vcr_config(): return dict(