From 8a8add648f6e3039d088e906ab2a833a004f81c7 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 23 Jan 2025 13:53:22 +0100 Subject: [PATCH 01/31] add blocking to django auth --- ddtrace/appsec/_trace_utils.py | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 56f34d81f99..f7711b06cf4 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -357,21 +357,30 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi else: user_id = None - if not result_user: - with pin.tracer.trace("django.contrib.auth.login", span_type=SpanTypes.AUTH): + user_id_found, user_extra = info_retriever.get_user_info( + login=django_config.include_user_login, + email=django_config.include_user_email, + name=django_config.include_user_realname, + ) + if user_extra.get("login") is None: + user_extra["login"] = user_id + user_id = user_id_found or user_id + with pin.tracer.trace("django.contrib.auth.login", span_type=SpanTypes.AUTH): + if not result_user: exists = info_retriever.user_exists() - user_id_found, user_extra = info_retriever.get_user_info( - login=django_config.include_user_login, - email=django_config.include_user_email, - name=django_config.include_user_realname, - ) - if user_extra.get("login") is None: - user_extra["login"] = user_id - user_id = user_id_found or user_id - track_user_login_failure_event( pin.tracer, user_id=user_id, login_events_mode=mode, exists=exists, **user_extra ) + elif in_asm_context(): + res = call_waf_callback( + custom_data={ + "REQUEST_USER_ID": str(user_id) if user_id else None, + "REQUEST_USERNAME": user_extra.get("login"), + }, + force_sent=True, + ) + if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): + raise BlockingException(get_blocked()) return False, None From b27e320d50c9d7587d57c98c9db4191e19d8f534 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 13:36:44 +0100 Subject: [PATCH 02/31] add support for AuthentificationMiddleware to detect and block athenticated users on Django --- ddtrace/appsec/_trace_utils.py | 48 ++++++++++++++++++++++-- ddtrace/contrib/internal/django/patch.py | 25 +++++++++++- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index f7711b06cf4..ee4707aaad3 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -372,12 +372,15 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi pin.tracer, user_id=user_id, login_events_mode=mode, exists=exists, **user_extra ) elif in_asm_context(): - res = call_waf_callback( - custom_data={ + real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode + custom_data={ "REQUEST_USER_ID": str(user_id) if user_id else None, "REQUEST_USERNAME": user_extra.get("login"), - }, - force_sent=True, + "LOGIN_SUCCESS": real_mode, + } + res = call_waf_callback( + custom_data=custom_data, + force_sent=True ) if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): raise BlockingException(get_blocked()) @@ -385,5 +388,42 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi return False, None +def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_config): + if not asm_config._asm_enabled: + return + userid_list = info_retriever.possible_user_id_fields + info_retriever.possible_login_fields + + for possible_key in userid_list: + if possible_key in kwargs: + user_id = kwargs[possible_key] + break + else: + user_id = None + + user_id_found, user_extra = info_retriever.get_user_info( + login=django_config.include_user_login, + email=django_config.include_user_email, + name=django_config.include_user_realname, + ) + if user_extra.get("login") is None: + user_extra["login"] = user_id + user_id = user_id_found or user_id + with pin.tracer.trace("django.contrib.auth.middleware.AuthentificationMiddleware", span_type=SpanTypes.AUTH): + if result_user and in_asm_context(): + real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode + custom_data = { + "REQUEST_USER_ID": str(user_id) if user_id else None, + "REQUEST_USERNAME": user_extra.get("login"), + "LOGIN_SUCCESS": real_mode, + } + res = call_waf_callback( + custom_data=custom_data, + force_sent=True + ) + if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): + raise BlockingException(get_blocked()) + + core.on("django.login", _on_django_login) core.on("django.auth", _on_django_auth, "user") +core.on("django.process_request", _on_django_process,) diff --git a/ddtrace/contrib/internal/django/patch.py b/ddtrace/contrib/internal/django/patch.py index 7378d8da31c..740aa700261 100644 --- a/ddtrace/contrib/internal/django/patch.py +++ b/ddtrace/contrib/internal/django/patch.py @@ -811,9 +811,28 @@ def traced_authenticate(django, pin, func, instance, args, kwargs): return result.value[1] except Exception: log.debug("Error while trying to trace Django authenticate", exc_info=True) - return result_user +@trace_utils.with_traced_module +def traced_process_request(django, pin, func, instance, args, kwargs): + func(*args, **kwargs) + mode = asm_config._user_event_mode + if mode == "disabled": + return + try: + request = get_argument_value(args, kwargs, 0, "request") + if request: + if hasattr(request, "user") and hasattr(request.user, "_setup"): + request.user._setup() + request_user = request.user._wrapped + else: + request_user = request.user + core.dispatch( + "django.process_request", + (request_user, mode, kwargs, pin, _DjangoUserInfoRetriever(request_user, credentials=kwargs), config.django), + ) + except Exception: + log.debug("Error while trying to trace Django AuthenticationMiddleware process_request", exc_info=True) def unwrap_views(func, instance, args, kwargs): """ @@ -867,6 +886,10 @@ def _(m): trace_utils.wrap(m, "login", traced_login(django)) trace_utils.wrap(m, "authenticate", traced_authenticate(django)) + @when_imported("django.contrib.auth.middleware") + def _(m): + trace_utils.wrap(m, "AuthenticationMiddleware.process_request", traced_process_request(django)) + # Only wrap get_asgi_application if get_response_async exists. Otherwise we will effectively double-patch # because get_response and get_asgi_application will be used. We must rely on the version instead of coalescing # with the previous patching hook because of circular imports within `django.core.asgi`. From 3ee130c542101585e0944d2bdaad4bdc47e28265 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 13:42:19 +0100 Subject: [PATCH 03/31] lint --- ddtrace/appsec/_trace_utils.py | 33 +++++++++++------------- ddtrace/contrib/internal/django/patch.py | 13 ++++++++-- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index ee4707aaad3..0eb69b4f799 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -373,15 +373,12 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi ) elif in_asm_context(): real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode - custom_data={ - "REQUEST_USER_ID": str(user_id) if user_id else None, - "REQUEST_USERNAME": user_extra.get("login"), - "LOGIN_SUCCESS": real_mode, - } - res = call_waf_callback( - custom_data=custom_data, - force_sent=True - ) + custom_data = { + "REQUEST_USER_ID": str(user_id) if user_id else None, + "REQUEST_USERNAME": user_extra.get("login"), + "LOGIN_SUCCESS": real_mode, + } + res = call_waf_callback(custom_data=custom_data, force_sent=True) if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): raise BlockingException(get_blocked()) @@ -412,18 +409,18 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co if result_user and in_asm_context(): real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { - "REQUEST_USER_ID": str(user_id) if user_id else None, - "REQUEST_USERNAME": user_extra.get("login"), - "LOGIN_SUCCESS": real_mode, - } - res = call_waf_callback( - custom_data=custom_data, - force_sent=True - ) + "REQUEST_USER_ID": str(user_id) if user_id else None, + "REQUEST_USERNAME": user_extra.get("login"), + "LOGIN_SUCCESS": real_mode, + } + res = call_waf_callback(custom_data=custom_data, force_sent=True) if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): raise BlockingException(get_blocked()) core.on("django.login", _on_django_login) core.on("django.auth", _on_django_auth, "user") -core.on("django.process_request", _on_django_process,) +core.on( + "django.process_request", + _on_django_process, +) diff --git a/ddtrace/contrib/internal/django/patch.py b/ddtrace/contrib/internal/django/patch.py index 740aa700261..bf2e0ecc9de 100644 --- a/ddtrace/contrib/internal/django/patch.py +++ b/ddtrace/contrib/internal/django/patch.py @@ -813,6 +813,7 @@ def traced_authenticate(django, pin, func, instance, args, kwargs): log.debug("Error while trying to trace Django authenticate", exc_info=True) return result_user + @trace_utils.with_traced_module def traced_process_request(django, pin, func, instance, args, kwargs): func(*args, **kwargs) @@ -829,11 +830,19 @@ def traced_process_request(django, pin, func, instance, args, kwargs): request_user = request.user core.dispatch( "django.process_request", - (request_user, mode, kwargs, pin, _DjangoUserInfoRetriever(request_user, credentials=kwargs), config.django), - ) + ( + request_user, + mode, + kwargs, + pin, + _DjangoUserInfoRetriever(request_user, credentials=kwargs), + config.django, + ), + ) except Exception: log.debug("Error while trying to trace Django AuthenticationMiddleware process_request", exc_info=True) + def unwrap_views(func, instance, args, kwargs): """ Django channels uses path() and re_path() to route asgi applications. This broke our initial From 178feb77d54c73987ffa856cc7cb1e3b5478ea06 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 13:52:47 +0100 Subject: [PATCH 04/31] ensure authentification --- ddtrace/appsec/_trace_utils.py | 4 ++-- tests/appsec/contrib_appsec/db.sqlite3 | Bin 602112 -> 634880 bytes 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 0eb69b4f799..aadf6e39e4e 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -405,8 +405,8 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co if user_extra.get("login") is None: user_extra["login"] = user_id user_id = user_id_found or user_id - with pin.tracer.trace("django.contrib.auth.middleware.AuthentificationMiddleware", span_type=SpanTypes.AUTH): - if result_user and in_asm_context(): + if result_user and in_asm_context() and result_user.is_authenticated: + with pin.tracer.trace("django.contrib.auth.middleware.AuthentificationMiddleware", span_type=SpanTypes.AUTH): real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { "REQUEST_USER_ID": str(user_id) if user_id else None, diff --git a/tests/appsec/contrib_appsec/db.sqlite3 b/tests/appsec/contrib_appsec/db.sqlite3 index de7010cb03fd747185dcc8ca9f01204605cd37e2..158357614cc5695922c154ccc6b50e0965ddf7a2 100644 GIT binary patch delta 29083 zcmd^|$&+VURmZC-x@n-gp@kMjz($%$Bhxz%8KFVWmGhjHnW*KP=kdN7r4bgaDnmqR zkucN+E8P$sVMn-fD_fUt*c&eV54vKr*BJSxkcUC(cAw3AAaI3c&O|%v|q)1t=W4Ttr!0K^^>gwo7!hv zVP@Bc=@{w=CRZ5@c83`H_NA3vT<`UJV53Osj=xWDaJI`a>ud*8s^Rx8kf@L<_ zJFaTqyzx`BeM;<&t@UlwfxBl%rBQFNJ#uob*52yP?zk#iJRP5I&I;>>UF%mmmVE2dFJ7I zx!Lg5jV+jF(`Ss903SbNB3GbTCW7gYSd%*V;vK4fNPUP{6Cq;&yz=uX?NJivrV7$B zRLMO?g(2a1M72Mm%Hp($eFVA`IGu_}ADN+jk-h?X5Eu)NZ z-?Uy{(@>;oWLCLINE=zS^o#kTvRBU8X0pCjor2+#*w;4PTi-qut}w3P)#}pv#$}k@ zJoVziidQe5KY)B$LQV<&Hh#AI?(1Lt@PB^f2jJnxm!Ez8?l4AfO`#k4G7sYfcc4gZ z)3d84H!=@qsGcu2Z)LCO+oW8*QkPDCxn+9n}IksRT0EGS*yl3TN_f}x69HirH>;!00p|os5oZ`kB9MkLS^H=E3vGI zOkC7eHa46B)d@)K)Nm=C@nKxps&u@Au+*roCRB}sVCC~7vKpFoGNVFvgYOaBHroi; z_t~njBm`iM>dA!4M84~%VN#XS@=53AJvCx>RxKT5K8jjzBREX=P6Nw&?goT%SzyG)i0XVo z6_O02Ha7ew@lu}2wg(9o6vV#S`JG$dV^j?3fOPa#XA>$ULd43Bn?qA7bEslbl$D)= z0@e{!9>2$^2$fJWjq3CZcOht%7qnIsSqep&Wn=|a!yrP7kJE2NI zivpb-rkf|$Qn?p-kM~?wIOmXGjh9ZK2 zk){eX0?Ve4YFrGcB`mwgs5r$Gjp3Cwp>kZ^sYq+ldY%Oa($gGSEwF}ML>Uds{vM;E zQdiqYw{A|TbOfJR=D@VSs{7i=O`E7eACA3!Xi?q_OL#)K+{%n~K6JW_NF z4<(CBGc|M|U&K}&JK`Q&6~y}QcmV%+LKR46OULsJH-_Bt5}qPIMu9mX7Y;t2u~qpi z<>A`;y0d?toUlF5I_MvJJ7mv=2hsM1@%YT!-U;H^ALIp$qUYl(xGWLIVzjEKZ%%C+ z6uE}ic5c-bfRwtV?4~NSi>PBHZRp#74Xnl_JOcoA_ zagbH1O*GH8VdW8?u852uZa>GZb;W0nB+1TOx;o1JV+-d5Yi{Si3+3UmV zR(*N7c92%*w0+*Wvt=Y0(66$7Cu31m8LpR9tvMWTj>O(+0mPE@5Rag=1ln%6EXgN*|tgQBZs_T9+ zG))oB*t;)`iUZE%=-rD6RR&UqjM#4dupy>h_f^kmN3A&nHwi za@~$qtr0Qhce=%BfM^lHmV#RG)~zhF%?Z3&ey7*5b@xQ#%c-508rd2w3s%lf&(;>tB)yl4Zf6u3KqT$%M*NwcmmybR!fr z#Kko;8qUhTYIV_=bu*5u7DmMc6k^i9YR0L>wsLpS0A55rxzpr{?##OD_+;gP^n4>C+vFuUo z_w&t~7e}Qz0XbfTZ%wFTOJFB+MPO#CC|e&niBD}K4@CpKx`Frj00N*hoUteI%?VZ3 zd%{f{(pwx?Q6~I2QzeB`#YzVa^?bd%WGgA(Z%}`5qjh9`(b#9#2T$D9t&6>@C+EsM zGuY`dKiWAywwJ<#=Y5q}=DHL^$4K-~-}6;YYS5}N4BT`^8i$=9)Lrju@0Yodq;kGi zwJ<6wInl#P=-Ho4s0hnj8QPsGsO~R!f$KWJ4YZDEXa~aOeETjR{qt ztHx%YAW*nbtXnx84`B#u6;`brKcNy?mRgm@$7|@}Oz3IeR+%vlVzV#YY{vP`!l<;67Tpy? zy9pJ|wEoPC+b%YBQJr*7SK+wDLVGOU3v0f?0$;dsqo+i)*h5=YJE0u}B4LT3p*V25W$1Ba7Uw?AHc5^wxMS8!aoftE3mx-qCz!B;2aGW^^bk}$B6 zsF!+EMP61ZWx1+#p}o_ZhkJ}l*ZJ-WeYdYnsFGnYj3{sXHu1vV9tJybp>MjKuNM)x z#F;NDxpGDDe)Yt3j+DICQ`#qc<)%7XYmSo3sBt53)&+a+>Qnc0ji{fG8Ahwp@-)o( zomD-WQ2CjlVU+47pjko7@rNi3VO#c6f`B#sd zB}h*OQV^uIN!zx`ov5&(G<=d2Lv436z5&IGyJjhVVkcXBt2;OJYWo5&orIQkF0kv^ zC%Wou#hXB_daKE}gX?{%;ON%DvLINB(W*W)sg7%fTBdqWrsZIU9gl(_t{qIQUJiDt zW~(}u3zO2t1&GG5>ZQr3MB3N3?ZjHJ;M-0ZL@A3@%eb8msn%f4xD>N6Ds57CQ_Z(8 zzK1I7luL>-wyLTuTC>0)_mFIgFsh*i=9>vFZdC|s(f@b>esDrn>U<{caqBQMi^87s zGA|v&Eh9#)D(>bxO}TjAM(Pqf9J_%JCscK3<#lCgfq#;ETqZ+Cpl`IMqcZFy&U^(7 zi=z^N2*lr)G=JtUX>O^d{5rO?+8yUlZB=<)21!$;*wt$WW*oDxudJQ+L1-MC?bVW> zp6&v_@scOeH9R+V;@#aPi*4@gtgT)Z*Yp?mpp5q*(VM{Zf-BwWy8VsGxG0NlPjuN} zw~lZ#)^|;SL6+IwP*;^Z%r_8S7?t){ciVMtKRu!12={@b`&v|*kkws5#1IWR(H%il zD0QDvQGv9uUt86$PpAf?GaI2rL!Kg_HqxqSyu#D%RFRv~oAHE%T-d6xWJ2mr*X^%O zs64$kXkNK*O7e@t|t}u)-`==&UpvrUI77PTCk>Nw- z=P9Qx26n0&J^HQl^;K&tYhd@u_Gzj|(~2Xn?v4&m3RzvBak0DWl-uj8hj~V}!#b;w z_k4PTm!%e6-PIRU?!u7h4DI7#PZBAW3HxG*Yx{mDB8(d!c` zock3grrz#R8VN1?mc(6NE1)c^Hk^9C%PQo;s3;#73+My*$qCg^Frz1hCjwzYUHfE4 z+3Br?rQwjS#{s&>2XJgO{_0uQT@tq-(mufV4)9i|U-txNN*|i%YMRr*{Ru*Ctd!+v#b* z*a=!%B8L-7Ez=RcXM3TQreeP9n&iT$I3sLa*L3^w300hBII;Bdc{@toT+$(Ppzbnf zWseh}2fOC$s}@E@b&CtVzt#NJyR6wlB&I0^*tdJB!o1*q*S59tqy@~znz1}>YsEjm zzS%exIH?S>SM2n4J{(-1;HK%*u3bVV-=OKbUI_}%eqj~Sm^JfdrbSnyhUlR3`Jq;H8!88;qeO$a_!k zV~<3fB~4mXGmb>WoJOXusd7DfdC zq2t=L+b>S2NFQS9`zmN4Y*CxACW|UmDj1fwtcG;Hs5VzDPVJ!%4sQZqFQqb#;m}O4 zinI#WpqmFCO1g`i<0E?s_0JEJP_``eS0s$9R&U>&I$a6P1LkB^t{iNn9gBK_ESM#V z>8;w-nQy0SVNz7@&biYsd|@&wH3XjiHks57QAcE04mi^V86=`mHb!mDSQWD{DxzH# z8KbM;notEM=XQqd+J-_e9XDZ@Mu`^kwF&jIs-5qGIk_+@sB1BBj1@jVq0$%M0Gz@$ zX_2L#FRHkXbO$w4Rbte2IpeTuVN?K8Fm7$SePcq^LvDC(P&vWi>X;!(Vk@*QBY<_# zqK?jY)v?@I3AfIZ`pJ57xUQNo+(XszTK`mT@{_$Y0#=*z!}RDZh| zL1;wvn-i)shhgC`*QSk0BNg<0M;w6dy3JMT4>K;sER0I;uSerj_S?@*s8Ef=mb)+r zTVI+W;I)^S9`7J4VDCEhd}D=$QR!e^PeW)_pMCED8kLdNCgRu#4crg3mXTR!j-6Ga z2YANyg@sXRqayS!X@2R2SAY88)t|oe+8h7+>Q^+o-@eP9RpaC-OXJKB9n`BVl7{E# cl^->=sVv+77h%>n{tsW8w|GV<0H9p{54TC@rTf*EiqOG`G~l z&^#=_JkwMq&BfF>t0*I>EHSqtG~L3itRf>dHN_w$q{PwEvamGIvLdatAjj3xGs4x@ z$iT=%*T7uYz)-=^z{=Fz%G6NL%);2n*cc*XrfX!PU|?ZoY-D9pjE{k!p k3NrAWu0OT=Cvj6}9 From cfd1d1e6ec0a3bf7dd5370b9d527651117e47675 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 14:04:42 +0100 Subject: [PATCH 05/31] remove additional tracing --- ddtrace/appsec/_trace_utils.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index aadf6e39e4e..836f8739435 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -406,16 +406,15 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co user_extra["login"] = user_id user_id = user_id_found or user_id if result_user and in_asm_context() and result_user.is_authenticated: - with pin.tracer.trace("django.contrib.auth.middleware.AuthentificationMiddleware", span_type=SpanTypes.AUTH): - real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode - custom_data = { - "REQUEST_USER_ID": str(user_id) if user_id else None, - "REQUEST_USERNAME": user_extra.get("login"), - "LOGIN_SUCCESS": real_mode, - } - res = call_waf_callback(custom_data=custom_data, force_sent=True) - if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): - raise BlockingException(get_blocked()) + real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode + custom_data = { + "REQUEST_USER_ID": str(user_id) if user_id else None, + "REQUEST_USERNAME": user_extra.get("login"), + "LOGIN_SUCCESS": real_mode, + } + res = call_waf_callback(custom_data=custom_data, force_sent=True) + if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): + raise BlockingException(get_blocked()) core.on("django.login", _on_django_login) From b05361f3d52b8e0e6dd55d9a5ac21b45f17b1188 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 14:37:35 +0100 Subject: [PATCH 06/31] restore django_auth --- ddtrace/appsec/_trace_utils.py | 36 +++++++------------ .../django_tests/test_django_appsec.py | 2 +- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 836f8739435..2ffdcd915ac 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -357,30 +357,20 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi else: user_id = None - user_id_found, user_extra = info_retriever.get_user_info( - login=django_config.include_user_login, - email=django_config.include_user_email, - name=django_config.include_user_realname, - ) - if user_extra.get("login") is None: - user_extra["login"] = user_id - user_id = user_id_found or user_id - with pin.tracer.trace("django.contrib.auth.login", span_type=SpanTypes.AUTH): - if not result_user: + if not result_user: + with pin.tracer.trace("django.contrib.auth.login", span_type=SpanTypes.AUTH): + user_id_found, user_extra = info_retriever.get_user_info( + login=django_config.include_user_login, + email=django_config.include_user_email, + name=django_config.include_user_realname, + ) + if user_extra.get("login") is None: + user_extra["login"] = user_id + user_id = user_id_found or user_id exists = info_retriever.user_exists() track_user_login_failure_event( pin.tracer, user_id=user_id, login_events_mode=mode, exists=exists, **user_extra ) - elif in_asm_context(): - real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode - custom_data = { - "REQUEST_USER_ID": str(user_id) if user_id else None, - "REQUEST_USERNAME": user_extra.get("login"), - "LOGIN_SUCCESS": real_mode, - } - res = call_waf_callback(custom_data=custom_data, force_sent=True) - if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): - raise BlockingException(get_blocked()) return False, None @@ -406,6 +396,7 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co user_extra["login"] = user_id user_id = user_id_found or user_id if result_user and in_asm_context() and result_user.is_authenticated: + set_user(pin.tracer, str(user_id), propagate=True, **user_extra) real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { "REQUEST_USER_ID": str(user_id) if user_id else None, @@ -419,7 +410,4 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co core.on("django.login", _on_django_login) core.on("django.auth", _on_django_auth, "user") -core.on( - "django.process_request", - _on_django_process, -) +# core.on("django.process_request", _on_django_process) diff --git a/tests/appsec/integrations/django_tests/test_django_appsec.py b/tests/appsec/integrations/django_tests/test_django_appsec.py index e8515a75a2c..41540a375d9 100644 --- a/tests/appsec/integrations/django_tests/test_django_appsec.py +++ b/tests/appsec/integrations/django_tests/test_django_appsec.py @@ -186,7 +186,7 @@ def test_django_login_sucess_identification(client, test_spans, tracer, use_logi assert get_user(client).is_authenticated login_span = test_spans.find_span(name="django.contrib.auth.login") assert login_span - assert login_span.get_tag(user.ID) == "1" + assert login_span.get_tag(user.ID) == "1", login_span.get_tag(user.ID) assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".success.track") == "true" assert login_span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_SUCCESS_MODE) == LOGIN_EVENTS_MODE.IDENT if use_login: From be666323add3def4610b4a7f9e2bb0768c89f3ea Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 14:42:06 +0100 Subject: [PATCH 07/31] small restore --- ddtrace/appsec/_trace_utils.py | 2 +- tests/appsec/contrib_appsec/db.sqlite3 | Bin 634880 -> 602112 bytes 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 2ffdcd915ac..8d91e3ddb94 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -359,6 +359,7 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi if not result_user: with pin.tracer.trace("django.contrib.auth.login", span_type=SpanTypes.AUTH): + exists = info_retriever.user_exists() user_id_found, user_extra = info_retriever.get_user_info( login=django_config.include_user_login, email=django_config.include_user_email, @@ -367,7 +368,6 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi if user_extra.get("login") is None: user_extra["login"] = user_id user_id = user_id_found or user_id - exists = info_retriever.user_exists() track_user_login_failure_event( pin.tracer, user_id=user_id, login_events_mode=mode, exists=exists, **user_extra ) diff --git a/tests/appsec/contrib_appsec/db.sqlite3 b/tests/appsec/contrib_appsec/db.sqlite3 index 158357614cc5695922c154ccc6b50e0965ddf7a2..de7010cb03fd747185dcc8ca9f01204605cd37e2 100644 GIT binary patch delta 252 zcmZp8pw{p}Wr7r&NH+rm!{mtyb|98s-mZ-)yJYLljSPT5rNXq>C@rTf*EiqOG`G~l z&^#=_JkwMq&BfF>t0*I>EHSqtG~L3itRf>dHN_w$q{PwEvamGIvLdatAjj3xGs4x@ z$iT=%*T7uYz)-=^z{=Fz%G6NL%);2n*cc*XrfX!PU|?ZoY-D9pjE{k!p k3NrAWu0OT=Cvj6}9 delta 29083 zcmd^|$&+VURmZC-x@n-gp@kMjz($%$Bhxz%8KFVWmGhjHnW*KP=kdN7r4bgaDnmqR zkucN+E8P$sVMn-fD_fUt*c&eV54vKr*BJSxkcUC(cAw3AAaI3c&O|%v|q)1t=W4Ttr!0K^^>gwo7!hv zVP@Bc=@{w=CRZ5@c83`H_NA3vT<`UJV53Osj=xWDaJI`a>ud*8s^Rx8kf@L<_ zJFaTqyzx`BeM;<&t@UlwfxBl%rBQFNJ#uob*52yP?zk#iJRP5I&I;>>UF%mmmVE2dFJ7I zx!Lg5jV+jF(`Ss903SbNB3GbTCW7gYSd%*V;vK4fNPUP{6Cq;&yz=uX?NJivrV7$B zRLMO?g(2a1M72Mm%Hp($eFVA`IGu_}ADN+jk-h?X5Eu)NZ z-?Uy{(@>;oWLCLINE=zS^o#kTvRBU8X0pCjor2+#*w;4PTi-qut}w3P)#}pv#$}k@ zJoVziidQe5KY)B$LQV<&Hh#AI?(1Lt@PB^f2jJnxm!Ez8?l4AfO`#k4G7sYfcc4gZ z)3d84H!=@qsGcu2Z)LCO+oW8*QkPDCxn+9n}IksRT0EGS*yl3TN_f}x69HirH>;!00p|os5oZ`kB9MkLS^H=E3vGI zOkC7eHa46B)d@)K)Nm=C@nKxps&u@Au+*roCRB}sVCC~7vKpFoGNVFvgYOaBHroi; z_t~njBm`iM>dA!4M84~%VN#XS@=53AJvCx>RxKT5K8jjzBREX=P6Nw&?goT%SzyG)i0XVo z6_O02Ha7ew@lu}2wg(9o6vV#S`JG$dV^j?3fOPa#XA>$ULd43Bn?qA7bEslbl$D)= z0@e{!9>2$^2$fJWjq3CZcOht%7qnIsSqep&Wn=|a!yrP7kJE2NI zivpb-rkf|$Qn?p-kM~?wIOmXGjh9ZK2 zk){eX0?Ve4YFrGcB`mwgs5r$Gjp3Cwp>kZ^sYq+ldY%Oa($gGSEwF}ML>Uds{vM;E zQdiqYw{A|TbOfJR=D@VSs{7i=O`E7eACA3!Xi?q_OL#)K+{%n~K6JW_NF z4<(CBGc|M|U&K}&JK`Q&6~y}QcmV%+LKR46OULsJH-_Bt5}qPIMu9mX7Y;t2u~qpi z<>A`;y0d?toUlF5I_MvJJ7mv=2hsM1@%YT!-U;H^ALIp$qUYl(xGWLIVzjEKZ%%C+ z6uE}ic5c-bfRwtV?4~NSi>PBHZRp#74Xnl_JOcoA_ zagbH1O*GH8VdW8?u852uZa>GZb;W0nB+1TOx;o1JV+-d5Yi{Si3+3UmV zR(*N7c92%*w0+*Wvt=Y0(66$7Cu31m8LpR9tvMWTj>O(+0mPE@5Rag=1ln%6EXgN*|tgQBZs_T9+ zG))oB*t;)`iUZE%=-rD6RR&UqjM#4dupy>h_f^kmN3A&nHwi za@~$qtr0Qhce=%BfM^lHmV#RG)~zhF%?Z3&ey7*5b@xQ#%c-508rd2w3s%lf&(;>tB)yl4Zf6u3KqT$%M*NwcmmybR!fr z#Kko;8qUhTYIV_=bu*5u7DmMc6k^i9YR0L>wsLpS0A55rxzpr{?##OD_+;gP^n4>C+vFuUo z_w&t~7e}Qz0XbfTZ%wFTOJFB+MPO#CC|e&niBD}K4@CpKx`Frj00N*hoUteI%?VZ3 zd%{f{(pwx?Q6~I2QzeB`#YzVa^?bd%WGgA(Z%}`5qjh9`(b#9#2T$D9t&6>@C+EsM zGuY`dKiWAywwJ<#=Y5q}=DHL^$4K-~-}6;YYS5}N4BT`^8i$=9)Lrju@0Yodq;kGi zwJ<6wInl#P=-Ho4s0hnj8QPsGsO~R!f$KWJ4YZDEXa~aOeETjR{qt ztHx%YAW*nbtXnx84`B#u6;`brKcNy?mRgm@$7|@}Oz3IeR+%vlVzV#YY{vP`!l<;67Tpy? zy9pJ|wEoPC+b%YBQJr*7SK+wDLVGOU3v0f?0$;dsqo+i)*h5=YJE0u}B4LT3p*V25W$1Ba7Uw?AHc5^wxMS8!aoftE3mx-qCz!B;2aGW^^bk}$B6 zsF!+EMP61ZWx1+#p}o_ZhkJ}l*ZJ-WeYdYnsFGnYj3{sXHu1vV9tJybp>MjKuNM)x z#F;NDxpGDDe)Yt3j+DICQ`#qc<)%7XYmSo3sBt53)&+a+>Qnc0ji{fG8Ahwp@-)o( zomD-WQ2CjlVU+47pjko7@rNi3VO#c6f`B#sd zB}h*OQV^uIN!zx`ov5&(G<=d2Lv436z5&IGyJjhVVkcXBt2;OJYWo5&orIQkF0kv^ zC%Wou#hXB_daKE}gX?{%;ON%DvLINB(W*W)sg7%fTBdqWrsZIU9gl(_t{qIQUJiDt zW~(}u3zO2t1&GG5>ZQr3MB3N3?ZjHJ;M-0ZL@A3@%eb8msn%f4xD>N6Ds57CQ_Z(8 zzK1I7luL>-wyLTuTC>0)_mFIgFsh*i=9>vFZdC|s(f@b>esDrn>U<{caqBQMi^87s zGA|v&Eh9#)D(>bxO}TjAM(Pqf9J_%JCscK3<#lCgfq#;ETqZ+Cpl`IMqcZFy&U^(7 zi=z^N2*lr)G=JtUX>O^d{5rO?+8yUlZB=<)21!$;*wt$WW*oDxudJQ+L1-MC?bVW> zp6&v_@scOeH9R+V;@#aPi*4@gtgT)Z*Yp?mpp5q*(VM{Zf-BwWy8VsGxG0NlPjuN} zw~lZ#)^|;SL6+IwP*;^Z%r_8S7?t){ciVMtKRu!12={@b`&v|*kkws5#1IWR(H%il zD0QDvQGv9uUt86$PpAf?GaI2rL!Kg_HqxqSyu#D%RFRv~oAHE%T-d6xWJ2mr*X^%O zs64$kXkNK*O7e@t|t}u)-`==&UpvrUI77PTCk>Nw- z=P9Qx26n0&J^HQl^;K&tYhd@u_Gzj|(~2Xn?v4&m3RzvBak0DWl-uj8hj~V}!#b;w z_k4PTm!%e6-PIRU?!u7h4DI7#PZBAW3HxG*Yx{mDB8(d!c` zock3grrz#R8VN1?mc(6NE1)c^Hk^9C%PQo;s3;#73+My*$qCg^Frz1hCjwzYUHfE4 z+3Br?rQwjS#{s&>2XJgO{_0uQT@tq-(mufV4)9i|U-txNN*|i%YMRr*{Ru*Ctd!+v#b* z*a=!%B8L-7Ez=RcXM3TQreeP9n&iT$I3sLa*L3^w300hBII;Bdc{@toT+$(Ppzbnf zWseh}2fOC$s}@E@b&CtVzt#NJyR6wlB&I0^*tdJB!o1*q*S59tqy@~znz1}>YsEjm zzS%exIH?S>SM2n4J{(-1;HK%*u3bVV-=OKbUI_}%eqj~Sm^JfdrbSnyhUlR3`Jq;H8!88;qeO$a_!k zV~<3fB~4mXGmb>WoJOXusd7DfdC zq2t=L+b>S2NFQS9`zmN4Y*CxACW|UmDj1fwtcG;Hs5VzDPVJ!%4sQZqFQqb#;m}O4 zinI#WpqmFCO1g`i<0E?s_0JEJP_``eS0s$9R&U>&I$a6P1LkB^t{iNn9gBK_ESM#V z>8;w-nQy0SVNz7@&biYsd|@&wH3XjiHks57QAcE04mi^V86=`mHb!mDSQWD{DxzH# z8KbM;notEM=XQqd+J-_e9XDZ@Mu`^kwF&jIs-5qGIk_+@sB1BBj1@jVq0$%M0Gz@$ zX_2L#FRHkXbO$w4Rbte2IpeTuVN?K8Fm7$SePcq^LvDC(P&vWi>X;!(Vk@*QBY<_# zqK?jY)v?@I3AfIZ`pJ57xUQNo+(XszTK`mT@{_$Y0#=*z!}RDZh| zL1;wvn-i)shhgC`*QSk0BNg<0M;w6dy3JMT4>K;sER0I;uSerj_S?@*s8Ef=mb)+r zTVI+W;I)^S9`7J4VDCEhd}D=$QR!e^PeW)_pMCED8kLdNCgRu#4crg3mXTR!j-6Ga z2YANyg@sXRqayS!X@2R2SAY88)t|oe+8h7+>Q^+o-@eP9RpaC-OXJKB9n`BVl7{E# cl^->=sVv+77h%>n{tsW8w|GV<0H9p{54T Date: Fri, 24 Jan 2025 14:44:05 +0100 Subject: [PATCH 08/31] reenable listener --- ddtrace/appsec/_trace_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 8d91e3ddb94..edf227e10a6 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -368,6 +368,7 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi if user_extra.get("login") is None: user_extra["login"] = user_id user_id = user_id_found or user_id + track_user_login_failure_event( pin.tracer, user_id=user_id, login_events_mode=mode, exists=exists, **user_extra ) @@ -410,4 +411,4 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co core.on("django.login", _on_django_login) core.on("django.auth", _on_django_auth, "user") -# core.on("django.process_request", _on_django_process) +core.on("django.process_request", _on_django_process) From 88fa8248a19002b06235d6bb2cbc3d2d031cb87f Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 15:31:17 +0100 Subject: [PATCH 09/31] keep apm tools --- ddtrace/contrib/internal/django/patch.py | 68 +++++++++++++++--------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/ddtrace/contrib/internal/django/patch.py b/ddtrace/contrib/internal/django/patch.py index f9690802124..9a4e2c0d620 100644 --- a/ddtrace/contrib/internal/django/patch.py +++ b/ddtrace/contrib/internal/django/patch.py @@ -833,31 +833,49 @@ def traced_authenticate(django, pin, func, instance, args, kwargs): @trace_utils.with_traced_module def traced_process_request(django, pin, func, instance, args, kwargs): - func(*args, **kwargs) - mode = asm_config._user_event_mode - if mode == "disabled": - return - try: - request = get_argument_value(args, kwargs, 0, "request") - if request: - if hasattr(request, "user") and hasattr(request.user, "_setup"): - request.user._setup() - request_user = request.user._wrapped - else: - request_user = request.user - core.dispatch( - "django.process_request", - ( - request_user, - mode, - kwargs, - pin, - _DjangoUserInfoRetriever(request_user, credentials=kwargs), - config.django, - ), - ) - except Exception: - log.debug("Error while trying to trace Django AuthenticationMiddleware process_request", exc_info=True) + tags = {COMPONENT: config.django.integration_name} + with core.context_with_data( + "django.func.wrapped", + span_name="django.middleware", + resource="django.contrib.auth.middleware.AuthenticationMiddleware.process_request", + tags=tags, + pin=pin, + ) as ctx, ctx.span: + core.dispatch( + "django.func.wrapped", + ( + args, + kwargs, + django.core.handlers.wsgi.WSGIRequest if hasattr(django.core.handlers, "wsgi") else object, + ctx, + None, + ), + ) + func(*args, **kwargs) + mode = asm_config._user_event_mode + if mode == "disabled": + return + try: + request = get_argument_value(args, kwargs, 0, "request") + if request: + if hasattr(request, "user") and hasattr(request.user, "_setup"): + request.user._setup() + request_user = request.user._wrapped + else: + request_user = request.user + core.dispatch( + "django.process_request", + ( + request_user, + mode, + kwargs, + pin, + _DjangoUserInfoRetriever(request_user, credentials=kwargs), + config.django, + ), + ) + except Exception: + log.debug("Error while trying to trace Django AuthenticationMiddleware process_request", exc_info=True) def unwrap_views(func, instance, args, kwargs): From 3d5e91a07a0036591bae49ad37a4a7858ecae7e9 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 15:32:31 +0100 Subject: [PATCH 10/31] revert line --- ddtrace/contrib/internal/django/patch.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ddtrace/contrib/internal/django/patch.py b/ddtrace/contrib/internal/django/patch.py index 9a4e2c0d620..ba96675fef3 100644 --- a/ddtrace/contrib/internal/django/patch.py +++ b/ddtrace/contrib/internal/django/patch.py @@ -828,6 +828,7 @@ def traced_authenticate(django, pin, func, instance, args, kwargs): return result.value[1] except Exception: log.debug("Error while trying to trace Django authenticate", exc_info=True) + return result_user From db211918b82665676d7ccadb8b95bc2b16dfe7cf Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 16:46:43 +0100 Subject: [PATCH 11/31] fix set_user --- ddtrace/appsec/_trace_utils.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index edf227e10a6..61cb6f85e9f 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -377,6 +377,9 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_config): + import sys + + print(f">>> on_django_process: {result_user}, {result_user.is_authenticated}, {in_asm_context()}, {asm_config._asm_enabled}, {mode}, {kwargs}, {pin}, {info_retriever}, {django_config}", file=sys.stderr, flush=True) if not asm_config._asm_enabled: return userid_list = info_retriever.possible_user_id_fields + info_retriever.possible_login_fields @@ -397,7 +400,13 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co user_extra["login"] = user_id user_id = user_id_found or user_id if result_user and in_asm_context() and result_user.is_authenticated: - set_user(pin.tracer, str(user_id), propagate=True, **user_extra) + try: + if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): + set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True) + elif mode == LOGIN_EVENTS_MODE.IDENT: + set_user(pin.tracer, str(user_id), propagate=True, email=user_extra.get("email"), name=user_extra.get("name")) + except Exception as e: # nosec + pass real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { "REQUEST_USER_ID": str(user_id) if user_id else None, @@ -405,6 +414,7 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co "LOGIN_SUCCESS": real_mode, } res = call_waf_callback(custom_data=custom_data, force_sent=True) + print(f">>> WAF {custom_data} {res}", file=sys.stderr, flush=True) if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): raise BlockingException(get_blocked()) From d949d3dd70e4502e3b267f686a5d7382e052bd31 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Fri, 24 Jan 2025 16:48:07 +0100 Subject: [PATCH 12/31] remove debug code --- ddtrace/appsec/_trace_utils.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 61cb6f85e9f..eeaf9909033 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -377,9 +377,6 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_config): - import sys - - print(f">>> on_django_process: {result_user}, {result_user.is_authenticated}, {in_asm_context()}, {asm_config._asm_enabled}, {mode}, {kwargs}, {pin}, {info_retriever}, {django_config}", file=sys.stderr, flush=True) if not asm_config._asm_enabled: return userid_list = info_retriever.possible_user_id_fields + info_retriever.possible_login_fields @@ -404,8 +401,10 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True) elif mode == LOGIN_EVENTS_MODE.IDENT: - set_user(pin.tracer, str(user_id), propagate=True, email=user_extra.get("email"), name=user_extra.get("name")) - except Exception as e: # nosec + set_user( + pin.tracer, str(user_id), propagate=True, email=user_extra.get("email"), name=user_extra.get("name") + ) + except Exception: # nosec pass real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { @@ -414,7 +413,6 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co "LOGIN_SUCCESS": real_mode, } res = call_waf_callback(custom_data=custom_data, force_sent=True) - print(f">>> WAF {custom_data} {res}", file=sys.stderr, flush=True) if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): raise BlockingException(get_blocked()) From eb3738e57c18750c1ed7412c2c39d131006b388e Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Mon, 27 Jan 2025 14:30:28 +0100 Subject: [PATCH 13/31] import ddwaf import mechanism --- ddtrace/appsec/_metrics.py | 2 +- ddtrace/appsec/_processor.py | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/ddtrace/appsec/_metrics.py b/ddtrace/appsec/_metrics.py index 75c329b50e1..ccab9d9455a 100644 --- a/ddtrace/appsec/_metrics.py +++ b/ddtrace/appsec/_metrics.py @@ -1,7 +1,7 @@ from ddtrace.appsec import _asm_request_context from ddtrace.appsec import _constants +import ddtrace.appsec._ddwaf as ddwaf from ddtrace.appsec._deduplications import deduplication -from ddtrace.appsec._processor import ddwaf from ddtrace.internal import telemetry from ddtrace.internal.logger import get_logger from ddtrace.internal.telemetry.constants import TELEMETRY_LOG_LEVEL diff --git a/ddtrace/appsec/_processor.py b/ddtrace/appsec/_processor.py index 33f3ac8ced1..df9fa972ddd 100644 --- a/ddtrace/appsec/_processor.py +++ b/ddtrace/appsec/_processor.py @@ -4,6 +4,7 @@ from json.decoder import JSONDecodeError import os import os.path +from typing import TYPE_CHECKING from typing import Any from typing import Dict from typing import List @@ -11,6 +12,11 @@ from typing import Set from typing import Tuple from typing import Union + + +if TYPE_CHECKING: + import ddtrace.appsec._ddwaf as ddwaf + import weakref from ddtrace._trace.processor import SpanProcessor @@ -167,10 +173,14 @@ def __post_init__(self) -> None: def delayed_init(self) -> None: try: if self._rules is not None and not hasattr(self, "_ddwaf"): + import ddtrace.appsec._ddwaf as ddwaf # noqa: E402 + import ddtrace.appsec._metrics as metrics + + self.metrics = metrics self._ddwaf = ddwaf.DDWaf( self._rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp ) - _set_waf_init_metric(self._ddwaf.info) + self.metrics._set_waf_init_metric(self._ddwaf.info) except Exception: # Partial of DDAS-0005-00 log.warning("[DDAS-0005-00] WAF initialization failed") @@ -193,7 +203,7 @@ def _update_rules(self, new_rules: Dict[str, Any]) -> bool: if asm_config._asm_static_rule_file is not None: return result result = self._ddwaf.update_rules(new_rules) - _set_waf_updates_metric(self._ddwaf.info) + self.metrics._set_waf_updates_metric(self._ddwaf.info) self._update_required() return result @@ -241,7 +251,7 @@ def waf_callable(custom_data=None, **kwargs): return self._waf_action(span._local_root or span, ctx, custom_data, **kwargs) _asm_request_context.set_waf_callback(waf_callable) - _asm_request_context.add_context_callback(_set_waf_request_metrics) + _asm_request_context.add_context_callback(self.metrics._set_waf_request_metrics) if headers is not None: _asm_request_context.set_waf_address(SPAN_DATA_NAMES.REQUEST_HEADERS_NO_COOKIES, headers) _asm_request_context.set_waf_address( @@ -436,10 +446,3 @@ def on_span_finish(self, span: Span) -> None: del self._span_to_waf_ctx[s] except Exception: # nosec B110 pass - - -# load waf at the end only to avoid possible circular imports with gevent -import ddtrace.appsec._ddwaf as ddwaf # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_init_metric # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_request_metrics # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_updates_metric # noqa: E402 From 667be44b4273caf25ef4b537b432df7c9096bafb Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Mon, 27 Jan 2025 14:59:42 +0100 Subject: [PATCH 14/31] more log for waf initialisation --- ddtrace/appsec/_processor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ddtrace/appsec/_processor.py b/ddtrace/appsec/_processor.py index ac9d00beab1..1e3f850956a 100644 --- a/ddtrace/appsec/_processor.py +++ b/ddtrace/appsec/_processor.py @@ -181,10 +181,10 @@ def delayed_init(self) -> None: self._rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp ) self.metrics._set_waf_init_metric(self._ddwaf.info) - except Exception: + except Exception as e: # Partial of DDAS-0005-00 - log.warning("[DDAS-0005-00] WAF initialization failed") - raise + log.warning("[DDAS-0005-00] WAF initialization failed: %s", repr(e)) + raise BaseException(e, e.__traceback__) self._update_required() def _update_required(self): From 28ff78024f4e42b5126ec8050f54e123711ed3f6 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Mon, 27 Jan 2025 15:17:41 +0100 Subject: [PATCH 15/31] bypass Popen instrumentation for waf initialisation --- ddtrace/appsec/_processor.py | 3 +++ ddtrace/contrib/internal/subprocess/patch.py | 2 ++ ddtrace/settings/asm.py | 1 + 3 files changed, 6 insertions(+) diff --git a/ddtrace/appsec/_processor.py b/ddtrace/appsec/_processor.py index 1e3f850956a..517f390ab05 100644 --- a/ddtrace/appsec/_processor.py +++ b/ddtrace/appsec/_processor.py @@ -172,6 +172,7 @@ def __post_init__(self) -> None: def delayed_init(self) -> None: try: + asm_config._bypass_instrumentation_for_waf = True if self._rules is not None and not hasattr(self, "_ddwaf"): import ddtrace.appsec._ddwaf as ddwaf # noqa: E402 import ddtrace.appsec._metrics as metrics @@ -185,6 +186,8 @@ def delayed_init(self) -> None: # Partial of DDAS-0005-00 log.warning("[DDAS-0005-00] WAF initialization failed: %s", repr(e)) raise BaseException(e, e.__traceback__) + finally: + asm_config._bypass_instrumentation_for_waf = False self._update_required() def _update_required(self): diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 80d05b107bb..5b47d0bb528 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -327,6 +327,8 @@ def unpatch() -> None: @trace_utils.with_traced_module def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) if isinstance(args[0], str): for callback in _STR_CALLBACKS.values(): callback(args[0]) diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index 16937399501..fc2c9aa13fb 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -245,6 +245,7 @@ class ASMConfig(Env): default=r"^[+-]?((0b[01]+)|(0x[0-9A-Fa-f]+)|(\d+\.?\d*(?:[Ee][+-]?\d+)?|\.\d+(?:[Ee][+-]" + r"?\d+)?)|(X\'[0-9A-Fa-f]+\')|(B\'[01]+\'))$", ) + _bypass_instrumentation_for_waf = False def __init__(self): super().__init__() From 993afc934ab6d51e645fa68b2927e29693fd54fb Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Mon, 27 Jan 2025 15:21:58 +0100 Subject: [PATCH 16/31] bypass subprocess instrumentation for waf initialisation --- ddtrace/contrib/internal/subprocess/patch.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 5b47d0bb528..d45cb48631a 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -395,6 +395,8 @@ def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) cmd_args = args[0] if len(args) else kwargs["args"] if isinstance(cmd_args, (list, tuple, str)): if kwargs.get("shell", False): From 09f1ab2bb16965723ee9d17c35cbc249147456ba Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Mon, 27 Jan 2025 15:25:29 +0100 Subject: [PATCH 17/31] bypass subprocess.wait instrumentation for waf initialisation --- ddtrace/contrib/internal/subprocess/patch.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index d45cb48631a..2d66edd4737 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -429,6 +429,8 @@ def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_wait(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) binary = core.get_item("subprocess_popen_binary") with pin.tracer.trace(COMMANDS.SPAN_NAME, resource=binary, span_type=SpanTypes.SYSTEM) as span: From aaedab9bcd69363043aaf47602288f96faf5fcfe Mon Sep 17 00:00:00 2001 From: Yun Kim <35776586+Yun-Kim@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:23:45 -0500 Subject: [PATCH 18/31] chore: update changelog for version 2.19.2 (#12088) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --------- Co-authored-by: Nicole Cybul --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a29299f8987..6c72665472f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ Changelogs for versions not listed here can be found at https://github.com/DataD --- +## 2.19.2 +### Bug Fixes + +- Tracing + - celery: Fixes an issue where `celery.apply` spans from Celery prerun got closed too soon leading to span tags being missing. + - openai: Fixes a patching issue where asynchronous moderation endpoint calls resulted in coroutine scheduling errors. + - openai: Ensures the OpenAI integration is compatible with Python versions 3.12 and 3.13. + - vertexai: Resolves an issue with `chat.send_message()` where the content keyword argument was not parsed correctly. +- LLM Observability + - This fix resolves an issue where annotating a span with non latin-1 (but valid utf-8) input/output values resulted in encoding errors. +- Lib-Injection + - Fixes incorrect telemetry data payload format. + +--- + ## 2.19.1 ### Bug Fixes From e3045a19a20e0348f040aa589ef3c2517db74618 Mon Sep 17 00:00:00 2001 From: Nick Ripley Date: Mon, 27 Jan 2025 16:30:29 -0500 Subject: [PATCH 19/31] fix(profiling): fix SystemError when collecting memory profiler events (#12075) We added locking to the memory profiler to address crashes. These locks are mostly "try" locks, meaning we bail out if we can't acquire them right away. This was done defensively to mitigate the possibility of deadlock until we fully understood why the locks are needed and could guarantee their correctness. But as a result of using try locks, the `iter_events` function in particular can fail if the memory profiler lock is contended when it tries to collect profiling events. The function then returns NULL, leading to SystemError exceptions because we don't set an error. Even if we set an error, returning NULL isn't the right thing to do. It'll basically mean we wait until the next profile iteration, still accumulating events in the same buffer, and try again to upload the events. So we're going to get multiple iteration's worth of events. The right thing to do is take the lock unconditionally in `iter_events`. We can allocate the new tracker outside the memory allocation profiler lock so that we don't need to worry about reentrancy/deadlock issues if we start profiling that allocation. Then, the only thing we do under the lock is swap out the global tracker, so it's safe to take the lock unconditionally. Fixes #11831 TODO - regression test? --- ddtrace/profiling/collector/_memalloc.c | 22 +++++++++++++------ ...loc-iter-events-null-780fd50bbebbf616.yaml | 4 ++++ 2 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml diff --git a/ddtrace/profiling/collector/_memalloc.c b/ddtrace/profiling/collector/_memalloc.c index 1f2b87e0433..1e1b9dbf52e 100644 --- a/ddtrace/profiling/collector/_memalloc.c +++ b/ddtrace/profiling/collector/_memalloc.c @@ -394,20 +394,28 @@ iterevents_new(PyTypeObject* type, PyObject* Py_UNUSED(args), PyObject* Py_UNUSE } IterEventsState* iestate = (IterEventsState*)type->tp_alloc(type, 0); - if (!iestate) + if (!iestate) { + PyErr_SetString(PyExc_RuntimeError, "failed to allocate IterEventsState"); return NULL; + } memalloc_assert_gil(); - /* reset the current traceback list */ - if (memlock_trylock(&g_memalloc_lock)) { - iestate->alloc_tracker = global_alloc_tracker; - global_alloc_tracker = alloc_tracker_new(); - memlock_unlock(&g_memalloc_lock); - } else { + /* Reset the current traceback list. Do this outside lock so we can track it, + * and avoid reentrancy/deadlock problems, if we start tracking the raw + * allocator domain */ + alloc_tracker_t* tracker = alloc_tracker_new(); + if (!tracker) { + PyErr_SetString(PyExc_RuntimeError, "failed to allocate new allocation tracker"); Py_TYPE(iestate)->tp_free(iestate); return NULL; } + + memlock_lock(&g_memalloc_lock); + iestate->alloc_tracker = global_alloc_tracker; + global_alloc_tracker = tracker; + memlock_unlock(&g_memalloc_lock); + iestate->seq_index = 0; PyObject* iter_and_count = PyTuple_New(3); diff --git a/releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml b/releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml new file mode 100644 index 00000000000..52a43cbd2a1 --- /dev/null +++ b/releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + profiling: fix SystemError from the memory profiler returning NULL when collecting events From 55767a78063ed17de0e954a6db4b9ff64b762f1d Mon Sep 17 00:00:00 2001 From: William Conti <58711692+wconti27@users.noreply.github.com> Date: Tue, 28 Jan 2025 08:44:03 -0500 Subject: [PATCH 20/31] chore(tracing): refactor web server integrations to use the core module (#12035) ## Motivation Refactors all web server integrations still using `tracer.trace` to instead use `core.context_with_data`. This is in preparation for supporting AWS API Gateway to ensure all web servers share the same code path. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --- ddtrace/_trace/trace_handlers.py | 56 +++++++++- .../contrib/internal/aiohttp/middlewares.py | 105 ++++++++---------- ddtrace/contrib/internal/bottle/trace.py | 61 +++++----- .../contrib/internal/cherrypy/middleware.py | 53 +++++---- ddtrace/contrib/internal/falcon/middleware.py | 61 ++++------ ddtrace/contrib/internal/molten/patch.py | 52 ++++----- ddtrace/contrib/internal/pyramid/trace.py | 72 ++++++------ ddtrace/contrib/internal/sanic/patch.py | 61 +++++----- ...b.cherrypy.test_middleware.test_child.json | 1 + ...b.cherrypy.test_middleware.test_error.json | 1 + ...b.cherrypy.test_middleware.test_fatal.json | 1 + ...cherrypy.test_middleware.test_success.json | 1 + ...ver.test_multiple_requests_sanic_http.json | 2 + ...multiple_requests_sanic_http_pre_21.9.json | 2 + ...c.test_sanic_server.test_sanic_errors.json | 2 + ...nic_server.test_sanic_errors_pre_21.9.json | 2 + 16 files changed, 290 insertions(+), 243 deletions(-) diff --git a/ddtrace/_trace/trace_handlers.py b/ddtrace/_trace/trace_handlers.py index dab3f743146..3ebb28f33c8 100644 --- a/ddtrace/_trace/trace_handlers.py +++ b/ddtrace/_trace/trace_handlers.py @@ -109,11 +109,14 @@ def _get_parameters_for_new_span_directly_from_context(ctx: core.ExecutionContex def _start_span(ctx: core.ExecutionContext, call_trace: bool = True, **kwargs) -> "Span": span_kwargs = _get_parameters_for_new_span_directly_from_context(ctx) call_trace = ctx.get_item("call_trace", call_trace) - tracer = (ctx.get_item("middleware") or ctx["pin"]).tracer + tracer = ctx.get_item("tracer") or (ctx.get_item("middleware") or ctx["pin"]).tracer distributed_headers_config = ctx.get_item("distributed_headers_config") if distributed_headers_config: trace_utils.activate_distributed_headers( - tracer, int_config=distributed_headers_config, request_headers=ctx["distributed_headers"] + tracer, + int_config=distributed_headers_config, + request_headers=ctx["distributed_headers"], + override=ctx.get_item("distributed_headers_config_override"), ) distributed_context = ctx.get_item("distributed_context") if distributed_context and not call_trace: @@ -126,6 +129,42 @@ def _start_span(ctx: core.ExecutionContext, call_trace: bool = True, **kwargs) - return span +def _set_web_frameworks_tags(ctx, span, int_config): + span.set_tag_str(COMPONENT, int_config.integration_name) + span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + span.set_tag(_SPAN_MEASURED_KEY) + + analytics_enabled = ctx.get_item("analytics_enabled") + analytics_sample_rate = ctx.get_item("analytics_sample_rate", True) + + # Configure trace search sample rate + if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: + span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, analytics_sample_rate) + + +def _on_web_framework_start_request(ctx, int_config): + request_span = ctx.get_item("req_span") + _set_web_frameworks_tags(ctx, request_span, int_config) + + +def _on_web_framework_finish_request( + span, int_config, method, url, status_code, query, req_headers, res_headers, route, finish +): + trace_utils.set_http_meta( + span=span, + integration_config=int_config, + method=method, + url=url, + status_code=status_code, + query=query, + request_headers=req_headers, + response_headers=res_headers, + route=route, + ) + if finish: + span.finish() + + def _on_traced_request_context_started_flask(ctx): current_span = ctx["pin"].tracer.current_span() if not ctx["pin"].enabled or not current_span: @@ -761,6 +800,10 @@ def listen(): core.on("azure.functions.request_call_modifier", _on_azure_functions_request_span_modifier) core.on("azure.functions.start_response", _on_azure_functions_start_response) + # web frameworks general handlers + core.on("web.request.start", _on_web_framework_start_request) + core.on("web.request.finish", _on_web_framework_finish_request) + core.on("test_visibility.enable", _on_test_visibility_enable) core.on("test_visibility.disable", _on_test_visibility_disable) core.on("test_visibility.is_enabled", _on_test_visibility_is_enabled, "is_enabled") @@ -769,6 +812,14 @@ def listen(): core.on("rq.queue.enqueue_job", _propagate_context) for context_name in ( + # web frameworks + "aiohttp.request", + "bottle.request", + "cherrypy.request", + "falcon.request", + "molten.request", + "pyramid.request", + "sanic.request", "flask.call", "flask.jsonify", "flask.render_template", @@ -779,6 +830,7 @@ def listen(): "django.template.render", "django.process_exception", "django.func.wrapped", + # non web frameworks "botocore.instrumented_api_call", "botocore.instrumented_lib_function", "botocore.patched_kinesis_api_call", diff --git a/ddtrace/contrib/internal/aiohttp/middlewares.py b/ddtrace/contrib/internal/aiohttp/middlewares.py index ddb2d35fbc6..63a60734d3b 100644 --- a/ddtrace/contrib/internal/aiohttp/middlewares.py +++ b/ddtrace/contrib/internal/aiohttp/middlewares.py @@ -2,15 +2,10 @@ from aiohttp.web_urldispatcher import SystemRoute from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils from ddtrace.contrib.asyncio import context_provider -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.ext import http -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection @@ -35,47 +30,42 @@ async def attach_context(request): # application configs tracer = app[CONFIG_KEY]["tracer"] service = app[CONFIG_KEY]["service"] - distributed_tracing = app[CONFIG_KEY]["distributed_tracing_enabled"] - # Create a new context based on the propagated information. - trace_utils.activate_distributed_headers( - tracer, - int_config=config.aiohttp, - request_headers=request.headers, - override=distributed_tracing, - ) - - # trace the handler - request_span = tracer.trace( - schematize_url_operation("aiohttp.request", protocol="http", direction=SpanDirection.INBOUND), - service=service, - span_type=SpanTypes.WEB, - ) - request_span.set_tag(_SPAN_MEASURED_KEY) - - request_span.set_tag_str(COMPONENT, config.aiohttp.integration_name) - - # set span.kind tag equal to type of request - request_span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - # Configure trace search sample rate # DEV: aiohttp is special case maintains separate configuration from config api analytics_enabled = app[CONFIG_KEY]["analytics_enabled"] - if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: - request_span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, app[CONFIG_KEY].get("analytics_sample_rate", True)) - - # attach the context and the root span to the request; the Context - # may be freely used by the application code - request[REQUEST_CONTEXT_KEY] = request_span.context - request[REQUEST_SPAN_KEY] = request_span - request[REQUEST_CONFIG_KEY] = app[CONFIG_KEY] - try: - response = await handler(request) - if isinstance(response, web.StreamResponse): - request.task.add_done_callback(lambda _: finish_request_span(request, response)) - return response - except Exception: - request_span.set_traceback() - raise + # Create a new context based on the propagated information. + + with core.context_with_data( + "aiohttp.request", + span_name=schematize_url_operation("aiohttp.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, + service=service, + tags={}, + tracer=tracer, + distributed_headers=request.headers, + distributed_headers_config=config.aiohttp, + distributed_headers_config_override=app[CONFIG_KEY]["distributed_tracing_enabled"], + headers_case_sensitive=True, + analytics_enabled=analytics_enabled, + analytics_sample_rate=app[CONFIG_KEY].get("analytics_sample_rate", True), + ) as ctx: + req_span = ctx.span + + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.aiohttp)) + + # attach the context and the root span to the request; the Context + # may be freely used by the application code + request[REQUEST_CONTEXT_KEY] = req_span.context + request[REQUEST_SPAN_KEY] = req_span + request[REQUEST_CONFIG_KEY] = app[CONFIG_KEY] + try: + response = await handler(request) + if isinstance(response, web.StreamResponse): + request.task.add_done_callback(lambda _: finish_request_span(request, response)) + return response + except Exception: + req_span.set_traceback() + raise return attach_context @@ -122,19 +112,22 @@ def finish_request_span(request, response): # SystemRoute objects exist to throw HTTP errors and have no path route = aiohttp_route.resource.canonical - trace_utils.set_http_meta( - request_span, - config.aiohttp, - method=request.method, - url=str(request.url), # DEV: request.url is a yarl's URL object - status_code=response.status, - request_headers=request.headers, - response_headers=response.headers, - route=route, + core.dispatch( + "web.request.finish", + ( + request_span, + config.aiohttp, + request.method, + str(request.url), # DEV: request.url is a yarl's URL object + response.status, + None, # query arg = None + request.headers, + response.headers, + route, + True, + ), ) - request_span.finish() - async def on_prepare(request, response): """ diff --git a/ddtrace/contrib/internal/bottle/trace.py b/ddtrace/contrib/internal/bottle/trace.py index 3aabb4ccc81..58778a36ee1 100644 --- a/ddtrace/contrib/internal/bottle/trace.py +++ b/ddtrace/contrib/internal/bottle/trace.py @@ -5,13 +5,8 @@ import ddtrace from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection from ddtrace.internal.utils.formats import asbool @@ -42,24 +37,21 @@ def wrapped(*args, **kwargs): resource = "{} {}".format(request.method, route.rule) - trace_utils.activate_distributed_headers( - self.tracer, int_config=config.bottle, request_headers=request.headers - ) - - with self.tracer.trace( - schematize_url_operation("bottle.request", protocol="http", direction=SpanDirection.INBOUND), + with core.context_with_data( + "bottle.request", + span_name=schematize_url_operation("bottle.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, service=self.service, resource=resource, - span_type=SpanTypes.WEB, - ) as s: - s.set_tag_str(COMPONENT, config.bottle.integration_name) - - # set span.kind to the type of request being performed - s.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - s.set_tag(_SPAN_MEASURED_KEY) - # set analytics sample rate with global config enabled - s.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.bottle.get_analytics_sample_rate(use_global_config=True)) + tags={}, + tracer=self.tracer, + distributed_headers=request.headers, + distributed_headers_config=config.bottle, + headers_case_sensitive=True, + analytics_sample_rate=config.bottle.get_analytics_sample_rate(use_global_config=True), + ) as ctx, ctx.span as req_span: + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.bottle)) code = None result = None @@ -91,16 +83,21 @@ def wrapped(*args, **kwargs): method = request.method url = request.urlparts._replace(query="").geturl() full_route = "/".join([request.script_name.rstrip("/"), route.rule.lstrip("/")]) - trace_utils.set_http_meta( - s, - config.bottle, - method=method, - url=url, - status_code=response_code, - query=request.query_string, - request_headers=request.headers, - response_headers=response.headers, - route=full_route, + + core.dispatch( + "web.request.finish", + ( + req_span, + config.bottle, + method, + url, + response_code, + request.query_string, + request.headers, + response.headers, + full_route, + False, + ), ) return wrapped diff --git a/ddtrace/contrib/internal/cherrypy/middleware.py b/ddtrace/contrib/internal/cherrypy/middleware.py index 909a043175f..70f43059905 100644 --- a/ddtrace/contrib/internal/cherrypy/middleware.py +++ b/ddtrace/contrib/internal/cherrypy/middleware.py @@ -11,12 +11,10 @@ from ddtrace.constants import ERROR_MSG from ddtrace.constants import ERROR_STACK from ddtrace.constants import ERROR_TYPE -from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.internal import compat -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import SpanDirection from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -77,20 +75,23 @@ def _setup(self): cherrypy.request.hooks.attach("after_error_response", self._after_error_response, priority=5) def _on_start_resource(self): - trace_utils.activate_distributed_headers( - self._tracer, int_config=config.cherrypy, request_headers=cherrypy.request.headers - ) - - cherrypy.request._datadog_span = self._tracer.trace( - SPAN_NAME, - service=trace_utils.int_service(None, config.cherrypy, default="cherrypy"), + with core.context_with_data( + "cherrypy.request", + span_name=SPAN_NAME, span_type=SpanTypes.WEB, - ) + service=trace_utils.int_service(None, config.cherrypy, default="cherrypy"), + tags={}, + tracer=self._tracer, + distributed_headers=cherrypy.request.headers, + distributed_headers_config=config.cherrypy, + headers_case_sensitive=True, + ) as ctx: + req_span = ctx.span - cherrypy.request._datadog_span.set_tag_str(COMPONENT, config.cherrypy.integration_name) + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.cherrypy)) - # set span.kind to the type of request being performed - cherrypy.request._datadog_span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + cherrypy.request._datadog_span = req_span def _after_error_response(self): span = getattr(cherrypy.request, "_datadog_span", None) @@ -135,18 +136,22 @@ def _close_span(self, span): url = compat.to_unicode(cherrypy.request.base + cherrypy.request.path_info) status_code, _, _ = valid_status(cherrypy.response.status) - trace_utils.set_http_meta( - span, - config.cherrypy, - method=cherrypy.request.method, - url=url, - status_code=status_code, - request_headers=cherrypy.request.headers, - response_headers=cherrypy.response.headers, + core.dispatch( + "web.request.finish", + ( + span, + config.cherrypy, + cherrypy.request.method, + url, + status_code, + None, + cherrypy.request.headers, + cherrypy.response.headers, + None, + True, + ), ) - span.finish() - # Clear our span just in case. cherrypy.request._datadog_span = None diff --git a/ddtrace/contrib/internal/falcon/middleware.py b/ddtrace/contrib/internal/falcon/middleware.py index b4ec5434777..513ce6cce39 100644 --- a/ddtrace/contrib/internal/falcon/middleware.py +++ b/ddtrace/contrib/internal/falcon/middleware.py @@ -1,14 +1,8 @@ import sys from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.ext import http as httpx -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import SpanDirection from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -27,26 +21,27 @@ def __init__(self, tracer, service=None, distributed_tracing=None): def process_request(self, req, resp): # Falcon uppercases all header names. headers = dict((k.lower(), v) for k, v in req.headers.items()) - trace_utils.activate_distributed_headers(self.tracer, int_config=config.falcon, request_headers=headers) - span = self.tracer.trace( - schematize_url_operation("falcon.request", protocol="http", direction=SpanDirection.INBOUND), - service=self.service, + with core.context_with_data( + "falcon.request", + span_name=schematize_url_operation("falcon.request", protocol="http", direction=SpanDirection.INBOUND), span_type=SpanTypes.WEB, - ) - span.set_tag_str(COMPONENT, config.falcon.integration_name) - - # set span.kind to the type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - span.set_tag(_SPAN_MEASURED_KEY) - - # set analytics sample rate with global config enabled - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.falcon.get_analytics_sample_rate(use_global_config=True)) - - trace_utils.set_http_meta( - span, config.falcon, method=req.method, url=req.url, query=req.query_string, request_headers=req.headers - ) + service=self.service, + tags={}, + tracer=self.tracer, + distributed_headers=headers, + distributed_headers_config=config.falcon, + headers_case_sensitive=True, + analytics_sample_rate=config.falcon.get_analytics_sample_rate(use_global_config=True), + ) as ctx: + req_span = ctx.span + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.falcon)) + + core.dispatch( + "web.request.finish", + (req_span, config.falcon, req.method, req.url, None, req.query_string, req.headers, None, None, False), + ) def process_resource(self, req, resp, resource, params): span = self.tracer.current_span() @@ -69,8 +64,7 @@ def process_response(self, req, resp, resource, req_succeeded=None): if resource is None: status = "404" span.resource = "%s 404" % req.method - span.set_tag(httpx.STATUS_CODE, status) - span.finish() + core.dispatch("web.request.finish", (span, config.falcon, None, None, status, None, None, None, None, True)) return err_type = sys.exc_info()[0] @@ -87,20 +81,13 @@ def process_response(self, req, resp, resource, req_succeeded=None): route = req.root_path or "" + req.uri_template - trace_utils.set_http_meta( - span, - config.falcon, - status_code=status, - response_headers=resp._headers, - route=route, - ) - # Emit span hook for this response # DEV: Emit before closing so they can overwrite `span.resource` if they want config.falcon.hooks.emit("request", span, req, resp) - # Close the span - span.finish() + core.dispatch( + "web.request.finish", (span, config.falcon, None, None, status, None, None, resp._headers, route, True) + ) def _is_404(err_type): diff --git a/ddtrace/contrib/internal/molten/patch.py b/ddtrace/contrib/internal/molten/patch.py index 38fa949243c..dfec47eb17d 100644 --- a/ddtrace/contrib/internal/molten/patch.py +++ b/ddtrace/contrib/internal/molten/patch.py @@ -5,15 +5,11 @@ from wrapt import wrap_function_wrapper as _w from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils from ddtrace.contrib.internal.trace_utils import unwrap as _u -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes +from ddtrace.internal import core from ddtrace.internal.compat import urlencode -from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection @@ -89,25 +85,21 @@ def patch_app_call(wrapped, instance, args, kwargs): request = molten.http.Request.from_environ(environ) resource = func_name(wrapped) - # request.headers is type Iterable[Tuple[str, str]] - trace_utils.activate_distributed_headers( - pin.tracer, int_config=config.molten, request_headers=dict(request.headers) - ) - - with pin.tracer.trace( - schematize_url_operation("molten.request", protocol="http", direction=SpanDirection.INBOUND), + with core.context_with_data( + "molten.request", + span_name=schematize_url_operation("molten.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, service=trace_utils.int_service(pin, config.molten), resource=resource, - span_type=SpanTypes.WEB, - ) as span: - span.set_tag_str(COMPONENT, config.molten.integration_name) - - # set span.kind tag equal to type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - span.set_tag(_SPAN_MEASURED_KEY) - # set analytics sample rate with global config enabled - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.molten.get_analytics_sample_rate(use_global_config=True)) + tags={}, + tracer=pin.tracer, + distributed_headers=dict(request.headers), # request.headers is type Iterable[Tuple[str, str]] + distributed_headers_config=config.molten, + headers_case_sensitive=True, + analytics_sample_rate=config.molten.get_analytics_sample_rate(use_global_config=True), + ) as ctx, ctx.span as req_span: + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.molten)) @wrapt.function_wrapper def _w_start_response(wrapped, instance, args, kwargs): @@ -125,11 +117,13 @@ def _w_start_response(wrapped, instance, args, kwargs): except ValueError: pass - if not span.get_tag(MOLTEN_ROUTE): + if not req_span.get_tag(MOLTEN_ROUTE): # if route never resolve, update root resource - span.resource = "{} {}".format(request.method, code) + req_span.resource = "{} {}".format(request.method, code) - trace_utils.set_http_meta(span, config.molten, status_code=code) + core.dispatch( + "web.request.finish", (req_span, config.molten, None, None, code, None, None, None, None, False) + ) return wrapped(*args, **kwargs) @@ -143,11 +137,13 @@ def _w_start_response(wrapped, instance, args, kwargs): request.path, ) query = urlencode(dict(request.params)) - trace_utils.set_http_meta( - span, config.molten, method=request.method, url=url, query=query, request_headers=request.headers + + core.dispatch( + "web.request.finish", + (req_span, config.molten, request.method, url, None, query, request.headers, None, None, False), ) - span.set_tag_str("molten.version", molten.__version__) + req_span.set_tag_str("molten.version", molten.__version__) return wrapped(environ, start_response, **kwargs) diff --git a/ddtrace/contrib/internal/pyramid/trace.py b/ddtrace/contrib/internal/pyramid/trace.py index 9942c673d4e..9b221bd3f09 100644 --- a/ddtrace/contrib/internal/pyramid/trace.py +++ b/ddtrace/contrib/internal/pyramid/trace.py @@ -6,12 +6,8 @@ # project import ddtrace from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes +from ddtrace.internal import core from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name @@ -67,29 +63,30 @@ def trace_tween_factory(handler, registry): service = settings.get(SETTINGS_SERVICE) or schematize_service_name("pyramid") tracer = settings.get(SETTINGS_TRACER) or ddtrace.tracer enabled = asbool(settings.get(SETTINGS_TRACE_ENABLED, tracer.enabled)) - distributed_tracing = asbool(settings.get(SETTINGS_DISTRIBUTED_TRACING, True)) + + # ensure distributed tracing within pyramid settings matches config + config.pyramid.distributed_tracing_enabled = asbool(settings.get(SETTINGS_DISTRIBUTED_TRACING, True)) if enabled: # make a request tracing function def trace_tween(request): - trace_utils.activate_distributed_headers( - tracer, int_config=config.pyramid, request_headers=request.headers, override=distributed_tracing - ) - - span_name = schematize_url_operation("pyramid.request", protocol="http", direction=SpanDirection.INBOUND) - with tracer.trace(span_name, service=service, resource="404", span_type=SpanTypes.WEB) as span: - span.set_tag_str(COMPONENT, config.pyramid.integration_name) - - # set span.kind to the type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - span.set_tag(_SPAN_MEASURED_KEY) - # Configure trace search sample rate + with core.context_with_data( + "pyramid.request", + span_name=schematize_url_operation("pyramid.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, + service=service, + resource="404", + tags={}, + tracer=tracer, + distributed_headers=request.headers, + distributed_headers_config=config.pyramid, + headers_case_sensitive=True, # DEV: pyramid is special case maintains separate configuration from config api - analytics_enabled = settings.get(SETTINGS_ANALYTICS_ENABLED) - - if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, settings.get(SETTINGS_ANALYTICS_SAMPLE_RATE, True)) + analytics_enabled=settings.get(SETTINGS_ANALYTICS_ENABLED), + analytics_sample_rate=settings.get(SETTINGS_ANALYTICS_SAMPLE_RATE, True), + ) as ctx, ctx.span as req_span: + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.pyramid)) setattr(request, DD_TRACER, tracer) # used to find the tracer in templates response = None @@ -110,8 +107,8 @@ def trace_tween(request): finally: # set request tags if request.matched_route: - span.resource = "{} {}".format(request.method, request.matched_route.name) - span.set_tag_str("pyramid.route.name", request.matched_route.name) + req_span.resource = "{} {}".format(request.method, request.matched_route.name) + req_span.set_tag_str("pyramid.route.name", request.matched_route.name) # set response tags if response: status = response.status_code @@ -119,17 +116,22 @@ def trace_tween(request): else: response_headers = None - trace_utils.set_http_meta( - span, - config.pyramid, - method=request.method, - url=request.path_url, - status_code=status, - query=request.query_string, - request_headers=request.headers, - response_headers=response_headers, - route=request.matched_route.pattern if request.matched_route else None, + core.dispatch( + "web.request.finish", + ( + req_span, + config.pyramid, + request.method, + request.path_url, + status, + request.query_string, + request.headers, + response_headers, + request.matched_route.pattern if request.matched_route else None, + False, + ), ) + return response return trace_tween diff --git a/ddtrace/contrib/internal/sanic/patch.py b/ddtrace/contrib/internal/sanic/patch.py index 8e53ed41dc8..1babf7d44a0 100644 --- a/ddtrace/contrib/internal/sanic/patch.py +++ b/ddtrace/contrib/internal/sanic/patch.py @@ -4,14 +4,10 @@ import wrapt from wrapt import wrap_function_wrapper as _w -import ddtrace from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -47,7 +43,10 @@ def update_span(span, response): # and so use 500 status_code = getattr(response, "status", 500) response_headers = getattr(response, "headers", None) - trace_utils.set_http_meta(span, config.sanic, status_code=status_code, response_headers=response_headers) + + core.dispatch( + "web.request.finish", (span, config.sanic, None, None, status_code, None, None, response_headers, None, False) + ) def _wrap_response_callback(span, callback): @@ -200,31 +199,35 @@ def _create_sanic_request_span(request): headers = request.headers.copy() - trace_utils.activate_distributed_headers(ddtrace.tracer, int_config=config.sanic, request_headers=headers) - - span = pin.tracer.trace( - schematize_url_operation("sanic.request", protocol="http", direction=SpanDirection.INBOUND), + with core.context_with_data( + "sanic.request", + span_name=schematize_url_operation("sanic.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, service=trace_utils.int_service(None, config.sanic), resource=resource, - span_type=SpanTypes.WEB, - ) - span.set_tag_str(COMPONENT, config.sanic.integration_name) - - # set span.kind to the type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - sample_rate = config.sanic.get_analytics_sample_rate(use_global_config=True) - if sample_rate is not None: - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, sample_rate) - - method = request.method - url = "{scheme}://{host}{path}".format(scheme=request.scheme, host=request.host, path=request.path) - query_string = request.query_string - if isinstance(query_string, bytes): - query_string = query_string.decode() - trace_utils.set_http_meta(span, config.sanic, method=method, url=url, query=query_string, request_headers=headers) - - return span + tags={}, + pin=pin, + distributed_headers=headers, + distributed_headers_config=config.sanic, + headers_case_sensitive=True, + analytics_sample_rate=config.sanic.get_analytics_sample_rate(use_global_config=True), + ) as ctx: + req_span = ctx.span + + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.sanic)) + + method = request.method + url = "{scheme}://{host}{path}".format(scheme=request.scheme, host=request.host, path=request.path) + query_string = request.query_string + if isinstance(query_string, bytes): + query_string = query_string.decode() + + core.dispatch( + "web.request.finish", (req_span, config.sanic, method, url, None, query_string, headers, None, None, False) + ) + + return req_span async def sanic_http_lifecycle_handle(request): diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json index 1887282b06f..933eda3e420 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json index 49759fee3d3..58fde524828 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json @@ -25,6 +25,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json index 4153c8c2b18..a5a7c1ed14f 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json @@ -25,6 +25,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json index d1eb9fc5bee..ce13988add0 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json index 3803d07f360..66ca28d9606 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json @@ -23,6 +23,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -72,6 +73,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json index 4aac2721c02..aca376a974a 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -70,6 +71,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json index 435bbbc7b23..ab2aeaec920 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -58,6 +59,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json index c03813a43d6..ceabecd1ae7 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -54,6 +55,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, From 16d5280956bc259e77691536229824d3183a4181 Mon Sep 17 00:00:00 2001 From: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Date: Tue, 28 Jan 2025 15:48:22 +0100 Subject: [PATCH 21/31] ci(tracer): make serverless test unrequired (#12121) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --- .gitlab/benchmarks.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab/benchmarks.yml b/.gitlab/benchmarks.yml index e922d315444..692a61ea93f 100644 --- a/.gitlab/benchmarks.yml +++ b/.gitlab/benchmarks.yml @@ -96,6 +96,7 @@ benchmark-serverless: tags: ["arch:amd64"] when: on_success needs: [ "benchmark-serverless-trigger" ] + allow_failure: true script: - git clone https://gitlab-ci-token:${CI_JOB_TOKEN}@gitlab.ddbuild.io/DataDog/serverless-tools.git ./serverless-tools && cd ./serverless-tools - ./ci/check_trigger_status.sh From 4f0bcb5a59378fb6015230f9f9ad04199fc380b5 Mon Sep 17 00:00:00 2001 From: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:28:20 +0100 Subject: [PATCH 22/31] chore(asm): clean libddwaf loading (#12102) Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - https://github.com/DataDog/dd-trace-py/pull/11987 - https://github.com/DataDog/dd-trace-py/pull/12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --- ddtrace/appsec/_ddwaf/ddwaf_types.py | 3 +++ ddtrace/appsec/_processor.py | 26 +++++++++++--------- ddtrace/contrib/internal/subprocess/patch.py | 6 +++++ ddtrace/settings/asm.py | 1 + 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/ddtrace/appsec/_ddwaf/ddwaf_types.py b/ddtrace/appsec/_ddwaf/ddwaf_types.py index e998b6048f6..5942a2fa184 100644 --- a/ddtrace/appsec/_ddwaf/ddwaf_types.py +++ b/ddtrace/appsec/_ddwaf/ddwaf_types.py @@ -22,9 +22,12 @@ if system() == "Linux": try: + asm_config._bypass_instrumentation_for_waf = True ctypes.CDLL(ctypes.util.find_library("rt"), mode=ctypes.RTLD_GLOBAL) except Exception: # nosec pass + finally: + asm_config._bypass_instrumentation_for_waf = False ARCHI = machine().lower() diff --git a/ddtrace/appsec/_processor.py b/ddtrace/appsec/_processor.py index 6102ba1ded2..83b65584a66 100644 --- a/ddtrace/appsec/_processor.py +++ b/ddtrace/appsec/_processor.py @@ -4,6 +4,7 @@ from json.decoder import JSONDecodeError import os import os.path +from typing import TYPE_CHECKING from typing import Any from typing import Dict from typing import List @@ -11,6 +12,11 @@ from typing import Set from typing import Tuple from typing import Union + + +if TYPE_CHECKING: + import ddtrace.appsec._ddwaf as ddwaf + import weakref from ddtrace._trace.processor import SpanProcessor @@ -167,14 +173,17 @@ def __post_init__(self) -> None: def delayed_init(self) -> None: try: if self._rules is not None and not hasattr(self, "_ddwaf"): - self._ddwaf = ddwaf.DDWaf( + from ddtrace.appsec._ddwaf import DDWaf # noqa: E402 + import ddtrace.appsec._metrics as metrics # noqa: E402 + + self.metrics = metrics + self._ddwaf = DDWaf( self._rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp ) - _set_waf_init_metric(self._ddwaf.info) + self.metrics._set_waf_init_metric(self._ddwaf.info) except Exception: # Partial of DDAS-0005-00 log.warning("[DDAS-0005-00] WAF initialization failed") - raise self._update_required() def _update_required(self): @@ -193,7 +202,7 @@ def _update_rules(self, new_rules: Dict[str, Any]) -> bool: if asm_config._asm_static_rule_file is not None: return result result = self._ddwaf.update_rules(new_rules) - _set_waf_updates_metric(self._ddwaf.info) + self.metrics._set_waf_updates_metric(self._ddwaf.info) self._update_required() return result @@ -241,7 +250,7 @@ def waf_callable(custom_data=None, **kwargs): return self._waf_action(span._local_root or span, ctx, custom_data, **kwargs) _asm_request_context.set_waf_callback(waf_callable) - _asm_request_context.add_context_callback(_set_waf_request_metrics) + _asm_request_context.add_context_callback(self.metrics._set_waf_request_metrics) if headers is not None: _asm_request_context.set_waf_address(SPAN_DATA_NAMES.REQUEST_HEADERS_NO_COOKIES, headers) _asm_request_context.set_waf_address( @@ -436,10 +445,3 @@ def on_span_finish(self, span: Span) -> None: del self._span_to_waf_ctx[s] except Exception: # nosec B110 pass - - -# load waf at the end only to avoid possible circular imports with gevent -import ddtrace.appsec._ddwaf as ddwaf # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_init_metric # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_request_metrics # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_updates_metric # noqa: E402 diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 80d05b107bb..2d66edd4737 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -327,6 +327,8 @@ def unpatch() -> None: @trace_utils.with_traced_module def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) if isinstance(args[0], str): for callback in _STR_CALLBACKS.values(): callback(args[0]) @@ -393,6 +395,8 @@ def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) cmd_args = args[0] if len(args) else kwargs["args"] if isinstance(cmd_args, (list, tuple, str)): if kwargs.get("shell", False): @@ -425,6 +429,8 @@ def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_wait(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) binary = core.get_item("subprocess_popen_binary") with pin.tracer.trace(COMMANDS.SPAN_NAME, resource=binary, span_type=SpanTypes.SYSTEM) as span: diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index 16937399501..fc2c9aa13fb 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -245,6 +245,7 @@ class ASMConfig(Env): default=r"^[+-]?((0b[01]+)|(0x[0-9A-Fa-f]+)|(\d+\.?\d*(?:[Ee][+-]?\d+)?|\.\d+(?:[Ee][+-]" + r"?\d+)?)|(X\'[0-9A-Fa-f]+\')|(B\'[01]+\'))$", ) + _bypass_instrumentation_for_waf = False def __init__(self): super().__init__() From c4448ea4e5580cfa55684ef108a5dafafe1f6f75 Mon Sep 17 00:00:00 2001 From: Yun Kim <35776586+Yun-Kim@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:11:42 -0500 Subject: [PATCH 23/31] fix(llmobs): propagate distributed headers via signal dispatching, not config (#12089) This PR makes a change to our shared distributed tracing header injection method to dispatch signals/events instead of relying on the global config settings, which is only modifiable via env vars. This fixes distributed tracing for users that might rely solely on the `LLMObs.enable()` setup config. Programmatic `LLMObs.enable()/disable()` calls do not set the global `config._llmobs_enabled` boolean setting, which is only controlled by the `DD_LLMOBS_ENABLED` env var. This was problematic for users that relied on manual `LLMObs.enable()` setup (i.e. no env vars) because our distributed tracing injection code only checks the global config to inject llmobs parent IDs into request headers. If users manually enabled LLMObs without any env vars, then this would not be reflected in the global config value and thus LLMObs parent IDs would never be injected into the request headers. We can't check directly if LLMObs is enabled in the http injection module because: 1. This would require us to import significant product-specific LLMObs-code into the shared http injector helper module which would impact non-LLMObs users' app performance 2. Circular imports in LLMObs which imports http injector logic to use in its own helpers Instead of doing our check based on the global `config._llmobs_enabled` setting, we now send a tracing event to our shared product listeners, and register a corresponding `LLMObs._inject_llmobs_context()` hook to be called for all inject() calls if LLMObs is enabled (we check the LLMObs instance, not the global config setting value). ~One risk and why I don't like changing global config settings is because this then implies that it is no longer global or tied to an env var (I want to push for env var configuration where possible over manual overriding/enabling). If a global enabled config can be toggled indiscriminately then this could open a can of worms for enabling/disabling logic in our LLMObs service, which isn't really designed to be toggled on/off multiple times in the app's lifespan. However if some users cannot rely on env vars, then I don't see any other solution that does not couple tracer internal code with LLMObs code which is a no-option.~ (UPDATE: we avoided this issue by using signal dispatching) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) --- ddtrace/llmobs/_llmobs.py | 8 ++- ddtrace/propagation/http.py | 7 +-- ...nable-updates-config-45379a7a30e2e0e3.yaml | 5 ++ tests/llmobs/test_llmobs_service.py | 1 - tests/llmobs/test_propagation.py | 54 ++++++++++++------- tests/tracer/test_propagation.py | 24 --------- 6 files changed, 49 insertions(+), 50 deletions(-) create mode 100644 releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml diff --git a/ddtrace/llmobs/_llmobs.py b/ddtrace/llmobs/_llmobs.py index 768f4bdb292..bc5ddabcf00 100644 --- a/ddtrace/llmobs/_llmobs.py +++ b/ddtrace/llmobs/_llmobs.py @@ -285,6 +285,7 @@ def _stop_service(self) -> None: # Remove listener hooks for span events core.reset_listeners("trace.span_start", self._on_span_start) core.reset_listeners("trace.span_finish", self._on_span_finish) + core.reset_listeners("http.span_inject", self._inject_llmobs_context) forksafe.unregister(self._child_after_fork) @@ -369,6 +370,7 @@ def enable( # Register hooks for span events core.on("trace.span_start", cls._instance._on_span_start) core.on("trace.span_finish", cls._instance._on_span_finish) + core.on("http.span_inject", cls._instance._inject_llmobs_context) atexit.register(cls.disable) telemetry_writer.product_activated(TELEMETRY_APM_PRODUCT.LLMOBS, True) @@ -1162,6 +1164,11 @@ def submit_evaluation( cls._instance._llmobs_eval_metric_writer.enqueue(evaluation_metric) + def _inject_llmobs_context(self, span_context: Context, request_headers: Dict[str, str]) -> None: + if self.enabled is False: + return + _inject_llmobs_parent_id(span_context) + @classmethod def inject_distributed_headers(cls, request_headers: Dict[str, str], span: Optional[Span] = None) -> Dict[str, str]: """Injects the span's distributed context into the given request headers.""" @@ -1179,7 +1186,6 @@ def inject_distributed_headers(cls, request_headers: Dict[str, str], span: Optio if span is None: log.warning("No span provided and no currently active span found.") return request_headers - _inject_llmobs_parent_id(span.context) HTTPPropagator.inject(span.context, request_headers) return request_headers diff --git a/ddtrace/propagation/http.py b/ddtrace/propagation/http.py index fdaf97410ad..381acabb1bc 100644 --- a/ddtrace/propagation/http.py +++ b/ddtrace/propagation/http.py @@ -28,6 +28,7 @@ from ddtrace._trace.span import _get_64_lowest_order_bits_as_int from ddtrace._trace.span import _MetaDictType from ddtrace.appsec._constants import APPSEC +from ddtrace.internal.core import dispatch from ddtrace.settings.asm import config as asm_config from ..constants import AUTO_KEEP @@ -1052,6 +1053,7 @@ def parent_call(): :param dict headers: HTTP headers to extend with tracing attributes. :param Span non_active_span: Only to be used if injecting a non-active span. """ + dispatch("http.span_inject", (span_context, headers)) if not config._propagation_style_inject: return if non_active_span is not None and non_active_span.context is not span_context: @@ -1089,11 +1091,6 @@ def parent_call(): for key in span_context._baggage: headers[_HTTP_BAGGAGE_PREFIX + key] = span_context._baggage[key] - if config._llmobs_enabled: - from ddtrace.llmobs._utils import _inject_llmobs_parent_id - - _inject_llmobs_parent_id(span_context) - if PROPAGATION_STYLE_DATADOG in config._propagation_style_inject: _DatadogMultiHeader._inject(span_context, headers) if PROPAGATION_STYLE_B3_MULTI in config._propagation_style_inject: diff --git a/releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml b/releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml new file mode 100644 index 00000000000..4bf312f4680 --- /dev/null +++ b/releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + LLM Observability: Resolves an issue where explicitly only using ``LLMObs.enable()`` to configure LLM Observability + without environment variables would not automatically propagate distributed tracing headers. diff --git a/tests/llmobs/test_llmobs_service.py b/tests/llmobs/test_llmobs_service.py index de428999147..744fde885e9 100644 --- a/tests/llmobs/test_llmobs_service.py +++ b/tests/llmobs/test_llmobs_service.py @@ -60,7 +60,6 @@ def test_service_enable_proxy_default(): assert llmobs_instance.tracer == dummy_tracer assert isinstance(llmobs_instance._llmobs_span_writer._clients[0], LLMObsProxiedEventClient) assert run_llmobs_trace_filter(dummy_tracer) is not None - llmobs_service.disable() diff --git a/tests/llmobs/test_propagation.py b/tests/llmobs/test_propagation.py index e3ab9c80d66..27421e6b12f 100644 --- a/tests/llmobs/test_propagation.py +++ b/tests/llmobs/test_propagation.py @@ -57,20 +57,24 @@ def test_propagate_correct_llmobs_parent_id_simple(run_python_code_in_subprocess """ code = """ import json +import mock -from ddtrace import tracer -from ddtrace.ext import SpanTypes +from ddtrace.internal.utils.http import Response +from ddtrace.llmobs import LLMObs from ddtrace.propagation.http import HTTPPropagator -with tracer.trace("LLMObs span", span_type=SpanTypes.LLM) as root_span: - with tracer.trace("Non-LLMObs span") as child_span: - headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} - HTTPPropagator.inject(child_span.context, headers) +with mock.patch( + "ddtrace.internal.writer.HTTPWriter._send_payload", return_value=Response(status=200, body="{}"), +): + LLMObs.enable(ml_app="test-app", api_key="", agentless_enabled=True) + with LLMObs.workflow("LLMObs span") as root_span: + with LLMObs._instance.tracer.trace("Non-LLMObs span") as child_span: + headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} + HTTPPropagator.inject(child_span.context, headers) print(json.dumps(headers)) """ env = os.environ.copy() - env["DD_LLMOBS_ENABLED"] = "1" env["DD_TRACE_ENABLED"] = "0" stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) assert status == 0, (stdout, stderr) @@ -93,21 +97,33 @@ def test_propagate_llmobs_parent_id_complex(run_python_code_in_subprocess): """ code = """ import json +import mock -from ddtrace import tracer -from ddtrace.ext import SpanTypes +from ddtrace.internal.utils.http import Response +from ddtrace.llmobs import LLMObs from ddtrace.propagation.http import HTTPPropagator -with tracer.trace("LLMObs span", span_type=SpanTypes.LLM) as root_span: - with tracer.trace("Non-LLMObs span") as child_span: - headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} - HTTPPropagator.inject(child_span.context, headers) +with mock.patch( + "ddtrace.internal.writer.HTTPWriter._send_payload", return_value=Response(status=200, body="{}"), +): + from ddtrace import auto # simulate ddtrace-run startup to ensure env var configs also propagate + with LLMObs.workflow("LLMObs span") as root_span: + with LLMObs._instance.tracer.trace("Non-LLMObs span") as child_span: + headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} + HTTPPropagator.inject(child_span.context, headers) print(json.dumps(headers)) """ env = os.environ.copy() - env["DD_LLMOBS_ENABLED"] = "1" - env["DD_TRACE_ENABLED"] = "0" + env.update( + { + "DD_LLMOBS_ENABLED": "1", + "DD_TRACE_ENABLED": "0", + "DD_AGENTLESS_ENABLED": "1", + "DD_API_KEY": "", + "DD_LLMOBS_ML_APP": "test-app", + } + ) stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) assert status == 0, (stdout, stderr) assert stderr == b"", (stdout, stderr) @@ -124,7 +140,7 @@ def test_propagate_llmobs_parent_id_complex(run_python_code_in_subprocess): def test_no_llmobs_parent_id_propagated_if_no_llmobs_spans(run_python_code_in_subprocess): - """Test that the correct LLMObs parent ID (None) is extracted from the headers in a simple distributed scenario. + """Test that the correct LLMObs parent ID ('undefined') is extracted from headers in a simple distributed scenario. Service A (subprocess) has spans, but none are LLMObs spans. Service B (outside subprocess) has a LLMObs span. Service B's span should have no LLMObs parent ID as there are no LLMObs spans from service A. @@ -132,17 +148,17 @@ def test_no_llmobs_parent_id_propagated_if_no_llmobs_spans(run_python_code_in_su code = """ import json -from ddtrace import tracer +from ddtrace.llmobs import LLMObs from ddtrace.propagation.http import HTTPPropagator -with tracer.trace("Non-LLMObs span") as root_span: +LLMObs.enable(ml_app="ml-app", agentless_enabled=True, api_key="") +with LLMObs._instance.tracer.trace("Non-LLMObs span") as root_span: headers = {} HTTPPropagator.inject(root_span.context, headers) print(json.dumps(headers)) """ env = os.environ.copy() - env["DD_LLMOBS_ENABLED"] = "1" env["DD_TRACE_ENABLED"] = "0" stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) assert status == 0, (stdout, stderr) diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index 533e4974250..b073cd72e3b 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -4,7 +4,6 @@ import os import pickle -import mock import pytest import ddtrace @@ -3387,29 +3386,6 @@ def test_DD_TRACE_PROPAGATION_STYLE_INJECT_overrides_DD_TRACE_PROPAGATION_STYLE( assert result == expected_headers -def test_llmobs_enabled_injects_llmobs_parent_id(): - with override_global_config(dict(_llmobs_enabled=True)): - with mock.patch("ddtrace.llmobs._utils._inject_llmobs_parent_id") as mock_llmobs_inject: - context = Context(trace_id=1, span_id=2) - HTTPPropagator.inject(context, {}) - mock_llmobs_inject.assert_called_once_with(context) - - -def test_llmobs_disabled_does_not_inject_parent_id(): - with override_global_config(dict(_llmobs_enabled=False)): - with mock.patch("ddtrace.llmobs._utils._inject_llmobs_parent_id") as mock_llmobs_inject: - context = Context(trace_id=1, span_id=2) - HTTPPropagator.inject(context, {}) - mock_llmobs_inject.assert_not_called() - - -def test_llmobs_parent_id_not_injected_by_default(): - with mock.patch("ddtrace.llmobs._utils._inject_llmobs_parent_id") as mock_llmobs_inject: - context = Context(trace_id=1, span_id=2) - HTTPPropagator.inject(context, {}) - mock_llmobs_inject.assert_not_called() - - @pytest.mark.parametrize( "span_context,expected_headers", [ From 894bb19ca5f7f1e7ba3a800f3dc5b709dbed765e Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 29 Jan 2025 13:56:58 +0100 Subject: [PATCH 24/31] restore --- ddtrace/appsec/_processor.py | 1 - tests/appsec/integrations/django_tests/test_django_appsec.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ddtrace/appsec/_processor.py b/ddtrace/appsec/_processor.py index 44cba6f04ba..83b65584a66 100644 --- a/ddtrace/appsec/_processor.py +++ b/ddtrace/appsec/_processor.py @@ -172,7 +172,6 @@ def __post_init__(self) -> None: def delayed_init(self) -> None: try: - asm_config._bypass_instrumentation_for_waf = True if self._rules is not None and not hasattr(self, "_ddwaf"): from ddtrace.appsec._ddwaf import DDWaf # noqa: E402 import ddtrace.appsec._metrics as metrics # noqa: E402 diff --git a/tests/appsec/integrations/django_tests/test_django_appsec.py b/tests/appsec/integrations/django_tests/test_django_appsec.py index 41540a375d9..e8515a75a2c 100644 --- a/tests/appsec/integrations/django_tests/test_django_appsec.py +++ b/tests/appsec/integrations/django_tests/test_django_appsec.py @@ -186,7 +186,7 @@ def test_django_login_sucess_identification(client, test_spans, tracer, use_logi assert get_user(client).is_authenticated login_span = test_spans.find_span(name="django.contrib.auth.login") assert login_span - assert login_span.get_tag(user.ID) == "1", login_span.get_tag(user.ID) + assert login_span.get_tag(user.ID) == "1" assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".success.track") == "true" assert login_span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_SUCCESS_MODE) == LOGIN_EVENTS_MODE.IDENT if use_login: From d3d715e859a09d0510973246aee1114b0a097d0f Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 29 Jan 2025 14:50:03 +0100 Subject: [PATCH 25/31] remove dispatch set_user --- ddtrace/appsec/_trace_utils.py | 15 ++++++--------- ddtrace/contrib/internal/trace_utils.py | 5 ----- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index eeaf9909033..5e8a68e2f69 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -397,15 +397,12 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co user_extra["login"] = user_id user_id = user_id_found or user_id if result_user and in_asm_context() and result_user.is_authenticated: - try: - if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): - set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True) - elif mode == LOGIN_EVENTS_MODE.IDENT: - set_user( - pin.tracer, str(user_id), propagate=True, email=user_extra.get("email"), name=user_extra.get("name") - ) - except Exception: # nosec - pass + if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): + set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True) + elif mode == LOGIN_EVENTS_MODE.IDENT: + set_user( + pin.tracer, str(user_id), propagate=True, email=user_extra.get("email"), name=user_extra.get("name") + ) real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { "REQUEST_USER_ID": str(user_id) if user_id else None, diff --git a/ddtrace/contrib/internal/trace_utils.py b/ddtrace/contrib/internal/trace_utils.py index 5781c3f30df..14bde157972 100644 --- a/ddtrace/contrib/internal/trace_utils.py +++ b/ddtrace/contrib/internal/trace_utils.py @@ -666,11 +666,6 @@ def set_user( if session_id: span.set_tag_str(user.SESSION_ID, session_id) - if asm_config._asm_enabled: - exc = core.dispatch_with_results("set_user_for_asm", [tracer, user_id]).block_user.exception - if exc: - raise exc - else: log.warning( "No root span in the current execution. Skipping set_user tags. " From 2e20bca7fd058ecfeb9d3d167be844dd9ed7429e Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Wed, 29 Jan 2025 17:48:38 +0100 Subject: [PATCH 26/31] ensure that we don't call the waf twice for the same event --- ddtrace/appsec/_trace_utils.py | 32 +++++++++++++++---------- ddtrace/appsec/trace_utils/__init__.py | 2 ++ ddtrace/contrib/internal/trace_utils.py | 6 +++++ 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 5e8a68e2f69..20fa72e6173 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -136,7 +136,7 @@ def track_user_login_success_event( if login_events_mode != LOGIN_EVENTS_MODE.SDK: span.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) - set_user(tracer, user_id, name, email, scope, role, session_id, propagate, span) + set_user(tracer, user_id, name, email, scope, role, session_id, propagate, span, may_block=False) if in_asm_context(): res = call_waf_callback( custom_data={ @@ -396,22 +396,28 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co if user_extra.get("login") is None: user_extra["login"] = user_id user_id = user_id_found or user_id - if result_user and in_asm_context() and result_user.is_authenticated: + if result_user and result_user.is_authenticated: if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): - set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True) + set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True, may_block=False) elif mode == LOGIN_EVENTS_MODE.IDENT: set_user( - pin.tracer, str(user_id), propagate=True, email=user_extra.get("email"), name=user_extra.get("name") + pin.tracer, + str(user_id), + propagate=True, + email=user_extra.get("email"), + name=user_extra.get("name"), + may_block=False, ) - real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode - custom_data = { - "REQUEST_USER_ID": str(user_id) if user_id else None, - "REQUEST_USERNAME": user_extra.get("login"), - "LOGIN_SUCCESS": real_mode, - } - res = call_waf_callback(custom_data=custom_data, force_sent=True) - if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): - raise BlockingException(get_blocked()) + if in_asm_context(): + real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode + custom_data = { + "REQUEST_USER_ID": str(user_id) if user_id else None, + "REQUEST_USERNAME": user_extra.get("login"), + "LOGIN_SUCCESS": real_mode, + } + res = call_waf_callback(custom_data=custom_data, force_sent=True) + if res and any(action in [WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION] for action in res.actions): + raise BlockingException(get_blocked()) core.on("django.login", _on_django_login) diff --git a/ddtrace/appsec/trace_utils/__init__.py b/ddtrace/appsec/trace_utils/__init__.py index 25559d7f0e9..749480429c1 100644 --- a/ddtrace/appsec/trace_utils/__init__.py +++ b/ddtrace/appsec/trace_utils/__init__.py @@ -1,3 +1,5 @@ +"""Public API for User events""" + from ddtrace.appsec._trace_utils import block_request # noqa: F401 from ddtrace.appsec._trace_utils import block_request_if_user_blocked # noqa: F401 from ddtrace.appsec._trace_utils import should_block_user # noqa: F401 diff --git a/ddtrace/contrib/internal/trace_utils.py b/ddtrace/contrib/internal/trace_utils.py index 14bde157972..01fea989d4e 100644 --- a/ddtrace/contrib/internal/trace_utils.py +++ b/ddtrace/contrib/internal/trace_utils.py @@ -639,6 +639,7 @@ def set_user( session_id=None, # type: Optional[str] propagate=False, # type bool span=None, # type: Optional[Span] + may_block=True, # type: bool ): # type: (...) -> None """Set user tags. @@ -666,6 +667,11 @@ def set_user( if session_id: span.set_tag_str(user.SESSION_ID, session_id) + if may_block and asm_config._asm_enabled: + exc = core.dispatch_with_results("set_user_for_asm", [tracer, user_id]).block_user.exception + if exc: + raise exc + else: log.warning( "No root span in the current execution. Skipping set_user tags. " From 89baa2232de38fa222b2e5f9e00e68fc34634a25 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 30 Jan 2025 14:12:32 +0100 Subject: [PATCH 27/31] revert main changes --- ddtrace/_trace/trace_handlers.py | 56 +--------- .../contrib/internal/aiohttp/middlewares.py | 105 ++++++++++-------- ddtrace/contrib/internal/bottle/trace.py | 61 +++++----- ddtrace/contrib/internal/falcon/middleware.py | 61 ++++++---- ddtrace/contrib/internal/molten/patch.py | 52 +++++---- ddtrace/contrib/internal/pyramid/trace.py | 72 ++++++------ ddtrace/contrib/internal/sanic/patch.py | 61 +++++----- ...b.cherrypy.test_middleware.test_child.json | 1 - ...b.cherrypy.test_middleware.test_error.json | 1 - ...b.cherrypy.test_middleware.test_fatal.json | 1 - ...cherrypy.test_middleware.test_success.json | 1 - ...ver.test_multiple_requests_sanic_http.json | 2 - ...multiple_requests_sanic_http_pre_21.9.json | 2 - ...c.test_sanic_server.test_sanic_errors.json | 2 - ...nic_server.test_sanic_errors_pre_21.9.json | 2 - 15 files changed, 219 insertions(+), 261 deletions(-) diff --git a/ddtrace/_trace/trace_handlers.py b/ddtrace/_trace/trace_handlers.py index e0d99c0d020..89e31fa4e97 100644 --- a/ddtrace/_trace/trace_handlers.py +++ b/ddtrace/_trace/trace_handlers.py @@ -109,14 +109,11 @@ def _get_parameters_for_new_span_directly_from_context(ctx: core.ExecutionContex def _start_span(ctx: core.ExecutionContext, call_trace: bool = True, **kwargs) -> "Span": span_kwargs = _get_parameters_for_new_span_directly_from_context(ctx) call_trace = ctx.get_item("call_trace", call_trace) - tracer = ctx.get_item("tracer") or (ctx.get_item("middleware") or ctx["pin"]).tracer + tracer = (ctx.get_item("middleware") or ctx["pin"]).tracer distributed_headers_config = ctx.get_item("distributed_headers_config") if distributed_headers_config: trace_utils.activate_distributed_headers( - tracer, - int_config=distributed_headers_config, - request_headers=ctx["distributed_headers"], - override=ctx.get_item("distributed_headers_config_override"), + tracer, int_config=distributed_headers_config, request_headers=ctx["distributed_headers"] ) distributed_context = ctx.get_item("distributed_context") if distributed_context and not call_trace: @@ -129,42 +126,6 @@ def _start_span(ctx: core.ExecutionContext, call_trace: bool = True, **kwargs) - return span -def _set_web_frameworks_tags(ctx, span, int_config): - span.set_tag_str(COMPONENT, int_config.integration_name) - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - span.set_tag(_SPAN_MEASURED_KEY) - - analytics_enabled = ctx.get_item("analytics_enabled") - analytics_sample_rate = ctx.get_item("analytics_sample_rate", True) - - # Configure trace search sample rate - if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, analytics_sample_rate) - - -def _on_web_framework_start_request(ctx, int_config): - request_span = ctx.get_item("req_span") - _set_web_frameworks_tags(ctx, request_span, int_config) - - -def _on_web_framework_finish_request( - span, int_config, method, url, status_code, query, req_headers, res_headers, route, finish -): - trace_utils.set_http_meta( - span=span, - integration_config=int_config, - method=method, - url=url, - status_code=status_code, - query=query, - request_headers=req_headers, - response_headers=res_headers, - route=route, - ) - if finish: - span.finish() - - def _on_traced_request_context_started_flask(ctx): current_span = ctx["pin"].tracer.current_span() if not ctx["pin"].enabled or not current_span: @@ -800,10 +761,6 @@ def listen(): core.on("azure.functions.request_call_modifier", _on_azure_functions_request_span_modifier) core.on("azure.functions.start_response", _on_azure_functions_start_response) - # web frameworks general handlers - core.on("web.request.start", _on_web_framework_start_request) - core.on("web.request.finish", _on_web_framework_finish_request) - core.on("test_visibility.enable", _on_test_visibility_enable) core.on("test_visibility.disable", _on_test_visibility_disable) core.on("test_visibility.is_enabled", _on_test_visibility_is_enabled, "is_enabled") @@ -812,14 +769,6 @@ def listen(): core.on("rq.queue.enqueue_job", _propagate_context) for context_name in ( - # web frameworks - "aiohttp.request", - "bottle.request", - "cherrypy.request", - "falcon.request", - "molten.request", - "pyramid.request", - "sanic.request", "flask.call", "flask.jsonify", "flask.render_template", @@ -830,7 +779,6 @@ def listen(): "django.template.render", "django.process_exception", "django.func.wrapped", - # non web frameworks "botocore.instrumented_api_call", "botocore.instrumented_lib_function", "botocore.patched_kinesis_api_call", diff --git a/ddtrace/contrib/internal/aiohttp/middlewares.py b/ddtrace/contrib/internal/aiohttp/middlewares.py index 63a60734d3b..ddb2d35fbc6 100644 --- a/ddtrace/contrib/internal/aiohttp/middlewares.py +++ b/ddtrace/contrib/internal/aiohttp/middlewares.py @@ -2,10 +2,15 @@ from aiohttp.web_urldispatcher import SystemRoute from ddtrace import config +from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY +from ddtrace.constants import _SPAN_MEASURED_KEY +from ddtrace.constants import SPAN_KIND +from ddtrace.contrib import trace_utils from ddtrace.contrib.asyncio import context_provider +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.ext import http -from ddtrace.internal import core +from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection @@ -30,42 +35,47 @@ async def attach_context(request): # application configs tracer = app[CONFIG_KEY]["tracer"] service = app[CONFIG_KEY]["service"] - # DEV: aiohttp is special case maintains separate configuration from config api - analytics_enabled = app[CONFIG_KEY]["analytics_enabled"] + distributed_tracing = app[CONFIG_KEY]["distributed_tracing_enabled"] # Create a new context based on the propagated information. - - with core.context_with_data( - "aiohttp.request", - span_name=schematize_url_operation("aiohttp.request", protocol="http", direction=SpanDirection.INBOUND), - span_type=SpanTypes.WEB, + trace_utils.activate_distributed_headers( + tracer, + int_config=config.aiohttp, + request_headers=request.headers, + override=distributed_tracing, + ) + + # trace the handler + request_span = tracer.trace( + schematize_url_operation("aiohttp.request", protocol="http", direction=SpanDirection.INBOUND), service=service, - tags={}, - tracer=tracer, - distributed_headers=request.headers, - distributed_headers_config=config.aiohttp, - distributed_headers_config_override=app[CONFIG_KEY]["distributed_tracing_enabled"], - headers_case_sensitive=True, - analytics_enabled=analytics_enabled, - analytics_sample_rate=app[CONFIG_KEY].get("analytics_sample_rate", True), - ) as ctx: - req_span = ctx.span - - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.aiohttp)) - - # attach the context and the root span to the request; the Context - # may be freely used by the application code - request[REQUEST_CONTEXT_KEY] = req_span.context - request[REQUEST_SPAN_KEY] = req_span - request[REQUEST_CONFIG_KEY] = app[CONFIG_KEY] - try: - response = await handler(request) - if isinstance(response, web.StreamResponse): - request.task.add_done_callback(lambda _: finish_request_span(request, response)) - return response - except Exception: - req_span.set_traceback() - raise + span_type=SpanTypes.WEB, + ) + request_span.set_tag(_SPAN_MEASURED_KEY) + + request_span.set_tag_str(COMPONENT, config.aiohttp.integration_name) + + # set span.kind tag equal to type of request + request_span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + + # Configure trace search sample rate + # DEV: aiohttp is special case maintains separate configuration from config api + analytics_enabled = app[CONFIG_KEY]["analytics_enabled"] + if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: + request_span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, app[CONFIG_KEY].get("analytics_sample_rate", True)) + + # attach the context and the root span to the request; the Context + # may be freely used by the application code + request[REQUEST_CONTEXT_KEY] = request_span.context + request[REQUEST_SPAN_KEY] = request_span + request[REQUEST_CONFIG_KEY] = app[CONFIG_KEY] + try: + response = await handler(request) + if isinstance(response, web.StreamResponse): + request.task.add_done_callback(lambda _: finish_request_span(request, response)) + return response + except Exception: + request_span.set_traceback() + raise return attach_context @@ -112,22 +122,19 @@ def finish_request_span(request, response): # SystemRoute objects exist to throw HTTP errors and have no path route = aiohttp_route.resource.canonical - core.dispatch( - "web.request.finish", - ( - request_span, - config.aiohttp, - request.method, - str(request.url), # DEV: request.url is a yarl's URL object - response.status, - None, # query arg = None - request.headers, - response.headers, - route, - True, - ), + trace_utils.set_http_meta( + request_span, + config.aiohttp, + method=request.method, + url=str(request.url), # DEV: request.url is a yarl's URL object + status_code=response.status, + request_headers=request.headers, + response_headers=response.headers, + route=route, ) + request_span.finish() + async def on_prepare(request, response): """ diff --git a/ddtrace/contrib/internal/bottle/trace.py b/ddtrace/contrib/internal/bottle/trace.py index 58778a36ee1..3aabb4ccc81 100644 --- a/ddtrace/contrib/internal/bottle/trace.py +++ b/ddtrace/contrib/internal/bottle/trace.py @@ -5,8 +5,13 @@ import ddtrace from ddtrace import config +from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY +from ddtrace.constants import _SPAN_MEASURED_KEY +from ddtrace.constants import SPAN_KIND +from ddtrace.contrib import trace_utils +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal import core +from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection from ddtrace.internal.utils.formats import asbool @@ -37,21 +42,24 @@ def wrapped(*args, **kwargs): resource = "{} {}".format(request.method, route.rule) - with core.context_with_data( - "bottle.request", - span_name=schematize_url_operation("bottle.request", protocol="http", direction=SpanDirection.INBOUND), - span_type=SpanTypes.WEB, + trace_utils.activate_distributed_headers( + self.tracer, int_config=config.bottle, request_headers=request.headers + ) + + with self.tracer.trace( + schematize_url_operation("bottle.request", protocol="http", direction=SpanDirection.INBOUND), service=self.service, resource=resource, - tags={}, - tracer=self.tracer, - distributed_headers=request.headers, - distributed_headers_config=config.bottle, - headers_case_sensitive=True, - analytics_sample_rate=config.bottle.get_analytics_sample_rate(use_global_config=True), - ) as ctx, ctx.span as req_span: - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.bottle)) + span_type=SpanTypes.WEB, + ) as s: + s.set_tag_str(COMPONENT, config.bottle.integration_name) + + # set span.kind to the type of request being performed + s.set_tag_str(SPAN_KIND, SpanKind.SERVER) + + s.set_tag(_SPAN_MEASURED_KEY) + # set analytics sample rate with global config enabled + s.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.bottle.get_analytics_sample_rate(use_global_config=True)) code = None result = None @@ -83,21 +91,16 @@ def wrapped(*args, **kwargs): method = request.method url = request.urlparts._replace(query="").geturl() full_route = "/".join([request.script_name.rstrip("/"), route.rule.lstrip("/")]) - - core.dispatch( - "web.request.finish", - ( - req_span, - config.bottle, - method, - url, - response_code, - request.query_string, - request.headers, - response.headers, - full_route, - False, - ), + trace_utils.set_http_meta( + s, + config.bottle, + method=method, + url=url, + status_code=response_code, + query=request.query_string, + request_headers=request.headers, + response_headers=response.headers, + route=full_route, ) return wrapped diff --git a/ddtrace/contrib/internal/falcon/middleware.py b/ddtrace/contrib/internal/falcon/middleware.py index 513ce6cce39..b4ec5434777 100644 --- a/ddtrace/contrib/internal/falcon/middleware.py +++ b/ddtrace/contrib/internal/falcon/middleware.py @@ -1,8 +1,14 @@ import sys from ddtrace import config +from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY +from ddtrace.constants import _SPAN_MEASURED_KEY +from ddtrace.constants import SPAN_KIND +from ddtrace.contrib import trace_utils +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal import core +from ddtrace.ext import http as httpx +from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import SpanDirection from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -21,27 +27,26 @@ def __init__(self, tracer, service=None, distributed_tracing=None): def process_request(self, req, resp): # Falcon uppercases all header names. headers = dict((k.lower(), v) for k, v in req.headers.items()) + trace_utils.activate_distributed_headers(self.tracer, int_config=config.falcon, request_headers=headers) - with core.context_with_data( - "falcon.request", - span_name=schematize_url_operation("falcon.request", protocol="http", direction=SpanDirection.INBOUND), - span_type=SpanTypes.WEB, + span = self.tracer.trace( + schematize_url_operation("falcon.request", protocol="http", direction=SpanDirection.INBOUND), service=self.service, - tags={}, - tracer=self.tracer, - distributed_headers=headers, - distributed_headers_config=config.falcon, - headers_case_sensitive=True, - analytics_sample_rate=config.falcon.get_analytics_sample_rate(use_global_config=True), - ) as ctx: - req_span = ctx.span - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.falcon)) - - core.dispatch( - "web.request.finish", - (req_span, config.falcon, req.method, req.url, None, req.query_string, req.headers, None, None, False), - ) + span_type=SpanTypes.WEB, + ) + span.set_tag_str(COMPONENT, config.falcon.integration_name) + + # set span.kind to the type of operation being performed + span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + + span.set_tag(_SPAN_MEASURED_KEY) + + # set analytics sample rate with global config enabled + span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.falcon.get_analytics_sample_rate(use_global_config=True)) + + trace_utils.set_http_meta( + span, config.falcon, method=req.method, url=req.url, query=req.query_string, request_headers=req.headers + ) def process_resource(self, req, resp, resource, params): span = self.tracer.current_span() @@ -64,7 +69,8 @@ def process_response(self, req, resp, resource, req_succeeded=None): if resource is None: status = "404" span.resource = "%s 404" % req.method - core.dispatch("web.request.finish", (span, config.falcon, None, None, status, None, None, None, None, True)) + span.set_tag(httpx.STATUS_CODE, status) + span.finish() return err_type = sys.exc_info()[0] @@ -81,13 +87,20 @@ def process_response(self, req, resp, resource, req_succeeded=None): route = req.root_path or "" + req.uri_template + trace_utils.set_http_meta( + span, + config.falcon, + status_code=status, + response_headers=resp._headers, + route=route, + ) + # Emit span hook for this response # DEV: Emit before closing so they can overwrite `span.resource` if they want config.falcon.hooks.emit("request", span, req, resp) - core.dispatch( - "web.request.finish", (span, config.falcon, None, None, status, None, None, resp._headers, route, True) - ) + # Close the span + span.finish() def _is_404(err_type): diff --git a/ddtrace/contrib/internal/molten/patch.py b/ddtrace/contrib/internal/molten/patch.py index dfec47eb17d..38fa949243c 100644 --- a/ddtrace/contrib/internal/molten/patch.py +++ b/ddtrace/contrib/internal/molten/patch.py @@ -5,11 +5,15 @@ from wrapt import wrap_function_wrapper as _w from ddtrace import config +from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY +from ddtrace.constants import _SPAN_MEASURED_KEY +from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils from ddtrace.contrib.internal.trace_utils import unwrap as _u +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal import core from ddtrace.internal.compat import urlencode +from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection @@ -85,21 +89,25 @@ def patch_app_call(wrapped, instance, args, kwargs): request = molten.http.Request.from_environ(environ) resource = func_name(wrapped) - with core.context_with_data( - "molten.request", - span_name=schematize_url_operation("molten.request", protocol="http", direction=SpanDirection.INBOUND), - span_type=SpanTypes.WEB, + # request.headers is type Iterable[Tuple[str, str]] + trace_utils.activate_distributed_headers( + pin.tracer, int_config=config.molten, request_headers=dict(request.headers) + ) + + with pin.tracer.trace( + schematize_url_operation("molten.request", protocol="http", direction=SpanDirection.INBOUND), service=trace_utils.int_service(pin, config.molten), resource=resource, - tags={}, - tracer=pin.tracer, - distributed_headers=dict(request.headers), # request.headers is type Iterable[Tuple[str, str]] - distributed_headers_config=config.molten, - headers_case_sensitive=True, - analytics_sample_rate=config.molten.get_analytics_sample_rate(use_global_config=True), - ) as ctx, ctx.span as req_span: - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.molten)) + span_type=SpanTypes.WEB, + ) as span: + span.set_tag_str(COMPONENT, config.molten.integration_name) + + # set span.kind tag equal to type of operation being performed + span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + + span.set_tag(_SPAN_MEASURED_KEY) + # set analytics sample rate with global config enabled + span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.molten.get_analytics_sample_rate(use_global_config=True)) @wrapt.function_wrapper def _w_start_response(wrapped, instance, args, kwargs): @@ -117,13 +125,11 @@ def _w_start_response(wrapped, instance, args, kwargs): except ValueError: pass - if not req_span.get_tag(MOLTEN_ROUTE): + if not span.get_tag(MOLTEN_ROUTE): # if route never resolve, update root resource - req_span.resource = "{} {}".format(request.method, code) + span.resource = "{} {}".format(request.method, code) - core.dispatch( - "web.request.finish", (req_span, config.molten, None, None, code, None, None, None, None, False) - ) + trace_utils.set_http_meta(span, config.molten, status_code=code) return wrapped(*args, **kwargs) @@ -137,13 +143,11 @@ def _w_start_response(wrapped, instance, args, kwargs): request.path, ) query = urlencode(dict(request.params)) - - core.dispatch( - "web.request.finish", - (req_span, config.molten, request.method, url, None, query, request.headers, None, None, False), + trace_utils.set_http_meta( + span, config.molten, method=request.method, url=url, query=query, request_headers=request.headers ) - req_span.set_tag_str("molten.version", molten.__version__) + span.set_tag_str("molten.version", molten.__version__) return wrapped(environ, start_response, **kwargs) diff --git a/ddtrace/contrib/internal/pyramid/trace.py b/ddtrace/contrib/internal/pyramid/trace.py index 9b221bd3f09..9942c673d4e 100644 --- a/ddtrace/contrib/internal/pyramid/trace.py +++ b/ddtrace/contrib/internal/pyramid/trace.py @@ -6,8 +6,12 @@ # project import ddtrace from ddtrace import config +from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY +from ddtrace.constants import _SPAN_MEASURED_KEY +from ddtrace.constants import SPAN_KIND +from ddtrace.contrib import trace_utils +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal import core from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name @@ -63,30 +67,29 @@ def trace_tween_factory(handler, registry): service = settings.get(SETTINGS_SERVICE) or schematize_service_name("pyramid") tracer = settings.get(SETTINGS_TRACER) or ddtrace.tracer enabled = asbool(settings.get(SETTINGS_TRACE_ENABLED, tracer.enabled)) - - # ensure distributed tracing within pyramid settings matches config - config.pyramid.distributed_tracing_enabled = asbool(settings.get(SETTINGS_DISTRIBUTED_TRACING, True)) + distributed_tracing = asbool(settings.get(SETTINGS_DISTRIBUTED_TRACING, True)) if enabled: # make a request tracing function def trace_tween(request): - with core.context_with_data( - "pyramid.request", - span_name=schematize_url_operation("pyramid.request", protocol="http", direction=SpanDirection.INBOUND), - span_type=SpanTypes.WEB, - service=service, - resource="404", - tags={}, - tracer=tracer, - distributed_headers=request.headers, - distributed_headers_config=config.pyramid, - headers_case_sensitive=True, + trace_utils.activate_distributed_headers( + tracer, int_config=config.pyramid, request_headers=request.headers, override=distributed_tracing + ) + + span_name = schematize_url_operation("pyramid.request", protocol="http", direction=SpanDirection.INBOUND) + with tracer.trace(span_name, service=service, resource="404", span_type=SpanTypes.WEB) as span: + span.set_tag_str(COMPONENT, config.pyramid.integration_name) + + # set span.kind to the type of operation being performed + span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + + span.set_tag(_SPAN_MEASURED_KEY) + # Configure trace search sample rate # DEV: pyramid is special case maintains separate configuration from config api - analytics_enabled=settings.get(SETTINGS_ANALYTICS_ENABLED), - analytics_sample_rate=settings.get(SETTINGS_ANALYTICS_SAMPLE_RATE, True), - ) as ctx, ctx.span as req_span: - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.pyramid)) + analytics_enabled = settings.get(SETTINGS_ANALYTICS_ENABLED) + + if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: + span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, settings.get(SETTINGS_ANALYTICS_SAMPLE_RATE, True)) setattr(request, DD_TRACER, tracer) # used to find the tracer in templates response = None @@ -107,8 +110,8 @@ def trace_tween(request): finally: # set request tags if request.matched_route: - req_span.resource = "{} {}".format(request.method, request.matched_route.name) - req_span.set_tag_str("pyramid.route.name", request.matched_route.name) + span.resource = "{} {}".format(request.method, request.matched_route.name) + span.set_tag_str("pyramid.route.name", request.matched_route.name) # set response tags if response: status = response.status_code @@ -116,22 +119,17 @@ def trace_tween(request): else: response_headers = None - core.dispatch( - "web.request.finish", - ( - req_span, - config.pyramid, - request.method, - request.path_url, - status, - request.query_string, - request.headers, - response_headers, - request.matched_route.pattern if request.matched_route else None, - False, - ), + trace_utils.set_http_meta( + span, + config.pyramid, + method=request.method, + url=request.path_url, + status_code=status, + query=request.query_string, + request_headers=request.headers, + response_headers=response_headers, + route=request.matched_route.pattern if request.matched_route else None, ) - return response return trace_tween diff --git a/ddtrace/contrib/internal/sanic/patch.py b/ddtrace/contrib/internal/sanic/patch.py index 1babf7d44a0..8e53ed41dc8 100644 --- a/ddtrace/contrib/internal/sanic/patch.py +++ b/ddtrace/contrib/internal/sanic/patch.py @@ -4,10 +4,14 @@ import wrapt from wrapt import wrap_function_wrapper as _w +import ddtrace from ddtrace import config +from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY +from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal import core +from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -43,10 +47,7 @@ def update_span(span, response): # and so use 500 status_code = getattr(response, "status", 500) response_headers = getattr(response, "headers", None) - - core.dispatch( - "web.request.finish", (span, config.sanic, None, None, status_code, None, None, response_headers, None, False) - ) + trace_utils.set_http_meta(span, config.sanic, status_code=status_code, response_headers=response_headers) def _wrap_response_callback(span, callback): @@ -199,35 +200,31 @@ def _create_sanic_request_span(request): headers = request.headers.copy() - with core.context_with_data( - "sanic.request", - span_name=schematize_url_operation("sanic.request", protocol="http", direction=SpanDirection.INBOUND), - span_type=SpanTypes.WEB, + trace_utils.activate_distributed_headers(ddtrace.tracer, int_config=config.sanic, request_headers=headers) + + span = pin.tracer.trace( + schematize_url_operation("sanic.request", protocol="http", direction=SpanDirection.INBOUND), service=trace_utils.int_service(None, config.sanic), resource=resource, - tags={}, - pin=pin, - distributed_headers=headers, - distributed_headers_config=config.sanic, - headers_case_sensitive=True, - analytics_sample_rate=config.sanic.get_analytics_sample_rate(use_global_config=True), - ) as ctx: - req_span = ctx.span - - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.sanic)) - - method = request.method - url = "{scheme}://{host}{path}".format(scheme=request.scheme, host=request.host, path=request.path) - query_string = request.query_string - if isinstance(query_string, bytes): - query_string = query_string.decode() - - core.dispatch( - "web.request.finish", (req_span, config.sanic, method, url, None, query_string, headers, None, None, False) - ) - - return req_span + span_type=SpanTypes.WEB, + ) + span.set_tag_str(COMPONENT, config.sanic.integration_name) + + # set span.kind to the type of operation being performed + span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + + sample_rate = config.sanic.get_analytics_sample_rate(use_global_config=True) + if sample_rate is not None: + span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, sample_rate) + + method = request.method + url = "{scheme}://{host}{path}".format(scheme=request.scheme, host=request.host, path=request.path) + query_string = request.query_string + if isinstance(query_string, bytes): + query_string = query_string.decode() + trace_utils.set_http_meta(span, config.sanic, method=method, url=url, query=query_string, request_headers=headers) + + return span async def sanic_http_lifecycle_handle(request): diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json index 933eda3e420..1887282b06f 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json @@ -22,7 +22,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json index 58fde524828..49759fee3d3 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json @@ -25,7 +25,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json index a5a7c1ed14f..4153c8c2b18 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json @@ -25,7 +25,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json index ce13988add0..d1eb9fc5bee 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json @@ -22,7 +22,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json index 66ca28d9606..3803d07f360 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json @@ -23,7 +23,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -73,7 +72,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json index aca376a974a..4aac2721c02 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json @@ -22,7 +22,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -71,7 +70,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json index ab2aeaec920..435bbbc7b23 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json @@ -22,7 +22,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -59,7 +58,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json index ceabecd1ae7..c03813a43d6 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json @@ -22,7 +22,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -55,7 +54,6 @@ "span.kind": "server" }, "metrics": { - "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, From 346e026a79a3434c3b9b0efad2157429f76f41b5 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 30 Jan 2025 14:13:35 +0100 Subject: [PATCH 28/31] revert main changes --- ddtrace/contrib/internal/cherrypy/patch.py | 53 ++++++++++------------ 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/ddtrace/contrib/internal/cherrypy/patch.py b/ddtrace/contrib/internal/cherrypy/patch.py index 70f43059905..909a043175f 100644 --- a/ddtrace/contrib/internal/cherrypy/patch.py +++ b/ddtrace/contrib/internal/cherrypy/patch.py @@ -11,10 +11,12 @@ from ddtrace.constants import ERROR_MSG from ddtrace.constants import ERROR_STACK from ddtrace.constants import ERROR_TYPE +from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils +from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.internal import compat -from ddtrace.internal import core +from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import SpanDirection from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -75,23 +77,20 @@ def _setup(self): cherrypy.request.hooks.attach("after_error_response", self._after_error_response, priority=5) def _on_start_resource(self): - with core.context_with_data( - "cherrypy.request", - span_name=SPAN_NAME, - span_type=SpanTypes.WEB, + trace_utils.activate_distributed_headers( + self._tracer, int_config=config.cherrypy, request_headers=cherrypy.request.headers + ) + + cherrypy.request._datadog_span = self._tracer.trace( + SPAN_NAME, service=trace_utils.int_service(None, config.cherrypy, default="cherrypy"), - tags={}, - tracer=self._tracer, - distributed_headers=cherrypy.request.headers, - distributed_headers_config=config.cherrypy, - headers_case_sensitive=True, - ) as ctx: - req_span = ctx.span + span_type=SpanTypes.WEB, + ) - ctx.set_item("req_span", req_span) - core.dispatch("web.request.start", (ctx, config.cherrypy)) + cherrypy.request._datadog_span.set_tag_str(COMPONENT, config.cherrypy.integration_name) - cherrypy.request._datadog_span = req_span + # set span.kind to the type of request being performed + cherrypy.request._datadog_span.set_tag_str(SPAN_KIND, SpanKind.SERVER) def _after_error_response(self): span = getattr(cherrypy.request, "_datadog_span", None) @@ -136,22 +135,18 @@ def _close_span(self, span): url = compat.to_unicode(cherrypy.request.base + cherrypy.request.path_info) status_code, _, _ = valid_status(cherrypy.response.status) - core.dispatch( - "web.request.finish", - ( - span, - config.cherrypy, - cherrypy.request.method, - url, - status_code, - None, - cherrypy.request.headers, - cherrypy.response.headers, - None, - True, - ), + trace_utils.set_http_meta( + span, + config.cherrypy, + method=cherrypy.request.method, + url=url, + status_code=status_code, + request_headers=cherrypy.request.headers, + response_headers=cherrypy.response.headers, ) + span.finish() + # Clear our span just in case. cherrypy.request._datadog_span = None From 48a53a371b2b4628c455c6c9a473c41d8ab4bd77 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 30 Jan 2025 14:37:32 +0100 Subject: [PATCH 29/31] add appsec.user.id for auto collection on already authenticated and user.collection_mode tags --- ddtrace/appsec/_constants.py | 1 + ddtrace/appsec/_trace_utils.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ddtrace/appsec/_constants.py b/ddtrace/appsec/_constants.py index 8fd812a888b..2172548205b 100644 --- a/ddtrace/appsec/_constants.py +++ b/ddtrace/appsec/_constants.py @@ -90,6 +90,7 @@ class APPSEC(metaclass=Constant_Class): AUTO_LOGIN_EVENTS_FAILURE_MODE: Literal[ "_dd.appsec.events.users.login.failure.auto.mode" ] = "_dd.appsec.events.users.login.failure.auto.mode" + AUTO_LOGIN_EVENTS_COLLECTION_MODE: Literal["_dd.appsec.user.collection_mode"] = "_dd.appsec.user.collection_mode" BLOCKED: Literal["appsec.blocked"] = "appsec.blocked" EVENT: Literal["appsec.event"] = "appsec.event" AUTO_USER_INSTRUMENTATION_MODE: Literal[ diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index f1b8e838535..f9a6919725d 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -133,7 +133,7 @@ def track_user_login_success_event( return if real_mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): user_id = _hash_user_id(user_id) - + span.set_tag_str(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE, real_mode) if login_events_mode != LOGIN_EVENTS_MODE.SDK: span.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) set_user(tracer, user_id, name, email, scope, role, session_id, propagate, span, may_block=False) @@ -185,6 +185,7 @@ def track_user_login_failure_event( if login_events_mode != LOGIN_EVENTS_MODE.SDK: span.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) span.set_tag_str("%s.failure.%s" % (APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC, user.ID), str(user_id)) + span.set_tag_str(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE, real_mode) # if called from the SDK, set the login, email and name if login_events_mode in (LOGIN_EVENTS_MODE.SDK, LOGIN_EVENTS_MODE.AUTO): if login: @@ -408,6 +409,7 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co name=user_extra.get("name"), may_block=False, ) + pin.tracer.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) if in_asm_context(): real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { From b47ea609db2d2ab3aada1cb28a44174ffb35e309 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 30 Jan 2025 14:47:27 +0100 Subject: [PATCH 30/31] fix span usage for _on_django_process --- ddtrace/appsec/_trace_utils.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index f9a6919725d..a52bc082297 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -378,7 +378,7 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever, django_confi def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_config): - if not asm_config._asm_enabled: + if (not asm_config._asm_enabled) or mode == LOGIN_EVENTS_MODE.DISABLED: return userid_list = info_retriever.possible_user_id_fields + info_retriever.possible_login_fields @@ -398,9 +398,13 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co user_extra["login"] = user_id user_id = user_id_found or user_id if result_user and result_user.is_authenticated: + span = pin.tracer.current_root_span() if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): - set_user(pin.tracer, _hash_user_id(str(user_id)), propagate=True, may_block=False) + hash_id = _hash_user_id(user_id) + span.set_tag_str(APPSEC.USER_LOGIN_USERID, hash_id) + set_user(pin.tracer, hash_id, propagate=True, may_block=False, span=span) elif mode == LOGIN_EVENTS_MODE.IDENT: + span.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) set_user( pin.tracer, str(user_id), @@ -408,8 +412,8 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co email=user_extra.get("email"), name=user_extra.get("name"), may_block=False, + span=span, ) - pin.tracer.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) if in_asm_context(): real_mode = mode if mode != LOGIN_EVENTS_MODE.AUTO else asm_config._user_event_mode custom_data = { From a14acdb4ec87cb4b9494bc603b2a3d2c3404aeb1 Mon Sep 17 00:00:00 2001 From: Christophe Papazian Date: Thu, 30 Jan 2025 14:53:24 +0100 Subject: [PATCH 31/31] add collection mode for on_django_process --- ddtrace/appsec/_trace_utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index a52bc082297..53cc8d9b9b2 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -402,9 +402,11 @@ def _on_django_process(result_user, mode, kwargs, pin, info_retriever, django_co if mode == LOGIN_EVENTS_MODE.ANON and isinstance(user_id, str): hash_id = _hash_user_id(user_id) span.set_tag_str(APPSEC.USER_LOGIN_USERID, hash_id) + span.set_tag_str(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE, mode) set_user(pin.tracer, hash_id, propagate=True, may_block=False, span=span) elif mode == LOGIN_EVENTS_MODE.IDENT: span.set_tag_str(APPSEC.USER_LOGIN_USERID, str(user_id)) + span.set_tag_str(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE, mode) set_user( pin.tracer, str(user_id),