Add DDTrace\try_drop_span() API by bwoebi · Pull Request #3136 · DataDog/dd-trace-php

bwoebi · 2025-03-20T11:46:29Z

Allow dropping top-of-stack (i.e. active) spans in a safe manner.

codecov-commenter · 2025-03-20T11:53:05Z

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.94%. Comparing base (dfe83ec) to head (0d86cb3).
⚠️ Report is 750 commits behind head on master.

Files with missing lines	Patch %	Lines
...DTrace/Integrations/Symfony/SymfonyIntegration.php	0.00%	11 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3136      +/-   ##
============================================
- Coverage     56.98%   56.94%   -0.04%     
- Complexity     2883     2885       +2     
============================================
  Files           141      141              
  Lines         15844    15855      +11     
  Branches       1094     1094              
============================================
  Hits           9028     9028              
- Misses         6243     6254      +11     
  Partials        573      573

Flag	Coverage Δ
appsec-extension	`69.22% <ø> (ø)`
tracer-php	`52.21% <0.00%> (-0.06%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...DTrace/Integrations/Symfony/SymfonyIntegration.php	`0.00% <0.00%> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfe83ec...0d86cb3. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pr-commenter · 2025-03-20T12:15:54Z

Benchmarks [ tracer ]

Benchmark execution time: 2025-03-31 09:38:44

Comparing candidate commit 0d86cb3 in PR branch bob/drop-span with baseline commit dfe83ec in branch master.

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

scenario:EmptyFileBench/benchEmptyFileBaseline

🟥 execution_time [+70.219µs; +246.461µs] or [+2.364%; +8.298%]

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

🟥 execution_time [+94.866µs; +231.634µs] or [+3.095%; +7.558%]

scenario:EmptyFileBench/benchEmptyFileOverhead

🟥 execution_time [+158.086µs; +305.194µs] or [+4.768%; +9.205%]

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

🟥 execution_time [+132.079µs; +278.181µs] or [+3.837%; +8.081%]

scenario:PDOBench/benchPDOBaseline-opcache

🟩 execution_time [-16.395µs; -10.606µs] or [-8.067%; -5.219%]

PROFeNoM

The extension code looks good to me 👍 I left a small, yet important comment for the Integration part.

Additionally, on the bob/drop-span-tests branch, I've added a commit adding Symfony CLI Tests. Feel free to cherry pick it 😃 (I haven't checked if the CI passes as I'm writing these words)

PROFeNoM · 2025-03-21T15:05:19Z

+         */
+        foreach (['Symfony\Component\Console\Terminal::hasSttyAvailable', 'Symfony\Component\Console\Helper\QuestionHelper::isInteractiveInput'] as $method) {
+            \DDTrace\install_hook($method, function (HookData $hook) {
+                \DDTrace\active_stack()->spanCreationObservers[] = function (SpanData $span) use ($hook) {


Suggested change

\DDTrace\active_stack()->spanCreationObservers[] = function (SpanData $span) use ($hook) {

$hook->data = false;

\DDTrace\active_stack()->spanCreationObservers[] = function (SpanData $span) use ($hook) {

Considering that when the pre-hook first runs, $hook->data is not initialized, I believe that we would otherwise try to access the value in the span creation closure before it's initialized.

That's absolutely right, and thanks for adding tests, hadn't gotten round to that yet!

Signed-off-by: Bob Weinand <[email protected]>

Gaitholabi · 2025-04-02T15:23:58Z

@bwoebi hey, I was just staging our integration after the update to 1.8.0 but this seems to be causing the following error:

[ddtrace] [error] There is no user-span on the top of the stack. Cannot close.

This is reproducible for me on any CLI command on a skeleton symfony 7.2 app.

composer create-project symfony/skeleton:"7.2.x" skeleton --quiet && (bin/console | grep ddtrace)

on the following env:

Zend Engine v4.4.5, Copyright (c) Zend Technologies
    with Zend OPcache v8.4.5, Copyright (c), by Zend Technologies
    with Xdebug v3.4.1, Copyright (c) 2002-2025, by Derick Rethans
    with ddtrace v1.8.0, Copyright Datadog, by Datadog
    with datadog-profiling v1.8.0, Copyright Datadog, by Datadog
    with ddappsec v1.8.0, Copyright Datadog, by Datadog

bwoebi · 2025-04-02T15:34:22Z

@Gaitholabi You are right and we messed up :-/
Thank you for notifying us!

We do have detection for [error] messages in some integration tests, but apparently not for framework integration tests. I missed that.
We'll hotfix this soon.

Gaitholabi · 2025-04-02T16:03:52Z

@bwoebi Thank you! appreciated.

bwoebi · 2025-04-03T09:08:55Z

@Gaitholabi Hotfix released as 1.8.1; thanks again!

bwoebi requested a review from a team as a code owner March 20, 2025 11:46

bwoebi force-pushed the bob/drop-span branch from 28f0abb to da7d5e8 Compare March 20, 2025 16:11

bwoebi requested a review from a team as a code owner March 20, 2025 16:11

PROFeNoM reviewed Mar 21, 2025

View reviewed changes

bwoebi force-pushed the bob/drop-span branch from da7d5e8 to 38d2349 Compare March 28, 2025 14:52

bwoebi and others added 2 commits March 31, 2025 11:10

Add DDTrace\try_drop_span() API

be86236

Signed-off-by: Bob Weinand <[email protected]>

tests: Add testSilencedSpansAreDropped for Symfony CLI Tests

0d86cb3

bwoebi force-pushed the bob/drop-span branch from 38d2349 to 0d86cb3 Compare March 31, 2025 09:10

bwoebi enabled auto-merge (squash) March 31, 2025 14:38

bwoebi disabled auto-merge March 31, 2025 14:39

PROFeNoM approved these changes Mar 31, 2025

View reviewed changes

bwoebi merged commit cff2034 into master Mar 31, 2025
742 of 766 checks passed

bwoebi deleted the bob/drop-span branch March 31, 2025 18:21

github-actions Bot added this to the 1.8.0 milestone Mar 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DDTrace\try_drop_span() API#3136

Add DDTrace\try_drop_span() API#3136
bwoebi merged 2 commits intomasterfrom
bob/drop-span

bwoebi commented Mar 20, 2025

Uh oh!

codecov-commenter commented Mar 20, 2025 •

edited

Loading

Uh oh!

pr-commenter Bot commented Mar 20, 2025 •

edited

Loading

Uh oh!

PROFeNoM left a comment

Uh oh!

PROFeNoM Mar 21, 2025

Uh oh!

bwoebi Mar 22, 2025

Uh oh!

Uh oh!

Gaitholabi commented Apr 2, 2025

Uh oh!

bwoebi commented Apr 2, 2025 •

edited

Loading

Uh oh!

Gaitholabi commented Apr 2, 2025

Uh oh!

bwoebi commented Apr 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	\DDTrace\active_stack()->spanCreationObservers[] = function (SpanData $span) use ($hook) {
	$hook->data = false;
	\DDTrace\active_stack()->spanCreationObservers[] = function (SpanData $span) use ($hook) {

Conversation

bwoebi commented Mar 20, 2025

Uh oh!

codecov-commenter commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter Bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ tracer ]

scenario:EmptyFileBench/benchEmptyFileBaseline

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

scenario:EmptyFileBench/benchEmptyFileOverhead

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

scenario:PDOBench/benchPDOBaseline-opcache

Uh oh!

PROFeNoM left a comment

Choose a reason for hiding this comment

Uh oh!

PROFeNoM Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

bwoebi Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gaitholabi commented Apr 2, 2025

Uh oh!

bwoebi commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gaitholabi commented Apr 2, 2025

Uh oh!

bwoebi commented Apr 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Mar 20, 2025 •

edited

Loading

pr-commenter Bot commented Mar 20, 2025 •

edited

Loading

bwoebi commented Apr 2, 2025 •

edited

Loading