Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings usage related to RequestsWrapper, Openmetrics and Prometheus #5080

Merged
merged 11 commits into from
Nov 27, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
import requests
from prometheus_client.parser import text_fd_to_metric_families
from six import PY3, iteritems, itervalues, string_types
from urllib3 import disable_warnings
from urllib3.exceptions import InsecureRequestWarning

from datadog_checks.base.utils.warnings_util import disable_warnings_ctx
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved

from ...config import is_affirmative
from ...errors import CheckException
from ...utils.common import to_string
Expand Down Expand Up @@ -579,25 +580,27 @@ def send_request(self, endpoint, scraper_config, headers=None):
if scraper_config['ssl_ca_cert'] is False:
verify = False

disable_insecure_warnings = False
if isinstance(scraper_config['ssl_ca_cert'], string_types):
verify = scraper_config['ssl_ca_cert']
elif verify is False:
disable_warnings(InsecureRequestWarning)
disable_insecure_warnings = True

# Determine the authentication settings
username = scraper_config['username']
password = scraper_config['password']
auth = (username, password) if username is not None and password is not None else None

return requests.get(
endpoint,
headers=headers,
stream=True,
timeout=scraper_config['prometheus_timeout'],
cert=cert,
verify=verify,
auth=auth,
)
with disable_warnings_ctx(InsecureRequestWarning, disable=disable_insecure_warnings):
return requests.get(
endpoint,
headers=headers,
stream=True,
timeout=scraper_config['prometheus_timeout'],
cert=cert,
verify=verify,
auth=auth,
)

def get_hostname_for_sample(self, sample, scraper_config):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
from google.protobuf.internal.decoder import _DecodeVarint32 # pylint: disable=E0611,E0401
from prometheus_client.parser import text_fd_to_metric_families
from six import PY3, iteritems, itervalues, string_types
from urllib3 import disable_warnings
from urllib3.exceptions import InsecureRequestWarning

from datadog_checks.base.utils.warnings_util import disable_warnings_ctx
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved

from ...utils.prometheus import metrics_pb2
from .. import AgentCheck

Expand Down Expand Up @@ -523,15 +524,17 @@ def poll(self, endpoint, pFormat=PrometheusFormat.PROTOBUF, headers=None):
if isinstance(self.ssl_private_key, string_types):
cert = (self.ssl_cert, self.ssl_private_key)
verify = True
disable_insecure_warnings = False
if isinstance(self.ssl_ca_cert, string_types):
verify = self.ssl_ca_cert
elif self.ssl_ca_cert is False:
disable_warnings(InsecureRequestWarning)
disable_insecure_warnings = True
verify = False
try:
response = requests.get(
endpoint, headers=headers, stream=False, timeout=self.prometheus_timeout, cert=cert, verify=verify
)
with disable_warnings_ctx(InsecureRequestWarning, disable=disable_insecure_warnings):
response = requests.get(
endpoint, headers=headers, stream=False, timeout=self.prometheus_timeout, cert=cert, verify=verify
)
except requests.exceptions.SSLError:
self.log.error("Invalid SSL settings for requesting %s endpoint", endpoint)
raise
Expand Down
21 changes: 7 additions & 14 deletions datadog_checks_base/datadog_checks/base/utils/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
# Licensed under a 3-clause BSD style license (see LICENSE)
import logging
import os
import threading
import warnings
from contextlib import contextmanager

import requests
from six import iteritems, string_types
from six.moves.urllib.parse import urlparse
from urllib3.exceptions import InsecureRequestWarning

from datadog_checks.base.utils.warnings_util import disable_warnings_ctx
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved

from ..config import is_affirmative
from ..errors import ConfigurationError
from .headers import get_default_headers, update_headers
Expand Down Expand Up @@ -94,10 +94,6 @@ class RequestsWrapper(object):
'request_hooks',
)

# For modifying the warnings filter since the context
# manager that is provided changes module constants
warning_lock = threading.Lock()

def __init__(self, instance, init_config, remapper=None, logger=None):
self.logger = logger or LOGGER
default_fields = dict(STANDARD_FIELDS)
Expand Down Expand Up @@ -276,7 +272,9 @@ def __init__(self, instance, init_config, remapper=None, logger=None):
self.log_requests = is_affirmative(config['log_requests'])

# Context managers that should wrap all requests
self.request_hooks = [self.handle_tls_warning]
self.request_hooks = []
if self.ignore_tls_warning:
self.request_hooks.append(self.handle_tls_warning)

if config['kerberos_keytab']:
self.request_hooks.append(lambda: handle_kerberos_keytab(config['kerberos_keytab']))
Expand Down Expand Up @@ -339,13 +337,8 @@ def populate_options(self, options):

@contextmanager
def handle_tls_warning(self):
with self.warning_lock:

with warnings.catch_warnings():
if self.ignore_tls_warning:
warnings.simplefilter('ignore', InsecureRequestWarning)

yield
with disable_warnings_ctx(InsecureRequestWarning, disable=True):
yield

@property
def session(self):
Expand Down
36 changes: 36 additions & 0 deletions datadog_checks_base/datadog_checks/base/utils/warnings_util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# (C) Datadog, Inc. 2019
# All rights reserved
# Licensed under a 3-clause BSD style license (see LICENSE)
import warnings
from contextlib import contextmanager

from six import PY2


def _simplefilter_py2(action, category=Warning, lineno=0, append=0):
""" Add remove logic for py2 to avoid warnings.filters to growth
indefinitely if simplefilter is called multiple times """
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved
item = (action, None, category, None, int(lineno))
if not append:
therve marked this conversation as resolved.
Show resolved Hide resolved
try:
warnings.filters.remove(item)
except ValueError:
pass
warnings.simplefilter(action, category=category, lineno=lineno, append=append)


if PY2:
simplefilter = _simplefilter_py2
else:
simplefilter = warnings.simplefilter


@contextmanager
def disable_warnings_ctx(action, disable=True):
if disable:
simplefilter('ignore', action)
yield
simplefilter('default', action)
else:
# do nothing
yield
17 changes: 17 additions & 0 deletions datadog_checks_base/tests/test_openmetrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -2095,3 +2095,20 @@ def test_metadata_transformer(mocked_openmetrics_check_factory, text_data):
m.assert_any_call('test:123', 'version.raw', 'v1.6.0-alpha.0.680+3872cb93abf948-dirty')
m.assert_any_call('test:123', 'version.scheme', 'semver')
assert m.call_count == 7


def test_ssl_verify_not_raise_warning(mocked_openmetrics_check_factory, text_data):
instance = dict(
{
'prometheus_url': 'https://www.example.com',
'metrics': [{'foo': 'bar'}],
'namespace': 'openmetrics',
'ssl_verify': False,
}
)
check = mocked_openmetrics_check_factory(instance)
scraper_config = check.get_scraper_config(instance)

with pytest.warns(None):
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved
resp = check.send_request('https://httpbin.org/get', scraper_config)
assert "httpbin.org" in resp.content.decode('utf-8')
15 changes: 15 additions & 0 deletions datadog_checks_base/tests/test_prometheus.py
Original file line number Diff line number Diff line change
Expand Up @@ -1957,3 +1957,18 @@ def test_text_filter_input():

filtered = [x for x in check._text_filter_input(lines_in)]
assert filtered == expected_out


def test_ssl_verify_not_raise_warning(mocked_prometheus_check, text_data):
check = mocked_prometheus_check

with pytest.warns(None):
resp = check.poll('https://httpbin.org/get')
assert "httpbin.org" in resp.content.decode('utf-8')

check = mocked_prometheus_check
check.ssl_ca_cert = False

with pytest.warns(None):
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved
resp = check.poll('https://httpbin.org/get')
assert "httpbin.org" in resp.content.decode('utf-8')
43 changes: 43 additions & 0 deletions datadog_checks_base/tests/test_warnings_util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# (C) Datadog, Inc. 2019
# All rights reserved
# Licensed under a 3-clause BSD style license (see LICENSE)
import warnings

import pytest
import requests
from urllib3.exceptions import InsecureRequestWarning

from datadog_checks.base.utils.warnings_util import disable_warnings_ctx, simplefilter

pytestmark = pytest.mark.warnings
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved


def test_filters_count():
initial_count = len(warnings.filters)

for _ in range(100):
simplefilter('default', InsecureRequestWarning)

final_count = len(warnings.filters)

assert final_count in (initial_count, initial_count + 1)


def test_disable_warnings_ctx_disabled():

with disable_warnings_ctx(InsecureRequestWarning):
with pytest.warns(None):
requests.get('https://www.example.com')

with disable_warnings_ctx(InsecureRequestWarning, disable=True):
with pytest.warns(None):
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved
requests.get('https://www.example.com')


def test_disable_warnings_ctx_not_disabled():
with pytest.warns(InsecureRequestWarning):
requests.get('https://www.example.com', verify=False)

with disable_warnings_ctx(InsecureRequestWarning, disable=False):
with pytest.warns(InsecureRequestWarning):
requests.get('https://www.example.com', verify=False)
AlexandreYang marked this conversation as resolved.
Show resolved Hide resolved