Revisit CMake projects architecture by gleocadie · Pull Request #3329 · DataDog/dd-trace-dotnet

gleocadie · 2022-10-08T17:43:37Z

Summary of changes

Revisiting the CMake projects architecture, to unify dependencies amongst projects.

Reason for change

Profiler, tracer and native loader have native dependencies, but each project has its own way to download/retrieve it, to build it. This becomes difficult when we need to update to a newer version. We would have to touch different part of the repository.

Implementation details

Add top-level CMakeLists.txt file and intermediate ones
Define all dependencies as library in CMake packages.

Test coverage

Other details

corprof_i.cpp is compiled multiple times by the profiler, the tracer and the native loader. With this new type of library (OBJECT), we can compile it once and use the object file every time.

chrisnas

LGTM after validating comments

andrewlock · 2022-10-13T15:47:23Z

Benchmarks Report 🐌

Benchmarks for #3329 compared to master:

1 benchmarks are faster, with geometric mean 1.174
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	782μs	876ns	3.39μs	0.388	3.22 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	535μs	197ns	736ns	0	2.63 KB
#3329	`WriteAndFlushEnrichedTraces`	net472	804μs	1.31μs	6.15μs	0.434	3.22 KB
#3329	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	534μs	519ns	1.94μs	0	2.62 KB

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

Faster 🎉 in #3329

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.AppSecBodyBenchmark.BodyExtractorSimpleBody‑net472	1.174	316.94	269.85

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	198ns	0.0787ns	0.284ns	0.0726	0	457 B
master	`AllCycleSimpleBody`	netcoreapp3.1	255ns	0.146ns	0.548ns	0.00614	0	456 B
master	`AllCycleMoreComplexBody`	net472	200ns	0.052ns	0.18ns	0.0688	0	433 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	252ns	0.161ns	0.601ns	0.00595	0	432 B
master	`BodyExtractorSimpleBody`	net472	317ns	0.0885ns	0.319ns	0.0572	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	238ns	0.0973ns	0.351ns	0.0037	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	15.8μs	17.1ns	63.8ns	1.21	0.0158	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.8μs	5.29ns	20.5ns	0.089	0	6.75 KB
#3329	`AllCycleSimpleBody`	net472	196ns	0.0546ns	0.204ns	0.0727	0	457 B
#3329	`AllCycleSimpleBody`	netcoreapp3.1	261ns	0.393ns	1.52ns	0.00624	0	456 B
#3329	`AllCycleMoreComplexBody`	net472	195ns	0.0552ns	0.199ns	0.0689	0	433 B
#3329	`AllCycleMoreComplexBody`	netcoreapp3.1	258ns	0.395ns	1.48ns	0.0059	0	432 B
#3329	`BodyExtractorSimpleBody`	net472	270ns	0.118ns	0.443ns	0.0573	0	361 B
#3329	`BodyExtractorSimpleBody`	netcoreapp3.1	240ns	0.165ns	0.637ns	0.00374	0	272 B
#3329	`BodyExtractorMoreComplexBody`	net472	15.7μs	12.3ns	47.7ns	1.2	0.0156	7.62 KB
#3329	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.9μs	3.8ns	14.2ns	0.0903	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	181μs	185ns	715ns	0.27	20.68 KB
#3329	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3329	`SendRequest`	netcoreapp3.1	183μs	290ns	1.12μs	0.182	20.68 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.79μs	0.719ns	2.69ns	0.155	0.000901	979 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.46μs	0.646ns	2.33ns	0.0131	0	968 B
#3329	`ExecuteNonQuery`	net472	1.79μs	1.16ns	4.17ns	0.155	0.000895	979 B
#3329	`ExecuteNonQuery`	netcoreapp3.1	1.49μs	0.803ns	3.01ns	0.0126	0	968 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.48μs	1.12ns	4.35ns	0.188	1.19 KB
master	`CallElasticsearch`	netcoreapp3.1	1.58μs	0.725ns	2.81ns	0.015	1.13 KB
master	`CallElasticsearchAsync`	net472	2.7μs	1.22ns	4.73ns	0.209	1.32 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.63μs	0.616ns	2.31ns	0.0171	1.25 KB
#3329	`CallElasticsearch`	net472	2.53μs	0.716ns	2.58ns	0.188	1.19 KB
#3329	`CallElasticsearch`	netcoreapp3.1	1.6μs	1.53ns	5.71ns	0.0151	1.13 KB
#3329	`CallElasticsearchAsync`	net472	2.69μs	0.896ns	3.47ns	0.21	1.32 KB
#3329	`CallElasticsearchAsync`	netcoreapp3.1	1.7μs	0.828ns	2.99ns	0.017	1.25 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	2.76μs	1.59ns	6.16ns	0.23	1.45 KB
master	`ExecuteAsync`	netcoreapp3.1	1.73μs	0.672ns	2.42ns	0.0183	1.38 KB
#3329	`ExecuteAsync`	net472	2.85μs	1.22ns	4.39ns	0.231	1.45 KB
#3329	`ExecuteAsync`	netcoreapp3.1	1.75μs	0.564ns	2.03ns	0.0183	1.38 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	5.85μs	2.16ns	8.36ns	0.444	2.8 KB
master	`SendAsync`	netcoreapp3.1	3.63μs	1.26ns	4.7ns	0.0344	2.63 KB
#3329	`SendAsync`	net472	5.96μs	2.56ns	9.92ns	0.445	2.8 KB
#3329	`SendAsync`	netcoreapp3.1	3.76μs	20.1ns	108ns	0.0357	2.63 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.13μs	3.5ns	13.1ns	0.297	1.88 KB
master	`EnrichedLog`	netcoreapp3.1	2.44μs	2.13ns	7.96ns	0.0257	1.91 KB
#3329	`EnrichedLog`	net472	3.17μs	2.97ns	11.1ns	0.298	1.88 KB
#3329	`EnrichedLog`	netcoreapp3.1	2.53μs	1.02ns	3.82ns	0.0264	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	153μs	172ns	642ns	0.688	0.229	4.72 KB
master	`EnrichedLog`	netcoreapp3.1	122μs	229ns	886ns	0.0606	0	4.55 KB
#3329	`EnrichedLog`	net472	152μs	168ns	651ns	0.684	0.228	4.72 KB
#3329	`EnrichedLog`	netcoreapp3.1	121μs	177ns	686ns	0.0601	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.66μs	2.45ns	9.18ns	0.578	0.00284	3.65 KB
master	`EnrichedLog`	netcoreapp3.1	4.81μs	1.97ns	7.35ns	0.0531	0	3.98 KB
#3329	`EnrichedLog`	net472	6.02μs	2.4ns	8.97ns	0.578	0.00301	3.65 KB
#3329	`EnrichedLog`	netcoreapp3.1	4.54μs	3.37ns	13ns	0.0544	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.35μs	2.84ns	11ns	0.222	1.4 KB
master	`SendReceive`	netcoreapp3.1	1.87μs	0.794ns	2.97ns	0.0179	1.35 KB
#3329	`SendReceive`	net472	2.35μs	2.87ns	11.1ns	0.223	1.4 KB
#3329	`SendReceive`	netcoreapp3.1	1.86μs	1.09ns	4.06ns	0.0184	1.35 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	5.08μs	1.81ns	7ns	0.364	2.3 KB
master	`EnrichedLog`	netcoreapp3.1	4.18μs	1.36ns	5.28ns	0.0251	1.86 KB
#3329	`EnrichedLog`	net472	5.21μs	2.8ns	10.9ns	0.363	2.3 KB
#3329	`EnrichedLog`	netcoreapp3.1	4.19μs	14.9ns	57.5ns	0.0249	1.86 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	1.21μs	0.369ns	1.33ns	0.134	842 B
master	`StartFinishSpan`	netcoreapp3.1	959ns	0.552ns	2.14ns	0.0111	792 B
master	`StartFinishScope`	net472	1.37μs	0.43ns	1.67ns	0.147	923 B
master	`StartFinishScope`	netcoreapp3.1	1.07μs	0.261ns	0.975ns	0.0123	912 B
#3329	`StartFinishSpan`	net472	1.25μs	0.63ns	2.36ns	0.134	842 B
#3329	`StartFinishSpan`	netcoreapp3.1	917ns	0.473ns	1.77ns	0.0105	792 B
#3329	`StartFinishScope`	net472	1.4μs	0.409ns	1.59ns	0.146	923 B
#3329	`StartFinishScope`	netcoreapp3.1	1.11μs	0.326ns	1.26ns	0.0123	912 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.54μs	0.34ns	1.27ns	0.147	923 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.18μs	0.32ns	1.24ns	0.0123	912 B
#3329	`RunOnMethodBegin`	net472	1.53μs	0.456ns	1.76ns	0.146	923 B
#3329	`RunOnMethodBegin`	netcoreapp3.1	1.12μs	0.565ns	2.04ns	0.0124	912 B

andrewlock · 2022-10-13T17:35:03Z

Code Coverage Report 📊

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

	master	#3329	Change
Lines	`19534` / `26924`	`19522` / `26930`
Lines %	`73%`	`72%`	`0%` ✔️
Branches	`11607` / `16747`	`11611` / `16747`
Branches %	`69%`	`69%`	`0%` ✔️
Complexity	`18113`	`18117`	`4` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3329	Change
Lines %	`73%`	`72%`	`0%` ✔️
Branches %	`69%`	`69%`	`0%` ✔️
Complexity	`18113`	`18117`	`4` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Ci.GitInfo	`-17%` ⛔	`-11%` ⛔	`0` ✔️
Datadog.Trace.Iast.StackWalker	`0%` ✔️	`-6%` ⛔	`0` ✔️
Datadog.Trace.Propagators.SpanContextPropagator	`8%` ✔️	`6%` ✔️	`0` ✔️

The following classes were added in #3329:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.Sampling.SpanRateLimiter	`0%`	`100%`	`4`

View the full reports for further details:

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch 2 times, most recently from 4fe9a14 to 38e6f74 Compare October 8, 2022 21:26

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch 2 times, most recently from d13f649 to 030c661 Compare October 8, 2022 22:36

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch from 9f8c407 to 68be990 Compare October 9, 2022 19:30

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch 3 times, most recently from 416275c to 47966d2 Compare October 10, 2022 00:36

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch 7 times, most recently from 09c3df5 to 1715a64 Compare October 10, 2022 02:03

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch from 1715a64 to d6daa9a Compare October 10, 2022 11:50

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch from d6daa9a to 5f0e463 Compare October 10, 2022 17:02

This comment has been minimized.

Sign in to view

gleocadie force-pushed the gleocadie/use-module-for-cmake branch from ab91842 to 78fc425 Compare October 10, 2022 21:06

This comment has been minimized.

Sign in to view

gleocadie added 17 commits October 13, 2022 13:33

Remove useless folders

5840997

Adjust output dir

3e09705

Fix Linux build

c6c907f

Fix linux publishing

a7c20d9

Fix MacOS build

db7bee7

Fix CMake warning on MacOS

77ea5c9

Complete the package

ec1376a

Add top-level CMakeLists.txt

9f4a9ed

Fix MacOS build

a62da72

Fix Linux build

f25c64c

Fix CI jobs

33a120c

Add profiler to the loop

1a709e2

Add profiler to the loop

7182cd4

Cleanup: Add missing CMakeLists.txt file to version bump

05b9dae

Ensure corprof_i.cpp is compiled once

b70c84e

corprof_i.cpp is compiled multiple times by the profiler, the tracer and the native loader. With this new type of library (OBJECT), we can compile it once and use the object file every time.

Address PR comment

9e43f32

Fix compilation warning for MacOS

0be69c0

gleocadie force-pushed the gleocadie/use-module-for-cmake branch from 523a02e to 0be69c0 Compare October 13, 2022 11:33

This comment has been minimized.

Sign in to view

chrisnas approved these changes Oct 13, 2022

View reviewed changes

Comment thread CMakeLists.txt Outdated

Comment thread tracer/build/_build/Build.Shared.Steps.cs Outdated

Comment thread tracer/build/_build/Build.Shared.Steps.cs Outdated

Address PR comment

608a9bd

gleocadie force-pushed the gleocadie/use-module-for-cmake branch from c4b12eb to 608a9bd Compare October 13, 2022 15:05

gleocadie merged commit e5f076c into master Oct 14, 2022

gleocadie deleted the gleocadie/use-module-for-cmake branch October 14, 2022 05:09

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

anna-git added the area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) label Oct 25, 2022

anna-git mentioned this pull request Oct 25, 2022

Change list of expected modified files for release #3387

Merged

Uh oh!

Conversation

gleocadie commented Oct 8, 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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

chrisnas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewlock commented Oct 13, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Faster 🎉 in #3329

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

andrewlock commented Oct 13, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gleocadie commented Oct 8, 2022 •

edited

Loading