feat(appsec/proxy): enable body processing by default (except GCP SE) #4069

e-n-0 · 2025-10-23T15:56:48Z

What does this PR do?

This PR enables body processing by default for all proxies integrations except for GCP Service Extensions.

The challenge of this PR was to enable body processing by default for Envoy based proxies but excluding GCP Service Extension from this. As the final integration type is detected during runtime (based on metadata set inside the gRPC stream of requests), the message processor configuration needed to be updated after its initialization.
The integration type is chose either at startup via specification in the code (if integration is different than the default GCP Service Extension) or when the first request is received.

This PR changes the public api for ProcessorConfig of the proxy package, but as this is only used internally by contribs and we have no current usage of this module outside, we accept this breaking change.

The default value set for body processing is 10MB.

Note:

The default body processing value for gateway api have been changed from 5MB to 10MB.

Why excluding GCP Service Extension

We currently have an ongoing support case where some request could provoke some backend timeout when body processing is enabled using GCP Service Extension. This integration will also be updated later when this case will be closed.

Motivation

Body processing needs to be enabled for the GA of the AAP Envoy integration.

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
There is a benchmark for any new code, or changes to existing code.
If this interacts with the agent in a new way, a system test has been added.
New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
Add an appropriate team label so this PR gets put in the right place for the release notes.
Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

pr-commenter · 2025-10-23T16:05:34Z

Benchmarks

Benchmark execution time: 2025-10-24 16:30:09

Comparing candidate commit 48767a0 in PR branch flavien/proxies/default-body-processing with baseline commit 3d45ade in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics.

datadog-datadog-prod-us1 · 2025-10-23T16:27:01Z

⚠️ Tests

⚠️ Warnings

❄️ 1 New flaky test detected

TestHealthMetricsRaceCondition from github.com/DataDog/dd-trace-go/v2/ddtrace/tracer (Datadog)

Failed

=== RUN   TestHealthMetricsRaceCondition
    metrics_test.go:272: 
        	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/ddtrace/tracer/metrics_test.go:272
        	Error:      	Condition never satisfied
        	Test:       	TestHealthMetricsRaceCondition
    metrics_test.go:274: 
        	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/ddtrace/tracer/metrics_test.go:274
        	Error:      	Should be true
...

ℹ️ Info

🧪 All tests passed

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 48767a0 | Docs | Was this helpful? Give us feedback!}

e-n-0 · 2025-10-24T14:46:45Z

contrib/envoyproxy/go-control-plane/envoy_test.go


 	setup := func() (envoyextproc.ExternalProcessorClient, mocktracer.Tracer, func()) {
-		rig, err := newEnvoyAppsecRig(t, EnvoyIntegration, false, 0)
+		rig, err := newEnvoyAppsecRig(t, GCPServiceExtensionIntegration, false, nil)


The integration was changed only to have a body parsing disabled for the tests

…or GCP Service Extension

eliottness · 2025-10-24T15:02:53Z

contrib/envoyproxy/go-control-plane/envoy.go

+		// Set default body parsing size limit if not specified for non-default integrations
+		if config.BodyParsingSizeLimit == nil {
+			defaultBody := proxy.DefaultBodyParsingSizeLimit
+			config.BodyParsingSizeLimit = &defaultBody
+		}


I think at this point the integration is not set properly in most cases and since GCP is the default value...

In the end yes it's not that useful. Removed in 48767a0

eliottness · 2025-10-24T15:04:11Z

contrib/envoyproxy/go-control-plane/envoy.go

+			defaultBody := proxy.DefaultBodyParsingSizeLimit
+			config.BodyParsingSizeLimit = &defaultBody


Suggested change

defaultBody := proxy.DefaultBodyParsingSizeLimit

config.BodyParsingSizeLimit = &defaultBody

config.BodyParsingSizeLimit = &

proxy.DefaultBodyParsingSizeLimit

proxy.DefaultBodyParsingSizeLimit is a const, I can't directly take its address

eliottness · 2025-10-24T15:06:23Z

instrumentation/appsec/proxy/config.go


+const (
+	// DefaultBodyParsingSizeLimit is the default number of bytes parsed for body analysis.
+	DefaultBodyParsingSizeLimit = 10_000_000 // 10MB


do you mind doing it in power of two ?

Applied in f61a2f2

eliottness · 2025-10-24T15:08:06Z

instrumentation/appsec/proxy/message_processor.go


+	mp.firstRequest.Do(func() {
+		if mp.BodyParsingSizeLimit == nil {
+			mp.computedBodyParsingSizeLimit = req.BodyParsingSizeLimit(ctx)


There is a data race here

Are you sure there is a data race here? The value of computedBodyParsingSizeLimit is only set once with the firstRequest.Do.

Changed using atomic value in fb33a1e

github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Oct 23, 2025

e-n-0 force-pushed the flavien/proxies/default-body-processing branch from 6a965ca to 5d0feb8 Compare October 23, 2025 16:02

e-n-0 added appsec and removed apm:ecosystem contrib/* related feature requests or bugs labels Oct 23, 2025

e-n-0 force-pushed the flavien/proxies/default-body-processing branch from b363d29 to d31115a Compare October 24, 2025 13:02

e-n-0 marked this pull request as ready for review October 24, 2025 14:38

e-n-0 requested a review from a team as a code owner October 24, 2025 14:38

e-n-0 commented Oct 24, 2025

View reviewed changes

e-n-0 added 3 commits October 24, 2025 16:50

enable body processing by default on all proxies integration except f…

ddaf5a2

…or GCP Service Extension

use the computed value instead of doing nil check everywhere

90cada9

update readmes

e873ce2

e-n-0 force-pushed the flavien/proxies/default-body-processing branch from d31115a to e873ce2 Compare October 24, 2025 14:51

eliottness requested changes Oct 24, 2025

View reviewed changes

e-n-0 added 3 commits October 24, 2025 17:54

change computedBodyParsingSizeLimit to atomic value

fb33a1e

using power of 2

f61a2f2

remove default integration body size limit check

48767a0

e-n-0 requested a review from eliottness October 24, 2025 16:15

		defaultBody := proxy.DefaultBodyParsingSizeLimit
		config.BodyParsingSizeLimit = &defaultBody

feat(appsec/proxy): enable body processing by default (except GCP SE) #4069

Are you sure you want to change the base?

feat(appsec/proxy): enable body processing by default (except GCP SE) #4069

Uh oh!

Conversation

e-n-0 commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why excluding GCP Service Extension

Motivation

Reviewer's Checklist

Uh oh!

pr-commenter bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

datadog-datadog-prod-us1 bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

ℹ️ Info

Uh oh!

e-n-0 Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e-n-0 Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

e-n-0 commented Oct 23, 2025 •

edited

Loading

pr-commenter bot commented Oct 23, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Oct 23, 2025 •

edited

Loading

e-n-0 Oct 24, 2025 •

edited

Loading

e-n-0 Oct 24, 2025 •

edited

Loading