-
Notifications
You must be signed in to change notification settings - Fork 440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ddtrace/tracer: Tracing as transport-only mode (APPSEC_STANDALONE) #3033
Conversation
dbd4501
to
8abea1d
Compare
BenchmarksBenchmark execution time: 2024-12-19 11:16:26 Comparing candidate commit 1a458ca in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 0 unstable metrics. scenario:BenchmarkSetTagMetric-24
|
8abea1d
to
666c79a
Compare
Datadog ReportBranch report: ✅ 0 Failed, 2981 Passed, 24 Skipped, 2m 27.99s Total Time |
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
ffc34c3
to
16c4012
Compare
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -53,7 +53,7 @@ func (op *ContextOperation) Run(eventReceiver dyngo.Operation, addrs waf.RunAddr | |||
actions.SendActionEvents(eventReceiver, result.Actions) | |||
|
|||
if result.HasEvents() { | |||
log.Debug("appsec: WAF detected a suspicious event") | |||
dyngo.EmitData(op, &SecurityEvent{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so meta indeed
@@ -59,6 +59,7 @@ type startupInfo struct { | |||
FeatureFlags []string `json:"feature_flags"` | |||
PropagationStyleInject string `json:"propagation_style_inject"` // Propagation style for inject | |||
PropagationStyleExtract string `json:"propagation_style_extract"` // Propagation style for extract | |||
TracingAsTransport bool `json:"tracing_as_transport"` // Whether the tracer is disabled and other products are using it as a transport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice one
@@ -423,7 +426,9 @@ func (t *tracer) worker(tick <-chan time.Time) { | |||
t.statsd.Incr("datadog.tracer.flush_triggered", []string{"reason:invoked"}, 1) | |||
t.traceWriter.flush() | |||
t.statsd.Flush() | |||
t.stats.flushAndSend(time.Now(), withCurrentBucket) | |||
if !t.config.tracingAsTransport { | |||
t.stats.flushAndSend(time.Now(), withCurrentBucket) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens with the data in withCurrentBucket
?
should we mute the emitter side of things too (this place being the receiver)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the emitter side of things is muted via the change done to canComputeStats()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit for the next PR to come but I suggest then to update t.stats
methods to noop them when this is disabled
@@ -53,7 +53,7 @@ func (op *ContextOperation) Run(eventReceiver dyngo.Operation, addrs waf.RunAddr | |||
actions.SendActionEvents(eventReceiver, result.Actions) | |||
|
|||
if result.HasEvents() { | |||
log.Debug("appsec: WAF detected a suspicious event") | |||
dyngo.EmitData(op, &SecurityEvent{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so meta indeed
Signed-off-by: Eliott Bouhana <[email protected]>
@@ -14,7 +14,6 @@ import ( | |||
"strings" | |||
|
|||
"gopkg.in/DataDog/dd-trace-go.v1/appsec/events" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness Could you backport this PR into the v2 branch? Thanks! 🙏🏼 |
…3033) Signed-off-by: Eliott Bouhana <[email protected]>
Applied comments appsec: stop storing span tags, directly call span.SetTag (#3044) Signed-off-by: Eliott Bouhana <[email protected]> ddtrace/tracer: Tracing as transport-only mode (APPSEC_STANDALONE) (#3033) Signed-off-by: Eliott Bouhana <[email protected]> fix: improving test logic for TestStreamSendsErrorCode to avoid flakiness (#3049) vuln: upgrade golang.org/x/{crypto,net} to non-vulnerable versions (#3050) contrib/miekg/dns: resolve flaky test in TestExchange* (#3045) ddtrace/tracer: report datadog.tracer.api.errors health metric (#3024) build(deps): bump google.golang.org/grpc from 1.64.0 to 1.64.1 (#3001) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Argüello <[email protected]> ddtrace/tracer: Report datadog.tracer.queue.enqueued.traces as health metric (#3019) ddtrace/tracer: Tracing as transport-only mode (APPSEC_STANDALONE) (#3033) Signed-off-by: Eliott Bouhana <[email protected]> fix: improving test logic for TestStreamSendsErrorCode to avoid flakiness (#3049) vuln: upgrade golang.org/x/{crypto,net} to non-vulnerable versions (#3050) contrib/miekg/dns: resolve flaky test in TestExchange* (#3045) ddtrace/tracer: report datadog.tracer.api.errors health metric (#3024) build(deps): bump google.golang.org/grpc from 1.64.0 to 1.64.1 (#3001) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Argüello <[email protected]> ddtrace/tracer: Report datadog.tracer.queue.enqueued.traces as health metric (#3019)
Applied comments appsec: stop storing span tags, directly call span.SetTag (#3044) Signed-off-by: Eliott Bouhana <[email protected]> ddtrace/tracer: Tracing as transport-only mode (APPSEC_STANDALONE) (#3033) Signed-off-by: Eliott Bouhana <[email protected]> fix: improving test logic for TestStreamSendsErrorCode to avoid flakiness (#3049) vuln: upgrade golang.org/x/{crypto,net} to non-vulnerable versions (#3050) contrib/miekg/dns: resolve flaky test in TestExchange* (#3045) ddtrace/tracer: report datadog.tracer.api.errors health metric (#3024) build(deps): bump google.golang.org/grpc from 1.64.0 to 1.64.1 (#3001) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Argüello <[email protected]> ddtrace/tracer: Report datadog.tracer.queue.enqueued.traces as health metric (#3019) ddtrace/tracer: Tracing as transport-only mode (APPSEC_STANDALONE) (#3033) Signed-off-by: Eliott Bouhana <[email protected]> fix: improving test logic for TestStreamSendsErrorCode to avoid flakiness (#3049) vuln: upgrade golang.org/x/{crypto,net} to non-vulnerable versions (#3050) contrib/miekg/dns: resolve flaky test in TestExchange* (#3045) ddtrace/tracer: report datadog.tracer.api.errors health metric (#3024) build(deps): bump google.golang.org/grpc from 1.64.0 to 1.64.1 (#3001) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Argüello <[email protected]> ddtrace/tracer: Report datadog.tracer.queue.enqueued.traces as health metric (#3019)
What does this PR do?
This PR adds a new
tracingAsTransport
running mode for the tracer. This mode was created to disable APM biling but still allow customers to use other products that use APM as it's transport layer.In short, what was done is:
(*config).tracingAsTransport
boolean fieldconfig
struct for ease of use_dd.p.*
)DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED
, do these:_dd.apm.enabled=0
Datadog-Client-Computed-Stats
http header_dd.p.appsec=1
is not received upstream or set locallyAll of this should make sure APM UI is disabled, but setting ManualKeep on certain traces will allow to bypass the rate limiter and will allow other products to still work when tracing-as-transport mode is enabled.
Motivation
This PR is the result of 1 approved RFC on ASM side and one draft RFC on APM side.
System-tests Run
https://github.com/DataDog/dd-trace-go/actions/runs/12397569796/job/34608280725
Reviewer's Checklist
v2-dev
branch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!