Add span field meta_struct by estringana · Pull Request #2673 · DataDog/dd-trace-php

estringana · 2024-05-24T14:03:53Z

Description

Add new field meta_struct to the span. This new field is a map where its values are encoded to msgpack and then the resulting codification to msgpack binary

APPSEC-52612

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

codecov-commenter · 2024-05-24T14:12:10Z

Codecov Report

❌ Patch coverage is 65.78947% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.94%. Comparing base (8d07c87) to head (77a1141).
⚠️ Report is 800 commits behind head on master.

Files with missing lines	Patch %	Lines
ext/serializer.c	60.60%	13 Missing ⚠️

❌ Your patch status has failed because the patch coverage (65.78%) 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    #2673      +/-   ##
============================================
- Coverage     77.96%   77.94%   -0.02%     
  Complexity     2212     2212              
============================================
  Files           227      227              
  Lines         26560    26598      +38     
  Branches        988      988              
============================================
+ Hits          20707    20732      +25     
- Misses         5327     5340      +13     
  Partials        526      526

Flag	Coverage Δ
appsec-extension	`69.13% <ø> (ø)`
tracer-extension	`78.81% <65.78%> (-0.04%)`	⬇️
tracer-php	`80.52% <ø> (ø)`

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

Files with missing lines	Coverage Δ
ext/ddtrace.c	`74.38% <100.00%> (+0.01%)`	⬆️
ext/ddtrace_arginfo.h	`100.00% <100.00%> (ø)`
ext/span.h	`100.00% <ø> (ø)`
ext/serializer.c	`81.39% <60.60%> (-0.66%)`	⬇️

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 8d07c87...77a1141. 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 · 2024-05-24T14:32:27Z

Benchmarks

Benchmark execution time: 2024-06-13 16:49:26

Comparing candidate commit 77a1141 in PR branch estringana/add-metatruct-field with baseline commit 8d07c87 in branch master.

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

scenario:ComposerTelemetryBench/benchTelemetryParsing

🟥 execution_time [+2.770µs; +4.230µs] or [+5.177%; +7.907%]

scenario:ComposerTelemetryBench/benchTelemetryParsing-opcache

🟥 execution_time [+3.262µs; +5.138µs] or [+5.899%; +9.291%]

scenario:EmptyFileBench/benchEmptyFileBaseline

🟩 execution_time [-395.491µs; -227.109µs] or [-14.294%; -8.208%]

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

🟩 execution_time [-497.856µs; -353.124µs] or [-17.283%; -12.258%]

scenario:EmptyFileBench/benchEmptyFileOverhead

🟩 execution_time [-585.115µs; -439.425µs] or [-18.312%; -13.753%]

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

🟩 execution_time [-623.554µs; -500.566µs] or [-19.247%; -15.451%]

scenario:HookBench/benchHookOverheadTraceFunction

🟥 mem_peak [+65.304KB; +65.304KB] or [+2.036%; +2.036%]

scenario:LaravelBench/benchLaravelBaseline

🟩 execution_time [-489.865µs; -296.435µs] or [-16.358%; -9.899%]

scenario:LaravelBench/benchLaravelBaseline-opcache

🟩 execution_time [-551.374µs; -385.026µs] or [-18.051%; -12.605%]

scenario:LaravelBench/benchLaravelOverhead

🟩 execution_time [-570.561µs; -400.259µs] or [-17.123%; -12.012%]

scenario:LaravelBench/benchLaravelOverhead-opcache

🟩 execution_time [-675.270µs; -526.690µs] or [-19.632%; -15.312%]

scenario:PDOBench/benchPDOBaseline-opcache

🟥 execution_time [+16.311µs; +17.075µs] or [+9.270%; +9.704%]

scenario:PDOBench/benchPDOOverhead-opcache

🟥 execution_time [+17.434µs; +18.887µs] or [+6.167%; +6.681%]

scenario:PDOBench/benchPDOOverheadWithDBM-opcache

🟥 execution_time [+7.574µs; +16.272µs] or [+2.425%; +5.209%]

scenario:SpanBench/benchDatadogAPI

🟥 mem_peak [+385.751KB; +385.758KB] or [+3.406%; +3.406%]

scenario:SpanBench/benchDatadogAPI-opcache

🟥 mem_peak [+384.612KB; +384.616KB] or [+3.954%; +3.954%]

scenario:SymfonyBench/benchSymfonyBaseline

🟩 execution_time [-336.734µs; -285.326µs] or [-5.565%; -4.715%]

scenario:SymfonyBench/benchSymfonyBaseline-opcache

🟩 execution_time [-343.363µs; -291.357µs] or [-5.581%; -4.735%]

scenario:SymfonyBench/benchSymfonyOverhead

🟩 execution_time [-773.919µs; -704.921µs] or [-10.892%; -9.921%]

scenario:SymfonyBench/benchSymfonyOverhead-opcache

🟩 execution_time [-748.531µs; -701.129µs] or [-10.400%; -9.742%]

scenario:TraceSerializationBench/benchSerializeTrace

🟥 execution_time [+11.112µs; +15.788µs] or [+6.021%; +8.555%]

scenario:TraceSerializationBench/benchSerializeTrace-opcache

🟥 execution_time [+7.514µs; +11.986µs] or [+4.568%; +7.287%]

scenario:WordPressBench/benchWordPressOverhead

🟩 execution_time [-10.950ms; -8.923ms] or [-5.053%; -4.118%]

scenario:WordPressBench/benchWordPressOverhead-opcache

🟩 execution_time [-8.442ms; -6.102ms] or [-3.900%; -2.819%]

Co-authored-by: Bob Weinand <[email protected]>

bwoebi

Looks correct to me

Anilm3

lgtm

estringana force-pushed the estringana/add-metatruct-field branch 5 times, most recently from 7c2dd1d to 0ece7d1 Compare June 3, 2024 10:03

estringana added 10 commits June 10, 2024 10:16

Add meta_struct

d640078

DELETE ME

e409dc1

WIP

8ed8d57

WIP

a2058c1

Avoid double encoding

137d2cd

wip

edec775

wip

50d7d51

Serialise metastruct

074a8c4

wip

4e1f7ae

Update arginfo

47f60ce

estringana force-pushed the estringana/add-metatruct-field branch from ddc0321 to 47f60ce Compare June 10, 2024 08:19

estringana added 3 commits June 10, 2024 14:59

Revert libdatadog changes

12533b8

Fix leak

77d9df1

Test meta_struct

1f04877

estringana force-pushed the estringana/add-metatruct-field branch from 6ef467c to 1f04877 Compare June 11, 2024 11:02

estringana added 2 commits June 11, 2024 13:04

Remove appsec

37a159e

Add description to meta struct

10f2355

estringana force-pushed the estringana/add-metatruct-field branch from 7b025a9 to 10f2355 Compare June 11, 2024 13:24

estringana marked this pull request as ready for review June 11, 2024 13:24

estringana requested a review from a team as a code owner June 11, 2024 13:24

estringana requested review from Anilm3 and cataphract June 11, 2024 13:26

bwoebi reviewed Jun 13, 2024

View reviewed changes

Comment thread ext/serializer.c Outdated

bwoebi reviewed Jun 13, 2024

View reviewed changes

Comment thread ext/serializer.c Outdated

Refactor dd_serialize_array_meta_struct_recursively

77a1141

Co-authored-by: Bob Weinand <[email protected]>

bwoebi approved these changes Jun 13, 2024

View reviewed changes

Anilm3 approved these changes Jun 14, 2024

View reviewed changes

estringana merged commit 5869a12 into master Jun 17, 2024

estringana deleted the estringana/add-metatruct-field branch June 17, 2024 09:48

github-actions Bot added this to the 1.2.0 milestone Jun 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add span field meta_struct#2673

Add span field meta_struct#2673
estringana merged 16 commits intomasterfrom
estringana/add-metatruct-field

estringana commented May 24, 2024 •

edited

Loading

Uh oh!

codecov-commenter commented May 24, 2024 •

edited

Loading

Uh oh!

pr-commenter Bot commented May 24, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

bwoebi left a comment

Uh oh!

Anilm3 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

estringana commented May 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer checklist

Uh oh!

codecov-commenter commented May 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter Bot commented May 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:ComposerTelemetryBench/benchTelemetryParsing

scenario:ComposerTelemetryBench/benchTelemetryParsing-opcache

scenario:EmptyFileBench/benchEmptyFileBaseline

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

scenario:EmptyFileBench/benchEmptyFileOverhead

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

scenario:HookBench/benchHookOverheadTraceFunction

scenario:LaravelBench/benchLaravelBaseline

scenario:LaravelBench/benchLaravelBaseline-opcache

scenario:LaravelBench/benchLaravelOverhead

scenario:LaravelBench/benchLaravelOverhead-opcache

scenario:PDOBench/benchPDOBaseline-opcache

scenario:PDOBench/benchPDOOverhead-opcache

scenario:PDOBench/benchPDOOverheadWithDBM-opcache

scenario:SpanBench/benchDatadogAPI

scenario:SpanBench/benchDatadogAPI-opcache

scenario:SymfonyBench/benchSymfonyBaseline

scenario:SymfonyBench/benchSymfonyBaseline-opcache

scenario:SymfonyBench/benchSymfonyOverhead

scenario:SymfonyBench/benchSymfonyOverhead-opcache

scenario:TraceSerializationBench/benchSerializeTrace

scenario:TraceSerializationBench/benchSerializeTrace-opcache

scenario:WordPressBench/benchWordPressOverhead

scenario:WordPressBench/benchWordPressOverhead-opcache

Uh oh!

Uh oh!

Uh oh!

bwoebi left a comment

Choose a reason for hiding this comment

Uh oh!

Anilm3 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

estringana commented May 24, 2024 •

edited

Loading

codecov-commenter commented May 24, 2024 •

edited

Loading

pr-commenter Bot commented May 24, 2024 •

edited

Loading