Flush LLM Obs intake writer synchronously in serverless environments - #12249
Flush LLM Obs intake writer synchronously in serverless environments#12249purple4reina wants to merge 2 commits into
Conversation
The DDAgentWriter branch of WriterFactory never wired up an LLM Obs DDIntakeWriter, so LLM Obs spans were silently dropped whenever the tracer picked DDAgentWriter (e.g. in Lambda/serverless mode with CI Visibility disabled). Broadcast to a dedicated LLM Obs DDIntakeWriter via MultiWriter, flushed synchronously in serverless environments just like the primary writer.
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4196c2186d
ℹ️ 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".
| .alwaysFlush(alwaysFlush) | ||
| .flushIntervalMilliseconds(flushIntervalMilliseconds) | ||
| .build(); | ||
| remoteWriter = new MultiWriter(new Writer[] {remoteWriter, llmObsWriter}); |
There was a problem hiding this comment.
When LLM Obs and AppSec API Security are both enabled, wrapping two independent RemoteWriters causes their TraceProcessingWorker threads to post-process the same DDSpan list concurrently. This violates SpanPostProcessor's single-thread guarantee, and AppSecSpanPostProcessor can consequently call the explicitly non-thread-safe ApiSecuritySampler.sampleRequest, extract schemas, close the WAF context, and release the same sampling permit twice. Ensure post-processing occurs once before fan-out, or otherwise prevent the LLM writer from post-processing the shared trace again.
Useful? React with 👍 / 👎.
| .alwaysFlush(alwaysFlush) | ||
| .flushIntervalMilliseconds(flushIntervalMilliseconds) | ||
| .build(); | ||
| remoteWriter = new MultiWriter(new Writer[] {remoteWriter, llmObsWriter}); |
There was a problem hiding this comment.
perf: When LLM Obs is enabled in a regular high-throughput APM service, MultiWriter.write publishes every completed trace to this second worker even when it contains no LLM span; only later does LLMObsSpanMapper.map scan the trace and allocate a filtered list before discarding it. This duplicates queue traffic and post-processing for all APM traces, and ordinary traffic can fill the LLM writer's 1024-entry queue and cause genuine LLM traces to be dropped. Filter before enqueueing into the LLM writer rather than broadcasting indiscriminately.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
| // this, they're silently dropped by the agent/extension instead of reaching LLM Obs | ||
| // intake. Flush this track synchronously too when the primary writer is (i.e. in a | ||
| // serverless environment), so spans aren't lost when the execution environment freezes. | ||
| if (config.isLlmObsEnabled()) { |
There was a problem hiding this comment.
When dd.writer.type is configured as MultiWriter:DDAgentWriter,DDIntakeWriter with LLM Obs enabled, the outer MultiWriter recursively constructs both children: this condition adds an LLM intake writer inside the DDAgentWriter child, while the explicit DDIntakeWriter child also adds its existing LLM track. Every LLM span is therefore submitted to the same intake twice, producing duplicate LLM Obs records for configurations that previously sent one copy. Deduplicate the automatically added track when an enclosing writer already supplies LLM intake.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
When LLM Observability and AppSec API Security run together outside serverless mode, both writer threads can post-process the same trace. This can close one AppSec request context twice and release its limit permit twice.
🤖 Datadog Autotest · Commit 4196c21 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| .alwaysFlush(alwaysFlush) | ||
| .flushIntervalMilliseconds(flushIntervalMilliseconds) | ||
| .build(); | ||
| remoteWriter = new MultiWriter(new Writer[] {remoteWriter, llmObsWriter}); |
There was a problem hiding this comment.
This race can damage the AppSec post-processing limit and allow more work than its set limit.
Assertion details
- Input: Enable LLM Observability and AppSec API Security with DDAgentWriter outside serverless mode. Process a request that stays open for API Security post-processing.
- Expected:
The system must post-process each trace one time before it sends the trace to both writers. - Actual:
MultiWriter sends the same mutable trace to two worker threads. Each worker calls the global span post-processor. Both workers can process and close the same AppSec request context. Both workers can also release its limit permit.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
The DDAgentWriter branch's dedicated LLM Obs DDIntakeWriter duplicated the LLM Obs track that the DD_INTAKE_WRITER_TYPE branch already builds whenever llmObsEnabled is true (Agent.java defaults to MultiWriter:DDIntakeWriter,DDAgentWriter for that case), so every span was sent twice. Verified live on Lambda: the pre-fix build sent LLMOBS/v2 payloads twice per invocation. The original bug this branch was fixing -- spans silently dropped in Lambda -- was actually caused by the DDIntakeWriter branch never setting alwaysFlush, so the periodic flush timer rarely beat the execution environment freezing after the handler returns. Verified live: without alwaysFlush, 0/8 invocations delivered a span; with it, 8/8 did, one send each.
Summary
The
DD_INTAKE_WRITER_TYPEbranch ofWriterFactory.createWriter()— which is what actually carries LLM Obs spans, sinceAgent.javadefaults the writer type toMultiWriter:DDIntakeWriter,DDAgentWriterwhenever LLM Obs is enabled — never setalwaysFlush. In Lambda, the execution environment can freeze as soon as the handler returns, before this writer's periodic flush timer next fires, so spans were silently dropped most of the time.This surfaced as
ml_obs.tracerarely/never being emitted for java Lambda functions inserverless-e2e-tests.What changed
Set
alwaysFlushon theDDIntakeWriterbranch the same way theDDAgentWriterbranch already does (config.isAgentConfiguredUsingDefault() && ServerlessInfo.get().isRunningInServerlessEnvironment()), and drop theDDAgentWriterbranch's separate LLM ObsDDIntakeWriter+MultiWriter— it duplicated the track the intake branch already builds, so every span was being sent twice (visible asml_obs.tracereporting exactly 2x the invocation count).Validation
Built and deployed custom Lambda layers to a sandbox stack to isolate each half of this fix:
alwaysFlushon the intake branch): 0/8 invocations delivered an LLM Obs span.alwaysFlushchange: 8/8 invocations delivered exactly one span each, confirmed via CloudWatch logs (Successfully sent 1 traces ... LLMOBS/v2).MultiWriterfix that it sent every span twice (two independentLLMOBS/v2payloads per invocation).