Exclude xml files from `dd-trace` tool and Datadog.Trace.Bundle by andrewlock · Pull Request #3363 · DataDog/dd-trace-dotnet

andrewlock · 2022-10-18T10:22:31Z

Summary of changes

Remove the XML files from the dd-trace tool and Datadog.Trace.Bundle NuGet package home folder
Add the pdb files to the dd-trace home folder (standalone tool mode)
Remove the XML files from the dd-trace bin folder
Remove the pdb files from the dd-trace bin folder

Reason for change

Xml files are only useful for dev-time experience, but the files in the home folder don't take part in that. Datadog.Trace.Bundle references the Datadog.Trace NuGet which includes them instead.

However, we do want the pdb files in the home folder. So added the ones that were missing.

The XML files and pdb files in the dd-trace bin folder aren't necessary.

Implementation details

MSBuild faffing.

Test coverage

Check the CI artifacts look correct:

xml files removed from bundle nuget
xml files removed from dd-trace nuget home
xml files removed from dd-trace standalone home
pdb files added to dd-trace nuget home
pdb files added to dd-trace standalone home
xml files removed from dd-trace nuget bin
xml files removed from dd-trace standalone bin
pdb files removed from dd-trace nuget bin
pdb files removed from dd-trace standalone bin

Other details

~~The dd-trace tool doesn't currently include the pdbs... I think it should?~~ confirmed and fixed

To ensure we strip out pdbs and xml files

This is necessary because we _want_ the pdbs etc when doing unit tests etc. Also, trying to do otherwise breaks various build stuff. It's not perfect, but it'll have to do

andrewlock · 2022-10-21T14:57:23Z

Benchmarks Report 🐌

Benchmarks for #3363 compared to master:

1 benchmarks are slower, with geometric mean 1.135
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	811μs	434ns	1.68μs	0.403	3.22 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	558μs	338ns	1.27μs	0	2.63 KB
#3363	`WriteAndFlushEnrichedTraces`	net472	799μs	435ns	1.69μs	0.398	3.22 KB
#3363	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	559μs	225ns	813ns	0	2.62 KB

Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	196ns	0.0447ns	0.161ns	0.0701	0	441 B
master	`AllCycleSimpleBody`	netcoreapp3.1	254ns	0.17ns	0.659ns	0.00601	0	440 B
master	`AllCycleMoreComplexBody`	net472	197ns	0.0723ns	0.28ns	0.0663	0	417 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	255ns	0.27ns	1.05ns	0.00555	0	416 B
master	`BodyExtractorSimpleBody`	net472	276ns	0.173ns	0.646ns	0.0574	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	258ns	0.175ns	0.679ns	0.00377	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	16.1μs	7.21ns	27ns	1.2	0.0161	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	13.3μs	5.12ns	18.5ns	0.0923	0	6.75 KB
#3363	`AllCycleSimpleBody`	net472	196ns	0.0481ns	0.167ns	0.0701	0	441 B
#3363	`AllCycleSimpleBody`	netcoreapp3.1	255ns	0.293ns	1.13ns	0.006	0	440 B
#3363	`AllCycleMoreComplexBody`	net472	194ns	0.0587ns	0.212ns	0.0663	0	417 B
#3363	`AllCycleMoreComplexBody`	netcoreapp3.1	254ns	0.13ns	0.486ns	0.00567	0	416 B
#3363	`BodyExtractorSimpleBody`	net472	278ns	0.218ns	0.817ns	0.0574	0	361 B
#3363	`BodyExtractorSimpleBody`	netcoreapp3.1	236ns	0.162ns	0.605ns	0.00366	0	272 B
#3363	`BodyExtractorMoreComplexBody`	net472	15.7μs	12.5ns	46.7ns	1.2	0.0158	7.62 KB
#3363	`BodyExtractorMoreComplexBody`	netcoreapp3.1	13μs	6.97ns	26.1ns	0.091	0	6.75 KB

Benchmarks.Trace.AspNetCoreBenchmark - Unknown 🤷 Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
master	`SendRequest`	netcoreapp3.1	178μs	276ns	1.07μs	0.267	20.66 KB
#3363	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3363	`SendRequest`	netcoreapp3.1	177μs	141ns	507ns	0.264	20.66 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net472	1.81μs	1.2ns	4.64ns	0.157	0.000909	995 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.42μs	0.463ns	1.67ns	0.0135	0	984 B
#3363	`ExecuteNonQuery`	net472	1.72μs	1ns	3.87ns	0.157	0.00086	995 B
#3363	`ExecuteNonQuery`	netcoreapp3.1	1.41μs	0.524ns	1.96ns	0.0134	0	984 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.57μs	0.855ns	3.2ns	0.191	1.2 KB
master	`CallElasticsearch`	netcoreapp3.1	1.54μs	0.628ns	2.27ns	0.0154	1.14 KB
master	`CallElasticsearchAsync`	net472	2.65μs	1.34ns	5.01ns	0.212	1.34 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.63μs	0.513ns	1.92ns	0.0171	1.26 KB
#3363	`CallElasticsearch`	net472	2.57μs	2.12ns	7.92ns	0.191	1.2 KB
#3363	`CallElasticsearch`	netcoreapp3.1	1.57μs	0.625ns	2.34ns	0.015	1.14 KB
#3363	`CallElasticsearchAsync`	net472	2.61μs	1.86ns	7.21ns	0.212	1.34 KB
#3363	`CallElasticsearchAsync`	netcoreapp3.1	1.57μs	0.753ns	2.82ns	0.0173	1.26 KB

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	2.64μs	1.84ns	7.14ns	0.233	1.47 KB
master	`ExecuteAsync`	netcoreapp3.1	1.72μs	0.709ns	2.46ns	0.0188	1.39 KB
#3363	`ExecuteAsync`	net472	2.74μs	2.32ns	8.67ns	0.233	1.47 KB
#3363	`ExecuteAsync`	netcoreapp3.1	1.76μs	0.792ns	2.96ns	0.0185	1.39 KB

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	5.95μs	2.16ns	8.09ns	0.445	2.82 KB
master	`SendAsync`	netcoreapp3.1	3.71μs	1.02ns	3.68ns	0.0353	2.65 KB
#3363	`SendAsync`	net472	5.9μs	2.15ns	8.35ns	0.447	2.82 KB
#3363	`SendAsync`	netcoreapp3.1	3.67μs	1.42ns	5.12ns	0.0348	2.65 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.19μs	4.13ns	15.4ns	0.297	1.88 KB
master	`EnrichedLog`	netcoreapp3.1	2.55μs	2.59ns	9.68ns	0.0256	1.91 KB
#3363	`EnrichedLog`	net472	3.2μs	3.43ns	13.3ns	0.298	1.88 KB
#3363	`EnrichedLog`	netcoreapp3.1	2.55μs	1.1ns	4.13ns	0.0267	1.91 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	150μs	66.3ns	257ns	0.748	0.224	4.72 KB
master	`EnrichedLog`	netcoreapp3.1	118μs	68.3ns	237ns	0.0591	0	4.55 KB
#3363	`EnrichedLog`	net472	153μs	213ns	823ns	0.689	0.23	4.72 KB
#3363	`EnrichedLog`	netcoreapp3.1	120μs	166ns	622ns	0.0596	0	4.55 KB

Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	5.84μs	2.37ns	9.18ns	0.579	0.00293	3.65 KB
master	`EnrichedLog`	netcoreapp3.1	4.4μs	1.26ns	4.55ns	0.0551	0	3.98 KB
#3363	`EnrichedLog`	net472	5.85μs	2.04ns	7.62ns	0.578	0.00292	3.65 KB
#3363	`EnrichedLog`	netcoreapp3.1	4.58μs	1.97ns	7.64ns	0.0527	0	3.98 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.22μs	4.44ns	17.2ns	0.225	1.42 KB
master	`SendReceive`	netcoreapp3.1	1.84μs	0.609ns	2.28ns	0.0184	1.37 KB
#3363	`SendReceive`	net472	2.25μs	2.76ns	10.7ns	0.225	1.42 KB
#3363	`SendReceive`	netcoreapp3.1	1.8μs	0.754ns	2.72ns	0.0189	1.37 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	5.07μs	1.53ns	5.92ns	0.363	2.3 KB
master	`EnrichedLog`	netcoreapp3.1	4.19μs	1.54ns	5.76ns	0.0254	1.86 KB
#3363	`EnrichedLog`	net472	5.16μs	2.01ns	7.51ns	0.363	2.3 KB
#3363	`EnrichedLog`	netcoreapp3.1	4.23μs	1.49ns	5.78ns	0.0254	1.86 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #3363

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1	1.135	1,047.88	1,189.58

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	1.23μs	0.308ns	1.15ns	0.136	859 B
master	`StartFinishSpan`	netcoreapp3.1	936ns	0.354ns	1.33ns	0.0112	808 B
master	`StartFinishScope`	net472	1.49μs	0.35ns	1.26ns	0.149	939 B
master	`StartFinishScope`	netcoreapp3.1	1.05μs	0.479ns	1.85ns	0.0125	928 B
#3363	`StartFinishSpan`	net472	1.15μs	0.214ns	0.828ns	0.136	859 B
#3363	`StartFinishSpan`	netcoreapp3.1	986ns	0.351ns	1.31ns	0.0109	808 B
#3363	`StartFinishScope`	net472	1.47μs	0.388ns	1.45ns	0.149	939 B
#3363	`StartFinishScope`	netcoreapp3.1	1.19μs	0.331ns	1.2ns	0.0119	928 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.57μs	0.352ns	1.32ns	0.149	939 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.16μs	0.389ns	1.45ns	0.0124	928 B
#3363	`RunOnMethodBegin`	net472	1.6μs	0.48ns	1.86ns	0.149	939 B
#3363	`RunOnMethodBegin`	netcoreapp3.1	1.15μs	0.322ns	1.2ns	0.0126	928 B

andrewlock · 2022-10-21T15:09:17Z

I couldn't for the life of me get the dd-trace tool to include the pdbs inside the standalone tool. It just refuses, and always outputs them side-by-side at best. @kevingosse now tells me he doesn't see much value in having the managed pdbs anyway.. so this exercise may have been moot 🙄 I'm not sure if we actually do want them for things like ciapp and live debugger though, so we can always revisit later.

Something worth considering though: using dd-trace myexe for instrumentation will not have the same information as when using msi/ other installers

andrewlock · 2022-10-21T16:05:53Z

Code Coverage Report 📊

✔️ Merging #3363 into master will not change line coverage
⚠️ Merging #3363 into master will will decrease branch coverage by 1%
✔️ Merging #3363 into master will not change complexity

	master	#3363	Change
Lines	`19616` / `27081`	`19481` / `27081`
Lines %	`72%`	`72%`	`0%` ✔️
Branches	`11602` / `16764`	`11499` / `16764`
Branches %	`69%`	`69%`	`-1%` ⚠️
Complexity	`18146`	`18146`	`0` ✔️

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3363	Change
Lines %	`72%`	`72%`	`0%` ✔️
Branches %	`69%`	`69%`	`-1%` ⚠️
Complexity	`18146`	`18146`	`0` ✔️

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Agent.TraceSamplers.AnalyticsEventsSampler	`-100%` ⛔	`-100%` ⛔	`0` ✔️
Datadog.Trace.Agent.TraceSamplers.ErrorSampler	`-100%` ⛔	`-100%` ⛔	`0` ✔️
Datadog.Trace.Agent.TraceSamplers.PrioritySampler	`-100%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.Sampling.GlobalSamplingRule	`-100%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.Agent.TraceSamplers.RareSampler	`-89%` ⛔	`-96%` ⛔	`0` ✔️
Datadog.Trace.Util.SamplingHelpers	`-50%` ⛔	`-50%` ⛔	`0` ✔️
Datadog.Trace.Telemetry.Transports.JsonTelemetryTransport	`-33%` ⛔	`-42%` ⛔	`0` ✔️
Datadog.Trace.Logging.DirectSubmission.DirectLogSubmissionManager	`-33%` ⛔	`-33%` ⛔	`0` ✔️
Datadog.Trace.Logging.DirectSubmission.Sink.NullDatadogSink	`-25%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.Processors.NormalizerTraceProcessor	`-22%` ⛔	`-19%` ⛔	`0` ✔️
...And 9 more

View the full reports for further details:

andrewlock added area:builds project files, build scripts, pipelines, versioning, releases, packages area:tools labels Oct 18, 2022

andrewlock requested a review from a team as a code owner October 18, 2022 10:22

kevingosse approved these changes Oct 18, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

pierotibou approved these changes Oct 18, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

andrewlock requested review from a team as code owners October 18, 2022 13:42

This comment has been minimized.

Sign in to view

lucaspimentel approved these changes Oct 18, 2022

View reviewed changes

andrewlock force-pushed the andrew/remove-xml-files-from-dotnet-tool branch from cdd165a to 8c77d5e Compare October 19, 2022 09:15

This comment has been minimized.

Sign in to view

andrewlock force-pushed the andrew/remove-xml-files-from-dotnet-tool branch from 8c77d5e to 50bfeb9 Compare October 19, 2022 14:49

This comment has been minimized.

Sign in to view

andrewlock force-pushed the andrew/remove-xml-files-from-dotnet-tool branch from 50bfeb9 to 09350c4 Compare October 20, 2022 11:29

This comment has been minimized.

Sign in to view

andrewlock force-pushed the andrew/remove-xml-files-from-dotnet-tool branch from 09350c4 to a0e4d09 Compare October 20, 2022 13:57

This comment has been minimized.

Sign in to view

andrewlock added 6 commits October 21, 2022 12:28

Exclude xml files from dotnet-tool and Datadog.Trace.Bundle

eae0a18

Include the pdbs in the standalone tool home folder

f92ce4f

Disable PDB generation for dd-trace-tool

eb754d4

Disable xml generation in dd-trace-tool

f2d00d1

Fix Bundle XML removal

553f6cc

Try to fix output for dd-trace

aff3596

andrewlock added 2 commits October 21, 2022 12:28

Rebuild all dependencies of runner tools

00578d0

To ensure we strip out pdbs and xml files

Create separate "PackRunnerToolNuget" with different settings

6205dcd

This is necessary because we _want_ the pdbs etc when doing unit tests etc. Also, trying to do otherwise breaks various build stuff. It's not perfect, but it'll have to do

andrewlock force-pushed the andrew/remove-xml-files-from-dotnet-tool branch from 86dd3d4 to 6205dcd Compare October 21, 2022 11:28

This comment has been minimized.

Sign in to view

I guess we can't have nice things

623e8b9

andrewlock merged commit f652d41 into master Oct 21, 2022

andrewlock deleted the andrew/remove-xml-files-from-dotnet-tool branch October 21, 2022 15:52

github-actions Bot added this to the vNext milestone Oct 21, 2022

Uh oh!

Conversation

andrewlock commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

andrewlock commented Oct 21, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #3363

Raw results

Raw results

Uh oh!

andrewlock commented Oct 21, 2022

Uh oh!

andrewlock commented Oct 21, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrewlock commented Oct 18, 2022 •

edited

Loading