Skip to content

Commit

Permalink
[Core] Do not use proxy for local connections
Browse files Browse the repository at this point in the history
Starting with Agent 5.0.0, there should always
be a local forwarder running and all payloads
should go through it.

So we should make sure that we pass the no_proxy
environment variable that will be used by requests

See: https://github.com/kennethreitz/requests/pull/945

Fix #1514
  • Loading branch information
Remi Hakim committed Apr 8, 2015
1 parent f79d93d commit ef3f417
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 17 deletions.
15 changes: 7 additions & 8 deletions dogstatsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
import os
os.umask(022)

# Starting with Agent 5.0.0, there should always be a local forwarder
# running and all payloads should go through it. So we should make sure
# that we pass the no_proxy environment variable that will be used by requests
# See: https://github.com/kennethreitz/requests/pull/945
os.environ['no_proxy'] = '127.0.0.1,localhost'

# stdlib
import logging
import optparse
Expand Down Expand Up @@ -193,18 +199,11 @@ def submit_events(self, events):
self.submit_http(url, json.dumps(payload), headers)

def submit_http(self, url, data, headers):
no_proxy = {
# See https://github.com/kennethreitz/requests/issues/879
# and https://github.com/DataDog/dd-agent/issues/1112
'no': 'pass',
}
headers["DD-Dogstatsd-Version"] = get_version()
log.debug("Posting payload to %s" % url)
try:
start_time = time()
r = requests.post(url, data=data, timeout=5,
headers=headers, proxies=no_proxy)

r = requests.post(url, data=data, timeout=5, headers=headers)
r.raise_for_status()

if r.status_code >= 200 and r.status_code < 205:
Expand Down
18 changes: 9 additions & 9 deletions emitter.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# stdlib
from hashlib import md5
import logging
import os
import re
import sys
import zlib
Expand All @@ -12,6 +13,12 @@
# project
from config import get_version

# Starting with Agent 5.0.0, there should always be a local forwarder
# running and all payloads should go through it. So we should make sure
# that we pass the no_proxy environment variable that will be used by requests
# See: https://github.com/kennethreitz/requests/pull/945
os.environ['no_proxy'] = '127.0.0.1,localhost'

# urllib3 logs a bunch of stuff at the info level
requests_log = logging.getLogger("requests.packages.urllib3")
requests_log.setLevel(logging.WARN)
Expand All @@ -21,13 +28,6 @@
control_chars = ''.join(map(unichr, range(0,32) + range(127,160)))
control_char_re = re.compile('[%s]' % re.escape(control_chars))

NO_PROXY = {
# See https://github.com/kennethreitz/requests/issues/879
# and https://github.com/DataDog/dd-agent/issues/1112
'no': 'pass',
}


def remove_control_chars(s):
return control_char_re.sub('', s)

Expand Down Expand Up @@ -55,8 +55,8 @@ def http_emitter(message, log, agentConfig):
url = "{0}/intake?api_key={1}".format(url, apiKey)

try:
r = requests.post(url, data=zipped, timeout=5,
headers=post_headers(agentConfig, zipped), proxies=NO_PROXY)
headers = post_headers(agentConfig, zipped)
r = requests.post(url, data=zipped, timeout=5, headers=headers)

r.raise_for_status()

Expand Down
37 changes: 37 additions & 0 deletions tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,43 @@ def test_collector(self):
tag = "check:%s" % check.name
assert tag in all_tags, all_tags

def test_no_proxy(self):
""" Starting with Agent 5.0.0, there should always be a local forwarder
running and all payloads should go through it. So we should make sure
that we pass the no_proxy environment variable that will be used by requests
(See: https://github.com/kennethreitz/requests/pull/945 )
"""
from requests.utils import get_environ_proxies
import dogstatsd
from os import environ as env

env["http_proxy"] = "http://localhost:3128"
env["https_proxy"] = env["http_proxy"]
env["HTTP_PROXY"] = env["http_proxy"]
env["HTTPS_PROXY"] = env["http_proxy"]

self.assertTrue("no_proxy" in env)

self.assertEquals(env["no_proxy"], "127.0.0.1,localhost")
self.assertEquals({}, get_environ_proxies(
"http://localhost:17123/intake"))

expected_proxies = {
'http': 'http://localhost:3128',
'https': 'http://localhost:3128',
'no': '127.0.0.1,localhost'
}
environ_proxies = get_environ_proxies("https://www.google.com")
self.assertEquals(expected_proxies, environ_proxies,
(expected_proxies, environ_proxies))

# Clear the env variables set
del env["http_proxy"]
del env["https_proxy"]
del env["HTTP_PROXY"]
del env["HTTPS_PROXY"]


def test_min_collection_interval(self):
if os.environ.get('TRAVIS', False):
raise SkipTest('ntp server times out too often on Travis')
Expand Down
38 changes: 38 additions & 0 deletions tests/test_dogstatsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -820,5 +820,43 @@ def test_packet_string_endings(self):
nt.assert_equals(third['metric'], 'line_ending.windows')
nt.assert_equals(third['points'][0][1], 300)


def test_no_proxy(self):
""" Starting with Agent 5.0.0, there should always be a local forwarder
running and all payloads should go through it. So we should make sure
that we pass the no_proxy environment variable that will be used by requests
(See: https://github.com/kennethreitz/requests/pull/945 )
"""
from requests.utils import get_environ_proxies
import dogstatsd
from os import environ as env

env["http_proxy"] = "http://localhost:3128"
env["https_proxy"] = env["http_proxy"]
env["HTTP_PROXY"] = env["http_proxy"]
env["HTTPS_PROXY"] = env["http_proxy"]

self.assertTrue("no_proxy" in env)

self.assertEquals(env["no_proxy"], "127.0.0.1,localhost")
self.assertEquals({}, get_environ_proxies(
"http://localhost:17123/api/v1/series"))

expected_proxies = {
'http': 'http://localhost:3128',
'https': 'http://localhost:3128',
'no': '127.0.0.1,localhost'
}
environ_proxies = get_environ_proxies("https://www.google.com")
self.assertEquals(expected_proxies, environ_proxies,
(expected_proxies, environ_proxies))

# Clear the env variables set
del env["http_proxy"]
del env["https_proxy"]
del env["HTTP_PROXY"]
del env["HTTPS_PROXY"]


if __name__ == "__main__":
unittest.main()

0 comments on commit ef3f417

Please sign in to comment.