Skip to content

Commit fd12042

Browse files
chore(tracer): core interface improvement (#7863)
# Motivation Currently core interface can be use to register and call listener on specific internal events with - `core.on` to register an event - `core.dispatch` to call listener on a specific event `core.dispatch` returns a tuple of lists containing results or possible exceptions of all listeners in the order they were registered. This is difficult to use, as we have to properly check the lists length and guess the registration order in case of several listeners. Using `[0][0]` to access result is not meaningful nor without a risk of errors. # Proposition of this PR - `core.on` to register an event with an additional optional name - `core.dispatch` to call listener on a specific event will return a EventResultDict that can be used like that: ```python # some file core.on("my_event", listener_function, "my_name") # some other file core.on("my_event", listener_function_2, "my_second_name") # some other listener, but you don't care about the return value, so no need to set a name core.on("my_event", listener_function_3) # in another file res = core.dispatch("my_event") if res.my_name: result = res.my_name.value ... # optional error handling else: if res.my_name.response_type == core.ResultType.RESULT_UNDEFINED: log.debug("listener my_name was not registered on my_event") elif res..my_name.response_type == core.ResultType.RESULT_EXCEPTION: log.debug("listener my_name on my_event raised exception %s", res.my_name.exception) # you can also just simply get the returned value of the listener. # In case of an error, this is set to None anyway, so it never breaks. second_name = res.my_second_name.value ``` ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that.
1 parent 4626db4 commit fd12042

File tree

15 files changed

+136
-92
lines changed

15 files changed

+136
-92
lines changed

ddtrace/appsec/_asm_request_context.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -447,12 +447,12 @@ def _set_headers_and_response(response, headers, *_):
447447

448448
def _call_waf_first(integration, *_):
449449
log.debug("%s WAF call for Suspicious Request Blocking on request", integration)
450-
call_waf_callback()
450+
return call_waf_callback()
451451

452452

453453
def _call_waf(integration, *_):
454454
log.debug("%s WAF call for Suspicious Request Blocking on response", integration)
455-
call_waf_callback()
455+
return call_waf_callback()
456456

457457

458458
def _on_block_decided(callback):
@@ -466,15 +466,15 @@ def _get_headers_if_appsec():
466466

467467
def listen_context_handlers():
468468
core.on("flask.finalize_request.post", _set_headers_and_response)
469-
core.on("flask.wrapped_view", _on_wrapped_view)
469+
core.on("flask.wrapped_view", _on_wrapped_view, "callback_and_args")
470470
core.on("flask._patched_request", _on_pre_tracedrequest)
471471
core.on("wsgi.block_decided", _on_block_decided)
472-
core.on("flask.start_response", _call_waf)
472+
core.on("flask.start_response", _call_waf, "waf")
473473

474474
core.on("django.start_response.post", _call_waf)
475475
core.on("django.finalize_response", _call_waf)
476-
core.on("django.after_request_headers", _get_headers_if_appsec)
477-
core.on("django.extract_body", _get_headers_if_appsec)
476+
core.on("django.after_request_headers", _get_headers_if_appsec, "headers")
477+
core.on("django.extract_body", _get_headers_if_appsec, "headers")
478478
core.on("django.after_request_headers.finalize", _set_headers_and_response)
479479
core.on("flask.set_request_tags", _on_set_request_tags)
480480

ddtrace/appsec/_handlers.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,14 +293,14 @@ def _on_django_patch():
293293

294294

295295
def listen():
296-
core.on("flask.request_call_modifier", _on_request_span_modifier)
296+
core.on("flask.request_call_modifier", _on_request_span_modifier, "request_body")
297297
core.on("flask.request_init", _on_request_init)
298298
core.on("flask.blocked_request_callable", _on_flask_blocked_request)
299299

300300

301301
core.on("django.func.wrapped", _on_django_func_wrapped)
302-
core.on("django.wsgi_environ", _on_wsgi_environ)
302+
core.on("django.wsgi_environ", _on_wsgi_environ, "wrapped_result")
303303
core.on("django.patch", _on_django_patch)
304304
core.on("flask.patch", _on_flask_patch)
305305

306-
core.on("asgi.request.parse.body", _on_asgi_request_parse_body)
306+
core.on("asgi.request.parse.body", _on_asgi_request_parse_body, "await_receive_and_body")

ddtrace/appsec/_metrics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def _set_waf_init_metric(info):
7171

7272
def _set_waf_request_metrics(*args):
7373
try:
74-
list_results, list_result_info, list_is_blocked = _asm_request_context.get_waf_results()
74+
list_results, list_result_info, list_is_blocked = _asm_request_context.get_waf_results() or ([], [], [])
7575
if any((list_results, list_result_info, list_is_blocked)):
7676
is_blocked = any(list_is_blocked)
7777
is_triggered = any((result.data for result in list_results))

ddtrace/appsec/_trace_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,4 +339,4 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever):
339339

340340

341341
core.on("django.login", _on_django_login)
342-
core.on("django.auth", _on_django_auth)
342+
core.on("django.auth", _on_django_auth, "user")

ddtrace/contrib/asgi/middleware.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ async def __call__(self, scope, receive, send):
194194
if not self.integration_config.trace_query_string:
195195
query_string = None
196196
body = None
197-
parse_body_result = core.dispatch_with_results("asgi.request.parse.body", (receive, headers))[0]
198-
if len(parse_body_result) == 1:
199-
receive, body = await parse_body_result[0]
197+
result = core.dispatch_with_results("asgi.request.parse.body", (receive, headers)).await_receive_and_body
198+
if result:
199+
receive, body = await result.value
200200

201201
client = scope.get("client")
202202
if isinstance(client, list) and len(client):
@@ -252,7 +252,11 @@ async def wrapped_send(message):
252252
span.finish()
253253

254254
async def wrapped_blocked_send(message):
255-
status, headers, content = core.dispatch_with_results("asgi.block.started", (ctx, url))[0][0]
255+
result = core.dispatch_with_results("asgi.block.started", (ctx, url)).status_headers_content
256+
if result:
257+
status, headers, content = result.value
258+
else:
259+
status, headers, content = 403, [], ""
256260
if span and message.get("type") == "http.response.start":
257261
message["headers"] = headers
258262
message["status"] = int(status)

ddtrace/contrib/django/patch.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -724,9 +724,9 @@ def traced_authenticate(django, pin, func, instance, args, kwargs):
724724
pin,
725725
_DjangoUserInfoRetriever(result_user),
726726
),
727-
)[0]
728-
if result and result[0][0]:
729-
return result[0][1]
727+
).user
728+
if result and result.value[0]:
729+
return result.value[1]
730730

731731
except Exception:
732732
log.debug("Error while trying to trace Django authenticate", exc_info=True)
@@ -822,7 +822,7 @@ def _(m):
822822

823823

824824
def wrap_wsgi_environ(wrapped, _instance, args, kwargs):
825-
return core.dispatch_with_results("django.wsgi_environ", (wrapped, _instance, args, kwargs))[0][0]
825+
return core.dispatch_with_results("django.wsgi_environ", (wrapped, _instance, args, kwargs)).wrapped_result.value
826826

827827

828828
def patch():

ddtrace/contrib/django/utils.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,7 @@ def _extract_body(request):
269269
if request.method in _BODY_METHODS:
270270
req_body = None
271271
content_type = request.content_type if hasattr(request, "content_type") else request.META.get("CONTENT_TYPE")
272-
results = core.dispatch_with_results("django.extract_body", ())[0]
273-
headers = results[0] if results else None
272+
headers = core.dispatch_with_results("django.extract_body").headers.value
274273
try:
275274
if content_type == "application/x-www-form-urlencoded":
276275
req_body = parse_form_params(request.body.decode("UTF-8", errors="ignore"))
@@ -366,8 +365,7 @@ def _after_request_tags(pin, span: Span, request, response):
366365

367366
url = get_request_uri(request)
368367

369-
results = core.dispatch_with_results("django.after_request_headers", ())[0]
370-
request_headers = results[0] if results else None
368+
request_headers = core.dispatch_with_results("django.after_request_headers").headers.value
371369
if not request_headers:
372370
request_headers = _get_request_headers(request)
373371

ddtrace/contrib/flask/patch.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,9 @@ def _wrapped_start_response(self, start_response, ctx, status_code, headers, exc
108108
core.dispatch("flask.start_response.pre", (flask.request, ctx, config.flask, status_code, headers))
109109
if not core.get_item(HTTP_REQUEST_BLOCKED):
110110
headers_from_context = ""
111-
results, exceptions = core.dispatch_with_results("flask.start_response", ("Flask",))
112-
if not any(exceptions) and results and results[0]:
113-
headers_from_context = results[0]
111+
result = core.dispatch_with_results("flask.start_response", ("Flask",)).waf
112+
if result:
113+
headers_from_context = result.value
114114
if core.get_item(HTTP_REQUEST_BLOCKED):
115115
# response code must be set here, or it will be too late
116116
block_config = core.get_item(HTTP_REQUEST_BLOCKED)
@@ -139,7 +139,7 @@ def _request_call_modifier(self, ctx, parsed_headers=None):
139139
request = _RequestType(environ)
140140

141141
req_body = None
142-
results, exceptions = core.dispatch_with_results(
142+
result = core.dispatch_with_results(
143143
"flask.request_call_modifier",
144144
(
145145
ctx,
@@ -151,12 +151,9 @@ def _request_call_modifier(self, ctx, parsed_headers=None):
151151
flask_version_str,
152152
BadRequest,
153153
),
154-
)
155-
if not any(exceptions) and results and any(results):
156-
for result in results:
157-
if result is not None:
158-
req_body = result
159-
break
154+
).request_body
155+
if result:
156+
req_body = result.value
160157
core.dispatch("flask.request_call_modifier.post", (ctx, config.flask, request, req_body))
161158

162159

ddtrace/contrib/flask/wrappers.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ def _wrap_call(
4747
call_key="flask_call",
4848
) as ctx, ctx.get_item("flask_call"):
4949
if do_dispatch:
50-
results, exceptions = core.dispatch_with_results("flask.wrapped_view", (kwargs,))
51-
if results and results[0]:
52-
callback_block, _kwargs = results[0]
50+
result = core.dispatch_with_results("flask.wrapped_view", (kwargs,)).callback_and_args
51+
if result:
52+
callback_block, _kwargs = result.value
5353
if callback_block:
5454
return callback_block()
5555
if _kwargs:

ddtrace/contrib/wsgi/wsgi.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,21 @@ def __call__(self, environ: Iterable, start_response: Callable) -> wrapt.ObjectP
109109
call_key="req_span",
110110
) as ctx:
111111
if core.get_item(HTTP_REQUEST_BLOCKED):
112-
status, headers, content = core.dispatch_with_results("wsgi.block.started", (ctx, construct_url))[0][0]
112+
result = core.dispatch_with_results("wsgi.block.started", (ctx, construct_url)).status_headers_content
113+
if result:
114+
status, headers, content = result.value
115+
else:
116+
status, headers, content = 403, [], ""
113117
start_response(str(status), headers)
114118
closing_iterable = [content]
115119
not_blocked = False
116120

117121
def blocked_view():
118-
status, headers, content = core.dispatch_with_results("wsgi.block.started", (ctx, construct_url))[0][0]
122+
result = core.dispatch_with_results("wsgi.block.started", (ctx, construct_url)).status_headers_content
123+
if result:
124+
status, headers, content = result.value
125+
else:
126+
status, headers, content = 403, [], ""
119127
return content, status, headers
120128

121129
core.dispatch("wsgi.block_decided", (blocked_view,))
@@ -130,12 +138,15 @@ def blocked_view():
130138
else:
131139
core.dispatch("wsgi.app.success", (ctx, closing_iterable))
132140
if core.get_item(HTTP_REQUEST_BLOCKED):
133-
_, _, content = core.dispatch_with_results("wsgi.block.started", (ctx, construct_url))[0][0]
141+
_, _, content = core.dispatch_with_results(
142+
"wsgi.block.started", (ctx, construct_url)
143+
).status_headers_content.value or (None, None, "")
134144
closing_iterable = [content]
135145

136-
return core.dispatch_with_results("wsgi.request.complete", (ctx, closing_iterable, self.app_is_iterator))[
137-
0
138-
][0]
146+
result = core.dispatch_with_results(
147+
"wsgi.request.complete", (ctx, closing_iterable, self.app_is_iterator)
148+
).traced_iterable
149+
return result.value if result else []
139150

140151
def _traced_start_response(self, start_response, request_span, app_span, status, environ, exc_info=None):
141152
# type: (Callable, Span, Span, str, Dict, Any) -> None

0 commit comments

Comments
 (0)