Conversation
Codeowners resolved as |
🎉 All green!❄️ No new flaky tests detected 🔗 Commit SHA: b05ce20 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-05-08 20:21:54 Comparing candidate commit b05ce20 in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 591 metrics, 4 unstable metrics. scenario:iastaspects-index_aspect
scenario:iastaspects-ljust_noaspect
scenario:iastaspects-stringio_noaspect
scenario:iastaspectsospath-ospathbasename_aspect
scenario:span-start
scenario:telemetryaddmetric-1-count-metric-1-times
|
df478a5 to
a96248a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53f175fd3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Replace ``core.raising_dispatch`` with a keyword-only ``allow_raise=True`` flag on ``core.dispatch`` (and ``dispatch_event``). Migrate all in-tree call sites in the langchain and openai contribs. Introduce ``DDBlockException(BaseException)`` as the umbrella class for in-tree block decisions. ``BlockingException`` (ASM) and ``AIGuardAbortError`` now derive from it. ``AIGuardAbortError``'s base moves from ``Exception`` to ``DDBlockException`` so a generic ``except Exception:`` no longer silently swallows a block — release note spells out the upgrade impact for users. Tag the LLM span on AI Guard block in the four LangChain non-stream wrappers: an explicit ``except DDBlockException:`` arm calls ``span.set_exc_info`` before re-raising, since the existing ``except Exception:`` does not catch ``BaseException``-derived aborts. Mirrors the contract from #17913 (openai ``_patched_endpoint``). Document the catchability asymmetry on ``OpenAIAIGuardAbortError``: it inherits from ``openai.UnprocessableEntityError`` (Exception-derived) *and* ``AIGuardAbortError`` (BaseException-derived), so that subclass remains catchable by ``except Exception:`` for OpenAI SDK compatibility, while plain ``AIGuardAbortError`` is not. Tests: - ``tests/internal/test_context_events_api.py`` — coverage for ``dispatch(..., allow_raise=True)`` semantics, ``BaseException`` propagation, and the new exception-class hierarchy. - ``tests/appsec/ai_guard/langchain/test_langchain.py`` — pin the LLM-span ``set_exc_info`` contract on AI Guard block (sync + async). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1cc763c to
8a5cb85
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9eb78aa6a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| completions = None | ||
| try: | ||
| core.raising_dispatch("langchain.llm.agenerate.before", (prompts,)) | ||
| core.dispatch("langchain.llm.agenerate.before", (prompts,), allow_raise=True) |
There was a problem hiding this comment.
So just to clarify we're just replacing raising_dispatch with dispatch and adding allow_raise=True to do the exact same thing (raise/propagate) as before?
| completions = await func(*args, **kwargs) | ||
| core.dispatch("langchain.llm.agenerate.after", (prompts, completions)) | ||
| except Exception: | ||
| except (DDBlockException, Exception): |
There was a problem hiding this comment.
Are DDBlockExceptions new or covering existing functionality?
There was a problem hiding this comment.
Ahh I saw the rest of the PR, AIGuard errors now derive from DDBlockException. Makes sense to me!
Description
Deprecates and removes
core.raising_dispatchin favor of an opt-inallow_raise=Trueflag on the existingcore.dispatchAPI. Tightens the in-tree blocking-exception hierarchy and aligns LangChain non-stream wrappers to tag the LLM span on AI Guard block.What changed
1.
core.dispatchgainsallow_raise=Trueddtrace/internal/core/event_hub.py—dispatch()anddispatch_event()accept a keyword-onlyallow_raise: bool = False. When set, listenerExceptions propagate to the caller (first raiser wins; subsequent listeners are skipped).BaseException-derived exceptions (includingDDBlockException) always propagate regardless of the flag — matching the existing default-deny semantics for blocks.This collapses two near-duplicate dispatch entry points into one and lets call sites be explicit about whether listener exceptions should surface.
2.
core.raising_dispatchremovedThe symbol is removed from
ddtrace.internal.coreandevent_hub.py. All in-tree call sites are migrated:ddtrace/contrib/internal/langchain/patch.py— sixbeforeevents (chat/llm × generate/agenerate/stream).ddtrace/contrib/internal/openai/patch.py— four sites (syncbefore/after, asyncbefore/after).The
.sgrule and snapshot tests forcore-raising-dispatchare removed since the symbol no longer exists.3. New
DDBlockExceptionbase classddtrace/internal/_exceptions.pyintroducesDDBlockException(BaseException)as the umbrella for any in-tree decision to abort the current operation (web-request blocking, AI Guard policy abort, future product blocks).BlockingException(ASM) andAIGuardAbortError(AI Guard) now derive from it.This gives integrations a single, semantically-correct catch target for "the platform decided to block this call" — distinct from arbitrary user/library
Exceptions. It still inherits fromBaseExceptionso a genericexcept Exception:cannot silently swallow a block decision.4. LangChain non-stream wrappers: tag the LLM span on block
The four non-stream wrappers (
traced_llm_generate,traced_llm_agenerate,traced_chat_model_generate,traced_chat_model_agenerate) now have an explicitexcept DDBlockException:arm that callsspan.set_exc_info(*sys.exc_info())before re-raising, in addition to the existingexcept Exception:arm.Required because
AIGuardAbortErrorderives fromBaseException(viaDDBlockException) and is therefore not caught byexcept Exception:. Without the explicit arm, blocked LangChain calls would emit an LLMObs span with no error info — leaving a hole between the AI Guard span (block decision) and the LLM span (no link back to the abort). Aligns LangChain's non-stream behavior with the contract from #17913 for openai (_patched_endpointusesexcept BaseException as e: err = e; raise, then_traced_endpoint'sfinallycallsset_exc_info).5.
OpenAIAIGuardAbortError: documented catchability asymmetryThe compound class inherits from both
openai.UnprocessableEntityError(Exception-derived) andAIGuardAbortError(nowBaseException-derived). Via MRO it remainsException-derived — soexcept openai.APIError:blocks still work for users migrating from non-AI-Guard error handling. The asymmetry vs plainAIGuardAbortErroris now spelled out in an AIDEV-NOTE on the class. Code that wants uniform block detection across providers should branch onisinstance(e, AIGuardAbortError).Testing
tests/internal/test_context_events_api.py— new coverage fordispatch(..., allow_raise=True)semantics:Exceptions propagate when set, are swallowed when not, listeners short-circuit on first raise,BaseException-derived listener exceptions (DDBlockExceptionsubclasses) always propagate regardless of the flag, thedispatch_eventvariant behaves identically, and the new exception inheritance is asserted.tests/appsec/ai_guard/langchain/test_langchain.py— two new tests pin theset_exc_info-on-block contract on the LLM span (syncchat.invoke+ asyncchat.ainvoke). A regression that swaps the explicitexcept DDBlockExceptionarm back toexcept Exceptionwould surface immediately.appsec::ai_guard_langchainvenv5484ca0(Python 3.13) — passes.appsec::ai_guard_openaivenv1224d93(Python 3.12) — passes (this was the suite that initially exposed the missing openai-contrib migration).hatch run lint:fmtclean on all modified files.Risks
AIGuardAbortErrorinheritance change is user-visible. It now derives fromDDBlockException(BaseException)instead ofException. User code today doingtry: model.invoke(...) except Exception: ...to handle aborts will silently stop catching them. This is intentional (theBaseExceptionderivation is what prevents accidental swallowing of blocks) but it does break code that relied on the prior behavior. Spelled out in the release note'supgradesection. The OpenAI-compatible variant (OpenAIAIGuardAbortError) remains catchable byexcept Exception:via MRO for SDK compatibility.core.raising_dispatchremoval. Internal helper underddtrace.internal.core— not part of the public API. All in-tree call sites are migrated in this PR. The release note'sdeprecationssection documents the migration path for any out-of-tree consumer that imported it.error == 1anderror.typecontainingAIGuardAbortError. Downstream consumers that filter LLM spans byerror == 0will see blocked LangChain calls as errored — desired observability change per feat(ai_guard): add OpenAI SDK integration (streaming) #17913 but worth flagging.core.dispatch.allow_raiseis keyword-only, defaults toFalse, preserves existing non-raising behavior for every existing caller.Additional Notes
Release note:
releasenotes/notes/aiguard-abort-baseexception-deprecate-raising-dispatch-3a5c7f61ee4d2e1b.yamlcovers theAIGuardAbortErrorinheritance change (upgradesection) and theraising_dispatchremoval (deprecationssection).