Add hash helpers for data streams monitoring by andrewlock · Pull Request #3131 · DataDog/dd-trace-dotnet

andrewlock · 2022-08-26T12:39:13Z

Summary of changes

Add a BinaryPrimitivesHelper to poly-fill allocation-free reading/writing of little endian long
Add a HashHelper to encapsulate building the Node and Pathway hashes for data streams monitoring
Created simple strongly-typed-ids for the hashes to avoid accidental misuse

Reason for change

I need to calculate these hashes for data streams monitoring, and want to avoid allocation where possible (as this could be on the hot path for an app)

Implementation details

Hash implementations based on the go and java implementations in data streams monitoring.

Test coverage

Should be covered, took the tests from go and java repos where available, and compared the pollyfilled "efficient" versions to the allocatey versions

Other details

Required for data streams monitoring

zacharycmontoya · 2022-08-29T18:07:56Z

+public class BinaryPrimitivesHelperTests
+{
+    [Fact]
+    public void CanRoundTripValue()


Can you also write the following tests:

Generate a random ulong number (or I suppose a hard-coded number would work as well), zero out the lowest byte, write to the byte buffer using WriteUInt64LittleEndian(bytes, expected), assert that bytes[0] == 0

Generate a random 8-bit byte array (or I suppose a hard-coded array would work as well), zero out bytes[0], read from the byte buffer using ReadUInt64LittleEndian(bytes), assert that the lowest byte is 0

I added this in 3e94261, but I don't think it actually satisfies your concerns, as we never test the big-endian code! 🙂

Do you think you could introduce a branch in the new test, one for little-endian and one for big-endian and inside each branch assert that the "lowest" byte is 0? I'm not sure when it would ever get hit, but if we were to run it on a big-endian system and it failed that would be a red flag

Do you think you could introduce a branch in the new test, one for little-endian and one for big-endian and inside each branch assert that the "lowest" byte is 0?

Not really 😄 There's no way to hit the "big-endian" branch without running on a big-endian machine. And if we did, then this test would be testing the big-endian code path (instead of the little endian) so that would be your red flag!

pierotibou · 2022-09-02T11:09:58Z

+
+namespace Datadog.Trace.Tests.DataStreamsMonitoring;
+
+public class HashHelperTests


Not necessary for this PR obv, more to start the discussion, those tests could be good candidates for property based testing, wdyt?

Yeah, could do

For calculating the NodeHash and PathwayHash required for DataStreams monitoring

Update HashHelper to use it

andrewlock · 2022-09-06T13:30:12Z

Benchmarks Report 🐌

Benchmarks for #3131 compared to master:

2 benchmarks are faster, with geometric mean 1.158
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	723μs	755ns	2.83μs	0.359	3.18 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	480μs	1.95μs	7.31μs	0	2.58 KB
#3131	`WriteAndFlushEnrichedTraces`	net472	722μs	620ns	2.4μs	0.361	3.18 KB
#3131	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	464μs	233ns	901ns	0	2.58 KB

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

Faster 🎉 in #3131

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.AppSecBodyBenchmark.AllCycleMoreComplexBody‑net472	1.203	232.64	193.33

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	185ns	0.0773ns	0.3ns	0.0676	0	425 B
master	`AllCycleSimpleBody`	netcoreapp3.1	237ns	0.338ns	1.31ns	0.00585	0	424 B
master	`AllCycleMoreComplexBody`	net472	234ns	1.06ns	4.11ns	0.0637	0	401 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	247ns	0.28ns	1.09ns	0.00549	0	400 B
master	`BodyExtractorSimpleBody`	net472	262ns	0.175ns	0.655ns	0.0574	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	219ns	0.322ns	1.2ns	0.00376	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	14.5μs	8.62ns	32.3ns	1.21	0.0217	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12μs	17.6ns	65.9ns	0.0897	0	6.75 KB
#3131	`AllCycleSimpleBody`	net472	192ns	0.143ns	0.554ns	0.0676	0	425 B
#3131	`AllCycleSimpleBody`	netcoreapp3.1	239ns	0.21ns	0.814ns	0.00585	0	424 B
#3131	`AllCycleMoreComplexBody`	net472	193ns	0.149ns	0.556ns	0.0638	0	401 B
#3131	`AllCycleMoreComplexBody`	netcoreapp3.1	240ns	0.372ns	1.39ns	0.00551	0	400 B
#3131	`BodyExtractorSimpleBody`	net472	267ns	0.206ns	0.771ns	0.0573	0	361 B
#3131	`BodyExtractorSimpleBody`	netcoreapp3.1	236ns	0.267ns	0.963ns	0.00365	0	272 B
#3131	`BodyExtractorMoreComplexBody`	net472	14.4μs	16.7ns	64.8ns	1.21	0.0215	7.62 KB
#3131	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12μs	14.4ns	55.9ns	0.0891	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	181μs	114ns	427ns	0.271	20.57 KB
#3131	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3131	`SendRequest`	netcoreapp3.1	184μs	128ns	460ns	0.184	20.57 KB

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

Faster 🎉 in #3131

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑netcoreapp3.1	1.115	1,612.13	1,445.84

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net472	1.8μs	0.753ns	2.71ns	0.15	0.000899	947 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.61μs	0.786ns	3.04ns	0.0121	0	936 B
#3131	`ExecuteNonQuery`	net472	1.8μs	0.544ns	2.04ns	0.15	0.000903	947 B
#3131	`ExecuteNonQuery`	netcoreapp3.1	1.45μs	1.39ns	5.39ns	0.0123	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.53μs	1.19ns	4.62ns	0.183	1.16 KB
master	`CallElasticsearch`	netcoreapp3.1	1.62μs	0.771ns	2.89ns	0.0146	1.1 KB
master	`CallElasticsearchAsync`	net472	2.64μs	0.732ns	2.74ns	0.204	1.29 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.64μs	2.07ns	7.74ns	0.0165	1.22 KB
#3131	`CallElasticsearch`	net472	2.58μs	0.83ns	3.11ns	0.183	1.16 KB
#3131	`CallElasticsearch`	netcoreapp3.1	1.54μs	0.807ns	3.02ns	0.0146	1.1 KB
#3131	`CallElasticsearchAsync`	net472	2.61μs	0.845ns	3.27ns	0.204	1.29 KB
#3131	`CallElasticsearchAsync`	netcoreapp3.1	1.65μs	0.795ns	3.08ns	0.0166	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.65μs	5.89ns	22.8ns	0.226	1.42 KB
master	`ExecuteAsync`	netcoreapp3.1	1.68μs	4.42ns	17.1ns	0.0185	1.34 KB
#3131	`ExecuteAsync`	net472	2.54μs	3.25ns	12.2ns	0.225	1.42 KB
#3131	`ExecuteAsync`	netcoreapp3.1	1.72μs	3.45ns	13.4ns	0.0181	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.64μs	8.3ns	32.2ns	0.439	2.77 KB
master	`SendAsync`	netcoreapp3.1	3.62μs	10.3ns	39.7ns	0.0348	2.6 KB
#3131	`SendAsync`	net472	5.56μs	11.7ns	45.1ns	0.439	2.77 KB
#3131	`SendAsync`	netcoreapp3.1	3.6μs	6ns	21.6ns	0.0338	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.23μs	3.78ns	14.6ns	0.287	1.81 KB
master	`EnrichedLog`	netcoreapp3.1	2.53μs	1.19ns	4.43ns	0.0253	1.85 KB
#3131	`EnrichedLog`	net472	3.18μs	3.01ns	11.6ns	0.287	1.81 KB
#3131	`EnrichedLog`	netcoreapp3.1	2.61μs	1.62ns	6.28ns	0.0247	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	150μs	109ns	392ns	0.672	0.224	4.65 KB
master	`EnrichedLog`	netcoreapp3.1	117μs	213ns	826ns	0.0595	0	4.49 KB
#3131	`EnrichedLog`	net472	151μs	68.5ns	237ns	0.683	0.228	4.65 KB
#3131	`EnrichedLog`	netcoreapp3.1	118μs	238ns	922ns	0.0598	0	4.49 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.62μs	7.97ns	29.8ns	0.568	0.00278	3.59 KB
master	`EnrichedLog`	netcoreapp3.1	4.33μs	9.47ns	36.7ns	0.0535	0	3.91 KB
#3131	`EnrichedLog`	net472	5.54μs	9.18ns	35.5ns	0.569	0.00279	3.59 KB
#3131	`EnrichedLog`	netcoreapp3.1	4.4μs	7.1ns	26.6ns	0.053	0	3.91 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.32μs	4.05ns	15.2ns	0.217	1.37 KB
master	`SendReceive`	netcoreapp3.1	1.85μs	1.37ns	5.12ns	0.0176	1.32 KB
#3131	`SendReceive`	net472	2.35μs	3.38ns	13.1ns	0.218	1.37 KB
#3131	`SendReceive`	netcoreapp3.1	1.87μs	0.884ns	3.43ns	0.0178	1.32 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	5.02μs	1.33ns	5.15ns	0.352	2.23 KB
master	`EnrichedLog`	netcoreapp3.1	4.23μs	1.3ns	5.05ns	0.0233	1.8 KB
#3131	`EnrichedLog`	net472	4.96μs	2.12ns	7.93ns	0.354	2.23 KB
#3131	`EnrichedLog`	netcoreapp3.1	4.16μs	1.45ns	5.41ns	0.0229	1.8 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	1.15μs	0.556ns	2.15ns	0.129	810 B
master	`StartFinishSpan`	netcoreapp3.1	935ns	3.25ns	12.1ns	0.0106	760 B
master	`StartFinishScope`	net472	1.45μs	0.761ns	2.95ns	0.141	891 B
master	`StartFinishScope`	netcoreapp3.1	1.06μs	0.62ns	2.32ns	0.0123	880 B
#3131	`StartFinishSpan`	net472	1.16μs	0.472ns	1.83ns	0.128	810 B
#3131	`StartFinishSpan`	netcoreapp3.1	938ns	0.26ns	0.936ns	0.0103	760 B
#3131	`StartFinishScope`	net472	1.35μs	0.684ns	2.65ns	0.141	891 B
#3131	`StartFinishScope`	netcoreapp3.1	1.03μs	0.245ns	0.917ns	0.0119	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.828ns	3.1ns	0.141	891 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.12μs	0.342ns	1.28ns	0.0118	880 B
#3131	`RunOnMethodBegin`	net472	1.55μs	0.36ns	1.4ns	0.141	891 B
#3131	`RunOnMethodBegin`	netcoreapp3.1	1.2μs	0.723ns	2.7ns	0.012	880 B

andrewlock · 2022-09-06T13:56:57Z

Code Coverage Report 📊

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

	master	#3131	Change
Lines	`17633` / `24209`	`17707` / `24277`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`10433` / `14900`	`10458` / `14914`
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`16087`	`16108`	`21` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3131	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`16087`	`16108`	`21` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Debugger.ProbeStatuses.ProbeStatusPoller	`0%` ✔️	`-6%` ⛔	`0` ✔️
Datadog.Trace.Ci.CIVisibility	`5%` ✔️	`6%` ✔️	`0` ✔️

The following classes were added in #3131:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.DataStreamsMonitoring.Hashes.HashHelper	`100%`	`100%`	`8`
Datadog.Trace.DataStreamsMonitoring.Hashes.NodeHash	`100%`	`100%`	`1`
Datadog.Trace.DataStreamsMonitoring.Hashes.NodeHashBase	`100%`	`100%`	`1`
Datadog.Trace.DataStreamsMonitoring.Hashes.PathwayHash	`100%`	`100%`	`1`
Datadog.Trace.DataStreamsMonitoring.Utils.BinaryPrimitivesHelper	`56%`	`50%`	`10`

View the full reports for further details:

andrewlock requested a review from a team as a code owner August 26, 2022 12:39