Add `VarEncodingHelper` for use in Data Streams Monitoring by andrewlock · Pull Request #3129 · DataDog/dd-trace-dotnet

andrewlock · 2022-08-26T12:24:05Z

Summary of changes

Adds VarEncodingHelper for use in Data Streams Monitoring

Reason for change

This is a prerequisite for the ongoing data streams monitoring work. This implementation (and the tests) are based on the version in sketches-go and sketches-java libraries. We can potentially move this to the sketches-dotnet library and re-vendor, but as it's unused in there, seemed it's easier to add it in here for now

Implementation details

Copied the java implementation mostly, reused the existing NumberOfLeadingZerosLong() implementation added to sketches-dotnet by @kevingosse

Test coverage

Copy-pasted from java/go to ensure we're compatible

Other details

This is a prerequisite for data streams monitoring

dromanol

LGTM 👌

bouwkast

Looks good to me, I have a few nits though on what I think are doc comment copy/paste errors

Also, I see in the Java/Go versions there is a version that uses double do we need that too?

bouwkast · 2022-08-29T14:54:04Z

+#endif
+
+    /// <summary>
+    /// Serializes 64-bit signed integers using zig-zag encoding,


nit, but I think this should say Serializes 32-bit signed integers

bouwkast · 2022-08-29T14:54:10Z

+        => WriteVarLongZigZag(bytes, offset, value);
+
+    /// <summary>
+    /// Serializes 64-bit signed integers using zig-zag encoding,


nit, but I think this should say Serializes 32-bit signed integers

bouwkast · 2022-08-29T14:54:17Z

+
+#if NETCOREAPP3_1_OR_GREATER
+    /// <summary>
+    /// Serializes 64-bit signed integers using zig-zag encoding,


nit, but I think this should say Serializes 32-bit signed integers

bouwkast · 2022-08-29T15:21:02Z

+#endif
+
+    /// <summary>
+    /// Deserializes 64-bit unsigned integers that have been encoded using


nit, but I think should be Deserializes 32-bit unsigned integers?

bouwkast · 2022-08-29T15:21:08Z

+
+#if NETCOREAPP3_1_OR_GREATER
+    /// <summary>
+    /// Deserializes 64-bit unsigned integers that have been encoded using


nit, but I think should be Deserializes 32-bit unsigned integers?

bouwkast · 2022-08-29T15:48:03Z

+    /// <param name="offset">The offset within the array to start writing</param>
+    /// <param name="bytesRead">The number of bytes read when successfully decoded</param>
+    /// <returns>The decoded value, or null if decoding failed</returns>
+    public static long? ReadVarIntZigZag(byte[] bytes, int offset, out int bytesRead)


Is this supposed to be a long? Seems like it should be an int if it gets casted to an int, unsure though if I'm missing something about this though.

Same question as the Span version of this as well.

Yep, you're right, good 👀, thanks!

Based on the version in go and Java dd-sketch libraries. We can move this to the dd-sketch-dotnet library, but for now it's easier to add it in here

andrewlock · 2022-08-31T09:58:35Z

Benchmarks Report 🐌

Benchmarks for #3129 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	712μs	241ns	932ns	0.374	3.18 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	451μs	177ns	662ns	0	2.58 KB
#3129	`WriteAndFlushEnrichedTraces`	net472	739μs	386ns	1.49μs	0.359	3.18 KB
#3129	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	456μs	164ns	635ns	0	2.59 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	198ns	0.881ns	3.41ns	0.0675	0	425 B
master	`AllCycleSimpleBody`	netcoreapp3.1	248ns	0.762ns	2.95ns	0.0058	0	424 B
master	`AllCycleMoreComplexBody`	net472	193ns	0.788ns	3.05ns	0.0637	0	401 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	241ns	0.848ns	3.29ns	0.00539	0	400 B
master	`BodyExtractorSimpleBody`	net472	274ns	1.26ns	4.89ns	0.0574	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	242ns	0.776ns	3.01ns	0.00366	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	14.7μs	53.5ns	207ns	1.2	0.0219	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.6μs	55.6ns	215ns	0.0923	0	6.75 KB
#3129	`AllCycleSimpleBody`	net472	189ns	0.637ns	2.47ns	0.0676	0	425 B
#3129	`AllCycleSimpleBody`	netcoreapp3.1	243ns	0.449ns	1.74ns	0.00585	0	424 B
#3129	`AllCycleMoreComplexBody`	net472	189ns	0.648ns	2.51ns	0.0637	0	401 B
#3129	`AllCycleMoreComplexBody`	netcoreapp3.1	244ns	0.515ns	1.99ns	0.00541	0	400 B
#3129	`BodyExtractorSimpleBody`	net472	260ns	1.07ns	4.13ns	0.0574	0	361 B
#3129	`BodyExtractorSimpleBody`	netcoreapp3.1	228ns	0.594ns	2.3ns	0.00375	0	272 B
#3129	`BodyExtractorMoreComplexBody`	net472	14.9μs	53.4ns	207ns	1.21	0.0149	7.62 KB
#3129	`BodyExtractorMoreComplexBody`	netcoreapp3.1	12.9μs	47ns	182ns	0.0917	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	182ns	705ns	0.271	20.57 KB
#3129	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#3129	`SendRequest`	netcoreapp3.1	182μs	129ns	499ns	0.275	20.57 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	0.401ns	1.5ns	0.15	0.000916	947 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.45μs	0.768ns	2.98ns	0.0123	0	936 B
#3129	`ExecuteNonQuery`	net472	1.84μs	0.746ns	2.89ns	0.15	0.000922	947 B
#3129	`ExecuteNonQuery`	netcoreapp3.1	1.51μs	0.534ns	2ns	0.0121	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.49μs	1.05ns	3.94ns	0.184	1.16 KB
master	`CallElasticsearch`	netcoreapp3.1	1.49μs	0.428ns	1.6ns	0.0149	1.1 KB
master	`CallElasticsearchAsync`	net472	2.67μs	0.832ns	3.22ns	0.205	1.29 KB
master	`CallElasticsearchAsync`	netcoreapp3.1	1.62μs	0.442ns	1.65ns	0.0162	1.22 KB
#3129	`CallElasticsearch`	net472	2.5μs	0.42ns	1.57ns	0.183	1.16 KB
#3129	`CallElasticsearch`	netcoreapp3.1	1.55μs	0.519ns	1.94ns	0.0146	1.1 KB
#3129	`CallElasticsearchAsync`	net472	2.56μs	0.909ns	3.52ns	0.204	1.29 KB
#3129	`CallElasticsearchAsync`	netcoreapp3.1	1.59μs	0.583ns	2.02ns	0.0167	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	4.98ns	19.3ns	0.224	1.41 KB
master	`ExecuteAsync`	netcoreapp3.1	1.62μs	2.5ns	9.67ns	0.0181	1.34 KB
#3129	`ExecuteAsync`	net472	2.52μs	8.6ns	32.2ns	0.224	1.41 KB
#3129	`ExecuteAsync`	netcoreapp3.1	1.67μs	4.17ns	16.2ns	0.0182	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.66μs	8.67ns	32.4ns	0.439	2.77 KB
master	`SendAsync`	netcoreapp3.1	3.57μs	7.69ns	29.8ns	0.0348	2.6 KB
#3129	`SendAsync`	net472	5.63μs	14.8ns	57.4ns	0.439	2.77 KB
#3129	`SendAsync`	netcoreapp3.1	3.55μs	7.83ns	29.3ns	0.0353	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.21μs	2.06ns	7.72ns	0.287	1.81 KB
master	`EnrichedLog`	netcoreapp3.1	2.46μs	1.34ns	4.83ns	0.0245	1.85 KB
#3129	`EnrichedLog`	net472	3.2μs	3.41ns	13.2ns	0.287	1.81 KB
#3129	`EnrichedLog`	netcoreapp3.1	2.5μs	0.94ns	3.52ns	0.025	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	149μs	91.4ns	354ns	0.671	0.224	4.65 KB
master	`EnrichedLog`	netcoreapp3.1	115μs	124ns	481ns	0	0	4.49 KB
#3129	`EnrichedLog`	net472	149μs	55.9ns	201ns	0.677	0.226	4.65 KB
#3129	`EnrichedLog`	netcoreapp3.1	114μs	75.3ns	291ns	0.0572	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.6μs	12.4ns	47.9ns	0.569	0.00279	3.59 KB
master	`EnrichedLog`	netcoreapp3.1	4.37μs	12.8ns	49.6ns	0.0541	0	3.91 KB
#3129	`EnrichedLog`	net472	5.51μs	8.77ns	34ns	0.569	0.0028	3.59 KB
#3129	`EnrichedLog`	netcoreapp3.1	4.25μs	7.07ns	27.4ns	0.0535	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.31μs	1.13ns	4.21ns	0.218	1.37 KB
master	`SendReceive`	netcoreapp3.1	1.82μs	0.519ns	1.94ns	0.0182	1.32 KB
#3129	`SendReceive`	net472	2.33μs	1.14ns	4.43ns	0.218	1.37 KB
#3129	`SendReceive`	netcoreapp3.1	1.81μs	0.616ns	2.3ns	0.0181	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.04μs	1.76ns	6.83ns	0.353	2.23 KB
master	`EnrichedLog`	netcoreapp3.1	4.28μs	1.83ns	6.83ns	0.0234	1.8 KB
#3129	`EnrichedLog`	net472	5.15μs	2.1ns	8.14ns	0.352	2.23 KB
#3129	`EnrichedLog`	netcoreapp3.1	4.33μs	1.12ns	4.18ns	0.0238	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.11μs	0.332ns	1.28ns	0.128	810 B
master	`StartFinishSpan`	netcoreapp3.1	893ns	0.371ns	1.39ns	0.0103	760 B
master	`StartFinishScope`	net472	1.35μs	0.91ns	3.4ns	0.141	891 B
master	`StartFinishScope`	netcoreapp3.1	1.06μs	1.13ns	4.39ns	0.0121	880 B
#3129	`StartFinishSpan`	net472	1.17μs	0.381ns	1.47ns	0.129	810 B
#3129	`StartFinishSpan`	netcoreapp3.1	928ns	0.267ns	0.999ns	0.0102	760 B
#3129	`StartFinishScope`	net472	1.38μs	0.784ns	2.93ns	0.141	891 B
#3129	`StartFinishScope`	netcoreapp3.1	1.11μs	0.414ns	1.55ns	0.0117	880 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.48μs	0.658ns	2.55ns	0.141	891 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.17μs	0.726ns	2.72ns	0.0118	880 B
#3129	`RunOnMethodBegin`	net472	1.49μs	0.625ns	2.34ns	0.141	891 B
#3129	`RunOnMethodBegin`	netcoreapp3.1	1.16μs	0.641ns	2.48ns	0.0121	880 B

andrewlock · 2022-08-31T10:21:25Z

Code Coverage Report 📊

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

	master	#3129	Change
Lines	`17259` / `23585`	`17419` / `23684`
Lines %	`73%`	`74%`	`0%` ✔️
Branches	`10242` / `14548`	`10324` / `14590`
Branches %	`70%`	`71%`	`0%` ✔️
Complexity	`15670`	`15735`	`65` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#3129	Change
Lines %	`73%`	`74%`	`0%` ✔️
Branches %	`70%`	`71%`	`0%` ✔️
Complexity	`15670`	`15735`	`65` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.Ci.CIVisibility	`5%` ✔️	`6%` ✔️	`0` ✔️

The following classes were added in #3129:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.DataStreamsMonitoring.VarEncodingHelper	`99%`	`100%`	`65`

View the full reports for further details:

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

This comment has been minimized.

Sign in to view

andrewlock added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Aug 26, 2022

This comment has been minimized.

Sign in to view

dromanol approved these changes Aug 29, 2022

View reviewed changes

bouwkast approved these changes Aug 29, 2022

View reviewed changes

andrewlock added 3 commits August 31, 2022 10:02

Add VarEncodingHelper

1142b67

Based on the version in go and Java dd-sketch libraries. We can move this to the dd-sketch-dotnet library, but for now it's easier to add it in here

PR feedback

d541da8

Add missing tests for coverage

706d135

andrewlock force-pushed the andrew/add-var-encoding-helper branch from b0f35f8 to 706d135 Compare August 31, 2022 09:20

andrewlock merged commit ae8975e into master Aug 31, 2022

andrewlock deleted the andrew/add-var-encoding-helper branch August 31, 2022 11:15

github-actions Bot added this to the vNext milestone Aug 31, 2022

andrewlock added the area:data-streams-monitoring label Aug 31, 2022

Uh oh!

Conversation

andrewlock commented Aug 26, 2022

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

Uh oh!

This comment has been minimized.

This comment has been minimized.

dromanol left a comment

Choose a reason for hiding this comment

Uh oh!

bouwkast left a comment

Choose a reason for hiding this comment

Uh oh!

bouwkast Aug 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bouwkast Aug 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bouwkast Aug 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bouwkast Aug 29, 2022

Choose a reason for hiding this comment

Uh oh!

bouwkast Aug 29, 2022

Choose a reason for hiding this comment

Uh oh!

bouwkast Aug 29, 2022

Choose a reason for hiding this comment

Uh oh!

andrewlock Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Aug 31, 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 Aug 31, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bouwkast Aug 29, 2022 •

edited

Loading

bouwkast Aug 29, 2022 •

edited

Loading

bouwkast Aug 29, 2022 •

edited

Loading