8000 fix: Do not clear environment in subprocess integration (#418) · nutell/sentry-python@84845ac · GitHub
[go: up one dir, main page]
Skip to content

Commit 84845ac

Browse files
untitakeruntitaker
authored
fix: Do not clear environment in subprocess integration (getsentry#418)
* fix: Do no 8000 t clear environment in subprocess integration Fix getsentry#417 * fix: Fix test under Python 2 * fix: Do not modify parent process env * fix: Only copy os.environ when necessary
1 parent de96351 commit 84845ac

File tree

2 files changed

+48
-10
lines changed

2 files changed

+48
-10
lines changed

‎sentry_sdk/integrations/stdlib.py

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff lin 8000 e change
@@ -111,17 +111,28 @@ def getresponse(self, *args, **kwargs):
111111
HTTPConnection.getresponse = getresponse
112112

113113

114-
def _get_argument(args, kwargs, name, position, setdefault=None):
114+
def _init_argument(args, kwargs, name, position, setdefault_callback=None):
115+
"""
116+
given (*args, **kwargs) of a function call, retrieve (and optionally set a
117+
default for) an argument by either name or position.
118+
119+
This is useful for wrapping functions with complex type signatures and
120+
extracting a few arguments without needing to redefine that function's
121+
entire type signature.
122+
"""
123+
115124
if name in kwargs:
116125
rv = kwargs[name]
117-
if rv is None and setdefault is not None:
118-
rv = kwargs[name] = setdefault
126+
if rv is None and setdefault_callback is not None:
127+
rv = kwargs[name] = setdefault_callback()
119128
elif position < len(args):
120129
rv = args[position]
121-
if rv is None and setdefault is not None:
122-
rv = args[position] = setdefault
130+
if rv is None and setdefault_callback is not None:
131+
rv = args[position] = setdefault_callback()
123132
else:
124-
rv = kwargs[name] = setdefault
133+
rv = setdefault_callback and setdefault_callback()
134+
if rv is not None:
135+
kwargs[name] = rv
125136

126137
return rv
127138

@@ -136,11 +147,14 @@ def sentry_patched_popen_init(self, *a, **kw):
136147

137148
# do not setdefault! args is required by Popen, doing setdefault would
138149
# make invalid calls valid
139-
args = _get_argument(a, kw, "args", 0) or []
140-
cwd = _get_argument(a, kw, "cwd", 10)
150+
args = _init_argument(a, kw, "args", 0) or []
151+
cwd = _init_argument(a, kw, "cwd", 10)
152+
153+
env = None
141154

142155
for k, v in hub.iter_trace_propagation_headers():
143-
env = _get_argument(a, kw, "env", 11, {})
156+
if env is None:
157+
env = _init_argument(a, kw, "env", 11, lambda: dict(os.environ))
144158
env["SUBPROCESS_" + k.upper().replace("-", "_")] = v
145159

146160
with hub.span(op="subprocess", description=" ".join(map(str, args))) as span:

‎tests/integrations/stdlib/test_subprocess.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,37 @@
1+
import os
12
import subprocess
23
import sys
34

5+
import pytest
6+
47
from sentry_sdk import Hub, capture_message
8+
from sentry_sdk._compat import PY2
59
from sentry_sdk.integrations.stdlib import StdlibIntegration
610

711

8-
def test_subprocess_basic(sentry_init, capture_events):
12+
def test_subprocess_basic(sentry_init, capture_events, monkeypatch):
13+
monkeypatch.setenv("FOO", "bar")
14+
15+
old_environ = dict(os.environ)
16+
917
sentry_init(integrations=[StdlibIntegration()], traces_sample_rate=1.0)
1018

1119
with Hub.current.span(transaction="foo", op="foo") as span:
1220
output = subprocess.check_output(
1321
[
1422
sys.executable,
1523
"-c",
24+
"import os; "
1625
"import sentry_sdk; "
1726
"from sentry_sdk.integrations.stdlib import get_subprocess_traceparent_headers; "
1827
"sentry_sdk.init(); "
28+
"assert os.environ['FOO'] == 'bar'; "
1929
"print(dict(get_subprocess_traceparent_headers()))",
2030
]
2131
)
2232

33+
assert os.environ == old_environ
34+
2335
assert span.trace_id in str(output)
2436

2537
events = capture_events()
@@ -35,3 +47,15 @@ def test_subprocess_basic(sentry_init, capture_events):
3547
"timestamp": crumb["timestamp"],
3648
"type": "subprocess",
3749
}
50+
51+
52+
def test_subprocess_invalid_args(sentry_init):
53+
sentry_init(integrations=[StdlibIntegration()])
54+
55+
with pytest.raises(TypeError) as excinfo:
56+
subprocess.Popen()
57+
58+
if PY2:
59+
assert "__init__() takes at least 2 arguments (1 given)" in str(excinfo.value)
60+
else:
61+
assert "missing 1 required positional argument: 'args" in str(excinfo.value)

0 commit comments

Comments
 (0)
0