Add continous profiler checks to dd-trace by kevingosse · Pull Request #3167 · DataDog/dd-trace-dotnet

kevingosse · 2022-09-05T09:09:10Z

Summary of changes

Add checks for the continuous profiler in the diagnostic tool.

Reason for change

Implementation details

Those checks are only run if DD_PROFILING_ENABLED is set, to avoid false-negatives.

We verify that:

The native loaded is loaded, and the loader.conf file is at the expected location
The continuous profiler is loaded
LD_PRELOAD is set to a correct path (Linux only)

Test coverage

The new tests run only on Linux.

andrewlock

LGTM, couple of minor comments

gleocadie

LGTM

zacharycmontoya · 2022-09-09T23:38:21Z

+            bool isContinuousProfilerEnabled;

-            if (profilerModule == null)
+            if (process.EnvironmentVariables.TryGetValue("DD_PROFILING_ENABLED", out var profilingEnabled))


You might want to run all of the ContinuousProfiler checks after identifying the loader module and the native tracer module because these console messages aren't actionable if the profiler isn't a part of the product they installed

zacharycmontoya · 2022-09-09T23:43:06Z

+            var loaderModule = FindLoader(process);
+            var nativeTracerModule = FindNativeTracerModule(process, loaderModule != null);
+
+            if (loaderModule == null)


If the user is running a pretty old (pre-loader DLL), then the loader isn't relevant and the user could get concerned over this message. What about strengthening the check like this?

Suggested change

if (loaderModule == null)

if (loaderModule == null && (nativeTracerModule?.StartsWith("Datadog.Tracer.Native", StringComparison.OrdinalIgnoreCase) ?? true))

Co-authored-by: Andrew Lock <[email protected]>

andrewlock · 2022-09-12T10:03:53Z

Benchmarks Report 🐌

Benchmarks for #3167 compared to master:

All benchmarks have the same speed
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	720μs	425ns	1.59μs	0.361	3.18 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	470μs	203ns	731ns	0	2.58 KB
#3167	`WriteAndFlushEnrichedTraces`	net472	721μs	390ns	1.51μs	0.357	3.18 KB
#3167	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	470μs	215ns	803ns	0	2.58 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	186ns	0.115ns	0.429ns	0.0676	0	425 B
master	`AllCycleSimpleBody`	netcoreapp3.1	242ns	0.658ns	2.55ns	0.00583	0	424 B
master	`AllCycleMoreComplexBody`	net472	203ns	0.173ns	0.668ns	0.0637	0	401 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	233ns	0.238ns	0.892ns	0.00547	0	400 B
master	`BodyExtractorSimpleBody`	net472	260ns	0.162ns	0.607ns	0.0573	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	235ns	0.213ns	0.825ns	0.00377	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	14.4μs	9.15ns	35.4ns	1.21	0.0216	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	11.8μs	17.9ns	69.2ns	0.0874	0	6.75 KB
#3167	`AllCycleSimpleBody`	net472	189ns	0.219ns	0.847ns	0.0676	0	425 B
#3167	`AllCycleSimpleBody`	netcoreapp3.1	242ns	0.45ns	1.62ns	0.00582	0	424 B
#3167	`AllCycleMoreComplexBody`	net472	184ns	0.213ns	0.826ns	0.0637	0	401 B
#3167	`AllCycleMoreComplexBody`	netcoreapp3.1	236ns	0.306ns	1.19ns	0.00555	0	400 B
#3167	`BodyExtractorSimpleBody`	net472	257ns	0.212ns	0.792ns	0.0573	0	361 B
#3167	`BodyExtractorSimpleBody`	netcoreapp3.1	232ns	0.234ns	0.877ns	0.0037	0	272 B
#3167	`BodyExtractorMoreComplexBody`	net472	14.4μs	7.16ns	26.8ns	1.21	0.0216	7.62 KB
#3167	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.7μs	16ns	59.8ns	0.0892	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	182μs	188ns	704ns	0.182	20.57 KB
#3167	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3167	`SendRequest`	netcoreapp3.1	184μs	312ns	1.21μs	0.273	20.58 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.83μs	1.08ns	4.06ns	0.15	0.000915	947 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.45μs	0.569ns	2.13ns	0.0123	0	936 B
#3167	`ExecuteNonQuery`	net472	1.91μs	1.02ns	3.95ns	0.15	0.000971	947 B
#3167	`ExecuteNonQuery`	netcoreapp3.1	1.47μs	0.806ns	3.12ns	0.0125	0	936 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.56μs	1.78ns	6.66ns	0.183	1.16 KB
master	`CallElasticsearch`	netcoreapp3.1	1.59μs	0.795ns	2.97ns	0.0143	1.1 KB
master	`CallElasticsearchAsync`	net472	2.82μs	3.96ns	14.3ns	0.205	1.29 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.69μs	0.436ns	1.57ns	0.016	1.22 KB
#3167	`CallElasticsearch`	net472	2.64μs	1.59ns	6.16ns	0.184	1.16 KB
#3167	`CallElasticsearch`	netcoreapp3.1	1.51μs	0.632ns	2.45ns	0.0144	1.1 KB
#3167	`CallElasticsearchAsync`	net472	2.68μs	1.99ns	7.45ns	0.204	1.29 KB
#3167	`CallElasticsearchAsync`	netcoreapp3.1	1.7μs	0.622ns	2.33ns	0.0163	1.22 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	2.84μs	8.68ns	32.5ns	0.225	1.42 KB
master	`ExecuteAsync`	netcoreapp3.1	1.69μs	3.49ns	13.5ns	0.0186	1.34 KB
#3167	`ExecuteAsync`	net472	2.72μs	6.98ns	27ns	0.226	1.42 KB
#3167	`ExecuteAsync`	netcoreapp3.1	1.68μs	2.27ns	8.78ns	0.0185	1.34 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	5.75μs	14.3ns	55.4ns	0.438	2.77 KB
master	`SendAsync`	netcoreapp3.1	3.62μs	7.02ns	26.3ns	0.0343	2.6 KB
#3167	`SendAsync`	net472	5.86μs	19.2ns	74.5ns	0.438	2.77 KB
#3167	`SendAsync`	netcoreapp3.1	3.68μs	7.03ns	27.2ns	0.035	2.6 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	2.25ns	8.43ns	0.288	1.81 KB
master	`EnrichedLog`	netcoreapp3.1	2.51μs	1.04ns	4.04ns	0.0252	1.85 KB
#3167	`EnrichedLog`	net472	3.04μs	2.71ns	9.78ns	0.287	1.81 KB
#3167	`EnrichedLog`	netcoreapp3.1	2.46μs	1.02ns	3.81ns	0.0246	1.85 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	181ns	702ns	0.686	0.229	4.65 KB
master	`EnrichedLog`	netcoreapp3.1	118μs	114ns	426ns	0	0	4.49 KB
#3167	`EnrichedLog`	net472	151μs	185ns	717ns	0.676	0.225	4.67 KB
#3167	`EnrichedLog`	netcoreapp3.1	116μs	123ns	478ns	0.0576	0	4.5 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.67μs	7.05ns	26.4ns	0.569	0.00282	3.59 KB
master	`EnrichedLog`	netcoreapp3.1	4.25μs	8.69ns	32.5ns	0.0522	0	3.91 KB
#3167	`EnrichedLog`	net472	5.68μs	12.9ns	50.1ns	0.567	0.00282	3.59 KB
#3167	`EnrichedLog`	netcoreapp3.1	4.46μs	10.8ns	40.6ns	0.0531	0	3.91 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.25μs	1.6ns	5.97ns	0.218	0	1.37 KB
master	`SendReceive`	netcoreapp3.1	1.74μs	0.793ns	2.97ns	0.0174	0	1.32 KB
#3167	`SendReceive`	net472	2.33μs	2.63ns	10.2ns	0.218	0.00117	1.37 KB
#3167	`SendReceive`	netcoreapp3.1	1.78μs	1.08ns	4.2ns	0.0178	0	1.32 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	4.98μs	3.47ns	13ns	0.353	2.23 KB
master	`EnrichedLog`	netcoreapp3.1	4.23μs	1.47ns	5.7ns	0.0233	1.8 KB
#3167	`EnrichedLog`	net472	5.03μs	2.24ns	8.67ns	0.353	2.23 KB
#3167	`EnrichedLog`	netcoreapp3.1	4.27μs	1.76ns	6.83ns	0.0236	1.8 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`StartFinishSpan`	net472	1.14μs	0.683ns	2.56ns	0.128	0	810 B
master	`StartFinishSpan`	netcoreapp3.1	921ns	0.327ns	1.22ns	0.0101	0	760 B
master	`StartFinishScope`	net472	1.41μs	0.824ns	3.19ns	0.141	0	891 B
master	`StartFinishScope`	netcoreapp3.1	1.11μs	4.28ns	16.6ns	0.0121	0	880 B
#3167	`StartFinishSpan`	net472	1.15μs	0.354ns	1.37ns	0.128	0	810 B
#3167	`StartFinishSpan`	netcoreapp3.1	917ns	0.296ns	1.11ns	0.0101	0	760 B
#3167	`StartFinishScope`	net472	1.46μs	0.962ns	3.73ns	0.141	0.000731	891 B
#3167	`StartFinishScope`	netcoreapp3.1	1.13μs	0.728ns	2.73ns	0.0124	0	880 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.5μs	0.94ns	3.64ns	0.141	891 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.16μs	1.65ns	6.18ns	0.012	880 B
#3167	`RunOnMethodBegin`	net472	1.55μs	1.89ns	7.09ns	0.141	891 B
#3167	`RunOnMethodBegin`	netcoreapp3.1	1.15μs	0.627ns	2.43ns	0.0119	880 B

andrewlock · 2022-09-12T14:44:49Z

Code Coverage Report 📊

✔️ Merging #3167 into master will will increase line coverage by 1%
✔️ Merging #3167 into master will will increase branch coverage by 1%
⛔ Merging #3167 into master will will increase complexity by 94

	master	#3167	Change
Lines	`17696` / `24369`	`18000` / `24521`
Lines %	`73%`	`73%`	`1%` ✔️
Branches	`10398` / `14968`	`10603` / `15054`
Branches %	`69%`	`70%`	`1%` ✔️
Complexity	`16169`	`16263`	`94` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3167	Change
Lines %	`73%`	`73%`	`1%` ✔️
Branches %	`69%`	`70%`	`1%` ✔️
Complexity	`16169`	`16263`	`94` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Agent.Transports.HttpStreamRequestFactory	`-12%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.Debugger.Sink.AgentBatchUploadApi	`-8%` ⛔	`0%` ✔️	`2` ⛔
Datadog.Trace.TracerManagerFactory	`3%` ✔️	`6%` ✔️	`2` ⛔
Datadog.Trace.Logging.DatadogSerilogLogger	`4%` ✔️	`6%` ✔️	`0` ✔️
Datadog.Trace.Ci.CIVisibility	`5%` ✔️	`6%` ✔️	`0` ✔️
Datadog.Trace.Propagators.SpanContextPropagator	`8%` ✔️	`6%` ✔️	`0` ✔️
Datadog.Trace.Processors.ObfuscatorTraceProcessor	`12%` ✔️	`15%` ✔️	`0` ✔️
Datadog.Trace.Agent.StatsAggregator	`12%` ✔️	`17%` ✔️	`-2` ✔️
Datadog.Trace.Telemetry.Transports.JsonTelemetryTransport	`12%` ✔️	`17%` ✔️	`0` ✔️
Datadog.Trace.Agent.DiscoveryService.DiscoveryService	`21%` ✔️	`46%` ✔️	`29` ⛔
...And 7 more

The following classes were added in #3167:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.Agent.DiscoveryService.AgentConfiguration	`100%`	`100%`	`1`
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.EnvironmentVariablesScrubber	`100%`	`100%`	`11`
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.ProcessStartCommon	`85%`	`80%`	`11`
Datadog.Trace.ClrProfiler.AutoInstrumentation.Process.ProcessStartIntegration	`80%`	`50%`	`3`
Datadog.Trace.Tagging.ProcessCommandStartTags	`100%`	`100%`	`1`
...And 1 more

View the full reports for further details:

This comment has been minimized.

Sign in to view

kevingosse added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:profiler Issues related to the continous-profiler labels Sep 9, 2022

kevingosse marked this pull request as ready for review September 9, 2022 07:39

kevingosse requested a review from a team as a code owner September 9, 2022 07:39

andrewlock approved these changes Sep 9, 2022

View reviewed changes

gleocadie approved these changes Sep 9, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

kevingosse force-pushed the kevin/profiler_dd_trace branch from 2a1d19a to d94acb0 Compare September 9, 2022 14:48

This comment has been minimized.

Sign in to view

zacharycmontoya reviewed Sep 9, 2022

View reviewed changes

kevingosse force-pushed the kevin/profiler_dd_trace branch from d94acb0 to 746f523 Compare September 12, 2022 09:19

kevingosse added 7 commits September 12, 2022 11:19

Add diagnostic checks for the profiler

9191dc8

Start adding tests

7912078

More tests

6a44933

Fix assertion

fd844fa

Fix paths

531ee10

Fix result of profiler tests

a6480e9

Fix module path

6c305fd

kevingosse and others added 6 commits September 12, 2022 11:19

Fix path on Alpine

85a358b

Fix path²

a96ef7c

Update tracer/src/Datadog.Trace.Tools.Runner/Checks/ProcessBasicCheck.cs

3b874fa

Co-authored-by: Andrew Lock <[email protected]>

Validate that LD_PRELOAD points to the right file

c9de3e7

Fix test case

cc19b5d

Fix test

1757dc4

kevingosse force-pushed the kevin/profiler_dd_trace branch from 746f523 to 1757dc4 Compare September 12, 2022 09:19

This comment has been minimized.

Sign in to view

kevingosse merged commit e867bec into master Sep 19, 2022

kevingosse deleted the kevin/profiler_dd_trace branch September 19, 2022 08:55

github-actions Bot added this to the vNext milestone Sep 19, 2022

	if (loaderModule == null)
	if (loaderModule == null && (nativeTracerModule?.StartsWith("Datadog.Tracer.Native", StringComparison.OrdinalIgnoreCase) ?? true))

Uh oh!

Conversation

kevingosse commented Sep 5, 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

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.

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gleocadie left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

zacharycmontoya Sep 9, 2022

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Sep 9, 2022

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

andrewlock commented Sep 12, 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

Raw results

Raw results

Uh oh!

andrewlock commented Sep 12, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kevingosse commented Sep 5, 2022 •

edited

Loading