[Profiler] Stop building the profiler outside of the repository folder by kevingosse · Pull Request #2651 · DataDog/dd-trace-dotnet

kevingosse · 2022-04-07T09:50:48Z

Move the profiler _build folder from ../ (outside of the repository) to profiler/
Move Datadog.AutoInstrumentation.Profiler.sln to the root folder
Move nuget packages from ../_build/ImportedPackages to packages/
Delete the CodeHotspotsPoC project (nobody uses it and it doesn't even compile)

andrewlock

What a saint

gleocadie

LGTM

andrewlock · 2022-04-07T19:14:26Z

Benchmarks Report 🐌

Benchmarks for #2651 compared to master:

1 benchmarks are faster, with geometric mean 1.227
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	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	919μs	3.7μs	14.3μs	3.16 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	708μs	2.83μs	11.3μs	2.57 KB
#2651	`WriteAndFlushEnrichedTraces`	net472	918μs	4.16μs	20.8μs	3.16 KB
#2651	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	710μs	3.42μs	14.1μs	2.57 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	293ns	1.38ns	6.18ns	0.052	0	329 B
master	`AllCycleSimpleBody`	netcoreapp3.1	385ns	2.15ns	13.9ns	0.00458	0	328 B
master	`AllCycleMoreComplexBody`	net472	287ns	1.1ns	3.95ns	0.0482	0	305 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	361ns	1.2ns	4.64ns	0.00418	0	304 B
master	`BodyExtractorSimpleBody`	net472	426ns	1.73ns	6.7ns	0.057	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	444ns	2.48ns	15.5ns	0.00362	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	24.1μs	107ns	413ns	1.19	0.0119	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	21.2μs	119ns	768ns	0.0931	0	6.75 KB
#2651	`AllCycleSimpleBody`	net472	290ns	0.924ns	3.33ns	0.052	0	329 B
#2651	`AllCycleSimpleBody`	netcoreapp3.1	373ns	1.28ns	4.77ns	0.00442	0	328 B
#2651	`AllCycleMoreComplexBody`	net472	288ns	0.875ns	3.39ns	0.0481	0	305 B
#2651	`AllCycleMoreComplexBody`	netcoreapp3.1	377ns	1.88ns	11.1ns	0.00407	0	304 B
#2651	`BodyExtractorSimpleBody`	net472	419ns	1.39ns	5.39ns	0.0569	0	361 B
#2651	`BodyExtractorSimpleBody`	netcoreapp3.1	430ns	2.06ns	8.5ns	0.00365	0	272 B
#2651	`BodyExtractorMoreComplexBody`	net472	23.7μs	101ns	391ns	1.19	0.0116	7.62 KB
#2651	`BodyExtractorMoreComplexBody`	netcoreapp3.1	21μs	75.9ns	274ns	0.0931	0	6.75 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ 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	295μs	1.4μs	5.41μs	0.144	19.74 KB
#2651	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#2651	`SendRequest`	netcoreapp3.1	301μs	974ns	3.77μs	0.146	19.74 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net472	1.77μs	9.04ns	39.4ns	0.0941	594 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.45μs	4.94ns	19.1ns	0.00869	632 B
#2651	`ExecuteNonQuery`	net472	1.76μs	6.96ns	27ns	0.0936	594 B
#2651	`ExecuteNonQuery`	netcoreapp3.1	1.48μs	5.3ns	20.5ns	0.00866	632 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.79μs	14.7ns	79.1ns	0.127	802 B
master	`CallElasticsearch`	netcoreapp3.1	1.7μs	8.93ns	41.9ns	0.0108	792 B
master	`CallElasticsearchAsync`	net472	2.93μs	14.7ns	67.4ns	0.149	939 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.81μs	7.84ns	30.4ns	0.0127	912 B
#2651	`CallElasticsearch`	net472	2.76μs	10.2ns	38.1ns	0.127	803 B
#2651	`CallElasticsearch`	netcoreapp3.1	1.83μs	7.06ns	26.4ns	0.0109	792 B
#2651	`CallElasticsearchAsync`	net472	2.91μs	10.4ns	38.9ns	0.149	939 B
#2651	`CallElasticsearchAsync`	netcoreapp3.1	1.8μs	9.15ns	43.9ns	0.0124	912 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	3.25μs	18.1ns	120ns	0.166	1.06 KB
master	`ExecuteAsync`	netcoreapp3.1	2.03μs	6.47ns	25.1ns	0.0136	1.03 KB
#2651	`ExecuteAsync`	net472	3.16μs	10.1ns	39.3ns	0.168	1.06 KB
#2651	`ExecuteAsync`	netcoreapp3.1	2μs	5.81ns	21.7ns	0.0141	1.03 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	6.92μs	35.6ns	181ns	0.362	2.28 KB
master	`SendAsync`	netcoreapp3.1	5.06μs	24.6ns	98.2ns	0.0298	2.21 KB
#2651	`SendAsync`	net472	6.74μs	31.9ns	128ns	0.361	2.28 KB
#2651	`SendAsync`	netcoreapp3.1	4.58μs	16.3ns	63.1ns	0.0301	2.21 KB

Benchmarks.Trace.ILoggerBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #2651

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.ILoggerBenchmark.EnrichedLog‑netcoreapp3.1	1.227	3,863.00	3,147.47

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.34μs	9.4ns	35.2ns	0.228	1.45 KB
master	`EnrichedLog`	netcoreapp3.1	3.86μs	28.7ns	285ns	0.0222	1.53 KB
#2651	`EnrichedLog`	net472	3.4μs	13ns	50.2ns	0.228	1.45 KB
#2651	`EnrichedLog`	netcoreapp3.1	3.16μs	17.2ns	97.5ns	0.0211	1.53 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	302μs	1.75μs	16.1μs	0.318	0.159	4.33 KB
master	`EnrichedLog`	netcoreapp3.1	240μs	1.3μs	7.68μs	0	0	4.21 KB
#2651	`EnrichedLog`	net472	290μs	789ns	3.06μs	0.307	0.154	4.33 KB
#2651	`EnrichedLog`	netcoreapp3.1	234μs	1.12μs	5.58μs	0	0	4.21 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	7.63μs	31.5ns	130ns	0.506	3.23 KB
master	`EnrichedLog`	netcoreapp3.1	6.71μs	36.4ns	206ns	0.0469	3.6 KB
#2651	`EnrichedLog`	net472	7.74μs	15.6ns	58.4ns	0.504	3.23 KB
#2651	`EnrichedLog`	netcoreapp3.1	6.51μs	27.9ns	108ns	0.0508	3.6 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`SendReceive`	net472	2.46μs	11ns	42.4ns	0.16	0.00119	1.01 KB
master	`SendReceive`	netcoreapp3.1	2.02μs	9.04ns	35ns	0.0139	0	1.01 KB
#2651	`SendReceive`	net472	2.37μs	11.8ns	49.9ns	0.16	0	1.01 KB
#2651	`SendReceive`	netcoreapp3.1	1.99μs	8.03ns	30ns	0.0143	0	1.01 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	6.02μs	28.5ns	106ns	0.291	1.87 KB
master	`EnrichedLog`	netcoreapp3.1	5.32μs	22.8ns	88.5ns	0.0189	1.49 KB
#2651	`EnrichedLog`	net472	6.31μs	28.2ns	132ns	0.291	1.87 KB
#2651	`EnrichedLog`	netcoreapp3.1	5.52μs	28.4ns	133ns	0.0205	1.49 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	892ns	4.14ns	16.6ns	0.0719	457 B
master	`StartFinishSpan`	netcoreapp3.1	898ns	2.59ns	9.68ns	0.00618	456 B
master	`StartFinishScope`	net472	1.11μs	4.58ns	16.5ns	0.0841	538 B
master	`StartFinishScope`	netcoreapp3.1	1.05μs	2.02ns	6.99ns	0.00768	576 B
#2651	`StartFinishSpan`	net472	878ns	2.77ns	10.4ns	0.0716	457 B
#2651	`StartFinishSpan`	netcoreapp3.1	874ns	3ns	11.2ns	0.006	456 B
#2651	`StartFinishScope`	net472	1.09μs	3.71ns	12.8ns	0.0845	538 B
#2651	`StartFinishScope`	netcoreapp3.1	1.05μs	4.24ns	16.4ns	0.00775	576 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.28μs	5.02ns	27.5ns	0.0845	538 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.16μs	3.5ns	13.1ns	0.00789	576 B
#2651	`RunOnMethodBegin`	net472	1.33μs	6.59ns	34.3ns	0.0842	538 B
#2651	`RunOnMethodBegin`	netcoreapp3.1	1.18μs	6.17ns	31.5ns	0.00782	576 B

zacharycmontoya

This is great 👍🏼

chrisnas · 2022-04-12T11:05:29Z

-# Visual Studio Version 16
-VisualStudioVersion = 16.0.30611.23
+# Visual Studio Version 17
+VisualStudioVersion = 17.1.32319.34


Would it be possible to open it with VS 2019?

You'll tell me 😛

chrisnas · 2022-04-12T11:15:38Z

        <!-- However, ForceBinaryOutputIntoEnlistment allows putting the binarlies INTO the repository root.                                                -->
-        <BuildOutputRoot Condition=" '$(ForceBinaryOutputIntoEnlistment)' == 'True' ">$(EnlistmentRoot)\$(BuildOutputFolderName)</BuildOutputRoot>
-        <BuildOutputRoot Condition=" '$(ForceBinaryOutputIntoEnlistment)' != 'True' ">$(EnlistmentRoot)\..\$(BuildOutputFolderName)</BuildOutputRoot>
+        <BuildOutputRoot>$(EnlistmentRoot)\profiler\$(BuildOutputFolderName)</BuildOutputRoot>


In this .props file, all the comments related to ForceBinaryOutputIntoEnlistment should be deleted because it is no more used (true by default if I understand well + profiler_build instead of .._build)

Fluff

andrewlock · 2022-04-12T13:57:42Z

Code Coverage Report 📊

✔️ Merging #2651 into master will not change line coverage
✔️ Merging #2651 into master will not change branch coverage
⛔ Merging #2651 into master will will increase complexity by 47

	master	#2651	Change
Lines	`13538` / `18627`	`13590` / `18669`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`7815` / `11125`	`7855` / `11165`
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`12431`	`12478`	`47` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#2651	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`12431`	`12478`	`47` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Telemetry.JsonWebRequestTelemetryTransport	`-22%` ⛔	`-11%` ⛔	`30` ⛔
Datadog.Trace.Agent.Transports.ApiWebRequestFactory	`-5%` ⛔	`-6%` ⛔	`11` ⛔
Datadog.Trace.Ci.CIVisibility	`7%` ✔️	`7%` ✔️	`0` ✔️
Datadog.Trace.Telemetry.JsonHttpClientTelemetryTransport	`26%` ✔️	`38%` ✔️	`0` ✔️

View the full reports for further details:

kevingosse force-pushed the kevin/profiler_build2 branch 8 times, most recently from 97b53ea to 55c825b Compare April 7, 2022 12:12

andrewlock added the area:profiler Issues related to the continous-profiler label Apr 7, 2022

kevingosse force-pushed the kevin/profiler_build2 branch from 55c825b to b8bbc47 Compare April 7, 2022 13:22

kevingosse marked this pull request as ready for review April 7, 2022 14:21

kevingosse requested review from a team as code owners April 7, 2022 14:21

kevingosse changed the title ~~Stop building the profiler outside of the repository folder~~ [Profiler] Stop building the profiler outside of the repository folder Apr 7, 2022

andrewlock added the area:builds project files, build scripts, pipelines, versioning, releases, packages label Apr 7, 2022

andrewlock approved these changes Apr 7, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

gleocadie approved these changes Apr 7, 2022

View reviewed changes

zacharycmontoya approved these changes Apr 7, 2022

View reviewed changes

chrisnas previously requested changes Apr 12, 2022

View reviewed changes

github-actions Bot added the area-profiler label Apr 12, 2022

kevingosse requested a review from chrisnas April 12, 2022 12:19

kevingosse added 6 commits April 12, 2022 14:23

Stop building the profiler outside of the repository folder

8ae84c1

fix Website-Aspnet

55b2882

Update gitignore

ae1436f

Fixes

10fd8ab

Update Directory.Build.props

0990b0c

Update RepoAndBuild-DirectoryStructure.props

a87ac7f

kevingosse force-pushed the kevin/profiler_build2 branch from e8733d8 to a87ac7f Compare April 12, 2022 12:26

Fix sln and props

d267028

kevingosse merged commit ccd05d5 into master Apr 12, 2022

kevingosse deleted the kevin/profiler_build2 branch April 12, 2022 14:38

github-actions Bot added this to the vNext milestone Apr 12, 2022

pierotibou removed the area-profiler label Apr 13, 2022

Uh oh!

Conversation

kevingosse commented Apr 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

gleocadie left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Apr 7, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Faster 🎉 in #2651

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

zacharycmontoya left a comment

Choose a reason for hiding this comment

Uh oh!

chrisnas Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

kevingosse Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

chrisnas Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

kevingosse Apr 12, 2022

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Apr 12, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kevingosse commented Apr 7, 2022 •

edited

Loading