Skip to content
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

Add new ini config datadog.crashtracking_enabled #2850

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

iamluc
Copy link
Contributor

@iamluc iamluc commented Sep 13, 2024

Description

Corresponding env var DD_CRASHTRACKING_ENABLED is handled at least by dd-trace-dotnet and dd-trace-py, and is required by a system-test.

Additionally, I reduced the wait time in the request replayer.
Previously, the default was 5 seconds per iteration with a maximum of 100 iterations.
Now, it’s 0.5 seconds per iteration, with a maximum of 200 iterations (500 on Windows).

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@codecov-commenter
Copy link

codecov-commenter commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.38%. Comparing base (4ea2324) to head (810ddce).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2850      +/-   ##
============================================
- Coverage     78.39%   78.38%   -0.02%     
  Complexity     2526     2526              
============================================
  Files           173      173              
  Lines         18710    18710              
  Branches        976      976              
============================================
- Hits          14667    14665       -2     
- Misses         3506     3508       +2     
  Partials        537      537              
Flag Coverage Δ
appsec-extension 69.08% <ø> (ø)
tracer-extension 78.18% <ø> (ø)
tracer-php 82.08% <ø> (-0.02%) ⬇️

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

see 1 file with indirect coverage changes


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 4ea2324...810ddce. Read the comment docs.

@iamluc iamluc force-pushed the luc/add_ini_dd_crashtracking_enabled branch from 847d7dd to 8255aae Compare September 13, 2024 10:08
@pr-commenter
Copy link

pr-commenter bot commented Sep 13, 2024

Benchmarks

Benchmark execution time: 2024-09-13 16:21:27

Comparing candidate commit 0d088b1 in PR branch luc/add_ini_dd_crashtracking_enabled with baseline commit 1a7182c in branch master.

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

scenario:BM_TeaSapiSpindown

  • 🟥 execution_time [+11.724µs; +32.438µs] or [+2.208%; +6.111%]

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

  • 🟩 execution_time [-223.361µs; -98.819µs] or [-7.383%; -3.266%]

scenario:EmptyFileBench/benchEmptyFileOverhead

  • 🟩 execution_time [-203.812µs; -63.028µs] or [-6.570%; -2.032%]

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

  • 🟩 execution_time [-190.097µs; -68.543µs] or [-5.861%; -2.113%]

scenario:LaravelBench/benchLaravelBaseline

  • 🟩 execution_time [-225.335µs; -111.285µs] or [-7.347%; -3.628%]

scenario:PDOBench/benchPDOBaseline-opcache

  • 🟩 execution_time [-14.012µs; -10.773µs] or [-7.319%; -5.627%]

scenario:PDOBench/benchPDOOverhead

  • 🟩 execution_time [-13.432µs; -11.674µs] or [-4.713%; -4.096%]

scenario:PDOBench/benchPDOOverhead-opcache

  • 🟩 execution_time [-12.141µs; -7.044µs] or [-4.055%; -2.353%]

scenario:PDOBench/benchPDOOverheadWithDBM

  • 🟩 execution_time [-16.322µs; -13.150µs] or [-5.270%; -4.246%]

scenario:PDOBench/benchPDOOverheadWithDBM-opcache

  • 🟩 execution_time [-18.424µs; -6.888µs] or [-5.438%; -2.033%]

scenario:SamplingRuleMatchingBench/benchRegexMatching1

  • 🟥 execution_time [+166.599ns; +511.001ns] or [+2.379%; +7.298%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4-opcache

  • 🟥 execution_time [+231.434ns; +646.166ns] or [+3.397%; +9.483%]

scenario:WordPressBench/benchWordPressOverhead

  • 🟩 execution_time [-21.832ms; -21.620ms] or [-82.886%; -82.080%]

@iamluc iamluc marked this pull request as ready for review September 13, 2024 12:03
@iamluc iamluc requested a review from a team as a code owner September 13, 2024 12:03
@iamluc iamluc force-pushed the luc/add_ini_dd_crashtracking_enabled branch from 0d088b1 to 6242e01 Compare September 18, 2024 14:59
@pr-commenter
Copy link

pr-commenter bot commented Sep 18, 2024

Benchmarks [ tracer ]

Benchmark execution time: 2024-09-18 16:23:05

Comparing candidate commit 810ddce in PR branch luc/add_ini_dd_crashtracking_enabled with baseline commit 4ea2324 in branch master.

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

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-6.258µs; -4.202µs] or [-4.225%; -2.837%]

scenario:PDOBench/benchPDOBaseline-opcache

  • 🟥 execution_time [+4.904µs; +12.740µs] or [+2.712%; +7.046%]

scenario:WordPressBench/benchWordPressOverhead

  • 🟩 execution_time [-21.803ms; -21.588ms] or [-82.866%; -82.049%]

@iamluc iamluc force-pushed the luc/add_ini_dd_crashtracking_enabled branch from 6242e01 to 810ddce Compare September 18, 2024 16:00
Copy link
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@iamluc iamluc merged commit 8b058f8 into master Sep 19, 2024
695 of 696 checks passed
@iamluc iamluc deleted the luc/add_ini_dd_crashtracking_enabled branch September 19, 2024 14:06
@github-actions github-actions bot added this to the 1.4.0 milestone Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants