Add wildcard support to DD_TRACE_METHODS by zacharycmontoya · Pull Request #2628 · DataDog/dd-trace-dotnet

zacharycmontoya · 2022-04-01T01:18:37Z

Summary of changes

In the first implementation of DD_TRACE_METHODS for the .NET Tracer, the developer needed to specify each method in a type to be instrumented. This feature adds the ability to set DD_TRACE_METHODS=TypeName[*] to instrument all methods within a specified type, except the following special methods:

Constructors (.ctor)
Static constructor (.cctor)
Equals
Finalize
GetHashCode
ToString
Property getters/setters (get_XXX/set_XXX)

If you want to instrument a special method via DD_TRACE_METHODS you must explicitly include it in the configuration. For example, to instrument all methods within a type named Type1, all of the special methods listed above, and the getter/setter for a property named Name, the configuration would be DD_TRACE_METHODS=Type1[*,.ctor,.cctor,Equals,GetHashCode,ToString,get_Name,set_Name]

Reason for change

It can be error-prone to configure a large number of methods within one type, so a wildcard character * lowers the burden for configuration.

Implementation details

The implementation has two steps:

Parsing the DD_TRACE_METHODS configuration: There is no special parsing to handle the * method specifier, it just generates an IntegrationDefinition object like all other strings.
In the cross-product RejitPreprocessor<RejitRequestDefinition>::ProcessTypeDefForRejit, allow the method name * to enumerate all methods for a typeDef (metadataImport->EnumMethods) instead of enumerating the methods with a specific method (metadataImport->EnumMethodsWithName). When processing the methods from the * method search, do string comparisons and string finds to exclude the special methods above.

Test coverage

Unit tests: Added another test case for the DD_TRACE_METHODS parsing logic to ensure that the wildcard modifier generates an IntegrationDefinition.
Integration tests: Existing tests have been modified to use the new * wildcard and a special method (property getter) is added and tested for span-ification.

Other details

N/A

andrewlock

LGTM, though c++ is not my forte!

andrewlock · 2022-04-04T11:32:47Z


 const size_t kPublicKeySize = 8;
 const shared::WSTRING tracemethodintegration_assemblyname = WStr("#TraceMethodFeature");
+const std::unordered_set<shared::WSTRING> tracemethodintegration_wildcard_ignored_methods(


niche, but wondering if we should add Finalize to this list? This has implications if they do have a method called Finalize which isn't a finalizer though, so I'm not sure whether it's a good idea or not.

In theory we can tell the difference by checking if Finalize() overrides object.Finalize(), but even without that, I agree we should ignore it by default.

In the latest commits I added Finalize to the list of method names to ignore by default, including ones that aren't finalizers. I don't think this will be an issue for now and we can revisit this later if it becomes an issue

andrewlock · 2022-04-04T11:39:26Z

 {
    internal class TestType
    {
+        static TestType() { }


If you do decide to add Finalize as an ignored method, we should add one here too (and potentially a non-finalizer called Finalize?

…methods: - .cctor - .ctor - Equals - GetHashCode - ToString - All set_* and get_* Unfortunately, if you specify * in your method list, it will ignore your other entries and go into wildcard mode, which stops you from instrumenting getters, setters, etc. The next step is to allow wildcard mode AND allow you to specify special methods that you really do want to instrument

…hods, even if there is a wildcard

…dd logic to sample to try to trigger object finalizer

andrewlock · 2022-04-06T18:23:50Z

Code Coverage Report 📊

✔️ Merging #2628 into master will not change line coverage
✔️ Merging #2628 into master will not change branch coverage
✔️ Merging #2628 into master will will decrease complexity by 2

	master	#2628	Change
Lines	`13462` / `18467`	`13490` / `18464`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`7782` / `11018`	`7804` / `11017`
Branches %	`71%`	`71%`	`0%` ✔️
Complexity	`12309`	`12307`	`-2` ✔️

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#2628	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`71%`	`71%`	`0%` ✔️
Complexity	`12309`	`12307`	`-2` ✔️

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Agent.StreamFactories.TcpStreamFactory	`-90%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.HttpOverStreams.TraceAgentHttpHeaderHelper	`-20%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.Agent.TracesTransportStrategy	`-4%` ⚠️	`15%` ✔️	`-1` ✔️

View the full reports for further details:

andrewlock · 2022-04-06T21:58:51Z

Benchmarks Report 🐌

Benchmarks for #2628 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	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	950μs	4.24μs	15.3μs	3.16 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	733μs	3.1μs	11.2μs	2.57 KB
#2628	`WriteAndFlushEnrichedTraces`	net472	971μs	3.93μs	16.7μs	3.16 KB
#2628	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	777μs	4.59μs	45.9μ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	315ns	1.46ns	5.48ns	0.0519	0	329 B
master	`AllCycleSimpleBody`	netcoreapp3.1	387ns	1.61ns	6.22ns	0.00444	0	328 B
master	`AllCycleMoreComplexBody`	net472	307ns	1.45ns	5.62ns	0.0481	0	305 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	375ns	1.37ns	4.93ns	0.00411	0	304 B
master	`BodyExtractorSimpleBody`	net472	454ns	1.98ns	7.41ns	0.0569	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	463ns	2.06ns	8.23ns	0.0039	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	25.5μs	110ns	605ns	1.19	0.0131	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	21.5μs	81.4ns	305ns	0.0971	0	6.75 KB
#2628	`AllCycleSimpleBody`	net472	313ns	1.79ns	12.4ns	0.052	0	329 B
#2628	`AllCycleSimpleBody`	netcoreapp3.1	380ns	1.5ns	5.39ns	0.00461	0	328 B
#2628	`AllCycleMoreComplexBody`	net472	299ns	1.08ns	4.03ns	0.0482	0	305 B
#2628	`AllCycleMoreComplexBody`	netcoreapp3.1	375ns	1.46ns	5.47ns	0.00423	0	304 B
#2628	`BodyExtractorSimpleBody`	net472	440ns	1.88ns	7.52ns	0.0569	0	361 B
#2628	`BodyExtractorSimpleBody`	netcoreapp3.1	447ns	1.45ns	5.61ns	0.00379	0	272 B
#2628	`BodyExtractorMoreComplexBody`	net472	25.1μs	112ns	433ns	1.19	0.0124	7.62 KB
#2628	`BodyExtractorMoreComplexBody`	netcoreapp3.1	21.7μs	99.9ns	387ns	0.096	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	350μs	1.61μs	6.42μs	0.17	19.74 KB
#2628	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#2628	`SendRequest`	netcoreapp3.1	338μs	1.31μs	5.09μs	0.168	19.74 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.93μs	11ns	84.1ns	0.0934	0.00103	594 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.56μs	5.48ns	20.5ns	0.00868	0	632 B
#2628	`ExecuteNonQuery`	net472	1.92μs	10.9ns	84.1ns	0.0934	0.000963	594 B
#2628	`ExecuteNonQuery`	netcoreapp3.1	1.55μs	6.08ns	22.8ns	0.00857	0	632 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net472	3.03μs	14.5ns	59.8ns	0.127	0	803 B
master	`CallElasticsearch`	netcoreapp3.1	1.79μs	10ns	75.1ns	0.0105	0	792 B
master	`CallElasticsearchAsync`	net472	3.05μs	15.3ns	68.2ns	0.148	0.00151	939 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.91μs	9.16ns	37.8ns	0.0132	0	912 B
#2628	`CallElasticsearch`	net472	2.86μs	15.6ns	90.9ns	0.125	0	802 B
#2628	`CallElasticsearch`	netcoreapp3.1	1.74μs	6.14ns	24.5ns	0.0114	0	792 B
#2628	`CallElasticsearchAsync`	net472	3.05μs	15.6ns	74.9ns	0.149	0.00149	939 B
#2628	`CallElasticsearchAsync`	netcoreapp3.1	1.89μs	7.29ns	27.3ns	0.0123	0	912 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteAsync`	net472	3.41μs	16.9ns	71.8ns	0.167	0.00167	1.06 KB
master	`ExecuteAsync`	netcoreapp3.1	2.13μs	9.34ns	36.2ns	0.0147	0	1.03 KB
#2628	`ExecuteAsync`	net472	3.2μs	12.2ns	47.4ns	0.167	0	1.06 KB
#2628	`ExecuteAsync`	netcoreapp3.1	2.14μs	7.02ns	27.2ns	0.0137	0	1.03 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	7.24μs	37.2ns	167ns	0.361	2.28 KB
master	`SendAsync`	netcoreapp3.1	5.15μs	16.4ns	63.7ns	0.0303	2.21 KB
#2628	`SendAsync`	net472	7.05μs	17.7ns	68.5ns	0.36	2.28 KB
#2628	`SendAsync`	netcoreapp3.1	5.38μs	27.9ns	139ns	0.0303	2.21 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.73μs	21.8ns	192ns	0.227	1.45 KB
master	`EnrichedLog`	netcoreapp3.1	3.15μs	11.8ns	44.1ns	0.0207	1.53 KB
#2628	`EnrichedLog`	net472	3.85μs	21.3ns	128ns	0.226	1.45 KB
#2628	`EnrichedLog`	netcoreapp3.1	3.24μs	17.4ns	107ns	0.0218	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.28μs	4.97μs	0.314	0.157	4.33 KB
master	`EnrichedLog`	netcoreapp3.1	244μs	1.13μs	4.37μs	0	0	4.21 KB
#2628	`EnrichedLog`	net472	300μs	916ns	3.55μs	0.445	0.148	4.33 KB
#2628	`EnrichedLog`	netcoreapp3.1	241μs	900ns	3.6μ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	8.21μs	38ns	142ns	0.504	3.23 KB
master	`EnrichedLog`	netcoreapp3.1	6.79μs	30.9ns	164ns	0.0499	3.6 KB
#2628	`EnrichedLog`	net472	8.18μs	27.1ns	101ns	0.505	3.23 KB
#2628	`EnrichedLog`	netcoreapp3.1	6.63μs	32.6ns	142ns	0.049	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.39μs	10.9ns	40.8ns	0.16	0.00121	1.01 KB
master	`SendReceive`	netcoreapp3.1	2.09μs	9.8ns	38ns	0.0139	0	1.01 KB
#2628	`SendReceive`	net472	2.53μs	10.5ns	39.4ns	0.16	0.00123	1.01 KB
#2628	`SendReceive`	netcoreapp3.1	2.15μs	4.8ns	17.9ns	0.0138	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.44μs	20.4ns	79.1ns	0.292	1.87 KB
master	`EnrichedLog`	netcoreapp3.1	5.23μs	12.8ns	47.9ns	0.021	1.49 KB
#2628	`EnrichedLog`	net472	6.27μs	24.4ns	94.5ns	0.292	1.87 KB
#2628	`EnrichedLog`	netcoreapp3.1	5.36μs	16.9ns	63.3ns	0.0189	1.49 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	964ns	5.14ns	27.2ns	0.0716	457 B
master	`StartFinishSpan`	netcoreapp3.1	991ns	4.58ns	17.7ns	0.00611	456 B
master	`StartFinishScope`	net472	1.24μs	6.9ns	42.5ns	0.0841	538 B
master	`StartFinishScope`	netcoreapp3.1	1.13μs	6.58ns	57.4ns	0.0079	576 B
#2628	`StartFinishSpan`	net472	927ns	3.02ns	11.7ns	0.072	457 B
#2628	`StartFinishSpan`	netcoreapp3.1	917ns	2.49ns	8.63ns	0.00639	456 B
#2628	`StartFinishScope`	net472	1.17μs	6.36ns	33.7ns	0.0843	538 B
#2628	`StartFinishScope`	netcoreapp3.1	1.12μs	6.49ns	55.8ns	0.00815	576 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.38μs	6.84ns	29ns	0.0838	538 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.24μs	6.47ns	42.4ns	0.00798	576 B
#2628	`RunOnMethodBegin`	net472	1.42μs	8.16ns	64.3ns	0.0844	538 B
#2628	`RunOnMethodBegin`	netcoreapp3.1	1.29μs	11.7ns	114ns	0.00776	576 B

zacharycmontoya added the area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) label Apr 1, 2022

zacharycmontoya requested review from GreenMatan and tonyredondo April 1, 2022 01:18

zacharycmontoya self-assigned this Apr 1, 2022

zacharycmontoya requested a review from a team as a code owner April 1, 2022 01:18

zacharycmontoya commented Apr 1, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace.ClrProfiler.Native/rejit_preprocessor.cpp

This comment has been minimized.

Sign in to view

andrewlock approved these changes Apr 4, 2022

View reviewed changes

lucaspimentel approved these changes Apr 4, 2022

View reviewed changes

zacharycmontoya added 6 commits April 5, 2022 14:36

Set up regression test to assert the "*" wildcard

aecd3cb

Allow specified methods in DD_TRACE_METHODS to instrument special met…

e3943ab

…hods, even if there is a wildcard

Cleanup and optimizations

e426f0e

Exclude any methods named Finalize from the wildcard method search. A…

fed87b7

…dd logic to sample to try to trigger object finalizer

Add warning log statement for wildcard search when it bails out

5cc9eba

zacharycmontoya force-pushed the zach/trace-methods-wildcard branch from 26780a7 to 5cc9eba Compare April 6, 2022 17:14

zacharycmontoya merged commit 4a7d43b into master Apr 7, 2022

zacharycmontoya deleted the zach/trace-methods-wildcard branch April 7, 2022 16:45

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

zacharycmontoya mentioned this pull request Jul 6, 2022

Instrument custom methods through configuration (DD_TRACE_METHODS) #2543

Merged

2 tasks

Uh oh!

Conversation

zacharycmontoya commented Apr 1, 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!

Uh oh!

This comment has been minimized.

This comment has been minimized.

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

lucaspimentel Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

andrewlock Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Apr 6, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

andrewlock commented Apr 6, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zacharycmontoya commented Apr 1, 2022 •

edited

Loading