-
Notifications
You must be signed in to change notification settings - Fork 150
[Tracer] Add new process tags #7651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing the following branches/commits: Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (75ms) : 73, 77
. : milestone, 75,
master - mean (75ms) : 74, 77
. : milestone, 75,
section Baseline
This PR (7651) - mean (71ms) : 70, 73
. : milestone, 71,
master - mean (71ms) : 68, 75
. : milestone, 71,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (1,067ms) : 1000, 1135
. : milestone, 1067,
master - mean (1,072ms) : 992, 1152
. : milestone, 1072,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (112ms) : 107, 117
. : milestone, 112,
master - mean (110ms) : 108, 112
. : milestone, 110,
section Baseline
This PR (7651) - mean (112ms) : 109, 115
. : milestone, 112,
master - mean (110ms) : 107, 112
. : milestone, 110,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (761ms) : 735, 787
. : milestone, 761,
master - mean (761ms) : 733, 788
. : milestone, 761,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (100ms) : 94, 106
. : milestone, 100,
master - mean (98ms) : 96, 100
. : milestone, 98,
section Baseline
This PR (7651) - mean (98ms) : 94, 102
. : milestone, 98,
master - mean (97ms) : 94, 101
. : milestone, 97,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (717ms) : 683, 751
. : milestone, 717,
master - mean (729ms) : 671, 788
. : milestone, 729,
gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (99ms) : 96, 102
. : milestone, 99,
master - mean (98ms) : 95, 100
. : milestone, 98,
section Baseline
This PR (7651) - mean (97ms) : 94, 101
. : milestone, 97,
master - mean (97ms) : 94, 99
. : milestone, 97,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (673ms) : 656, 690
. : milestone, 673,
master - mean (679ms) : 661, 698
. : milestone, 679,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (201ms) : 196, 206
. : milestone, 201,
master - mean (196ms) : 193, 199
. : milestone, 196,
section Baseline
This PR (7651) - mean (196ms) : 190, 202
. : milestone, 196,
master - mean (193ms) : 189, 197
. : milestone, 193,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (1,166ms) : 1109, 1223
. : milestone, 1166,
master - mean (1,164ms) : 1108, 1219
. : milestone, 1164,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (277ms) : 273, 282
. : milestone, 277,
master - mean (278ms) : 273, 283
. : milestone, 278,
section Baseline
This PR (7651) - mean (276ms) : 273, 280
. : milestone, 276,
master - mean (276ms) : 271, 281
. : milestone, 276,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (947ms) : 901, 993
. : milestone, 947,
master - mean (944ms) : 905, 982
. : milestone, 944,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (270ms) : 266, 274
. : milestone, 270,
master - mean (270ms) : 266, 274
. : milestone, 270,
section Baseline
This PR (7651) - mean (270ms) : 266, 274
. : milestone, 270,
master - mean (270ms) : 266, 273
. : milestone, 270,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (927ms) : 880, 975
. : milestone, 927,
master - mean (926ms) : 881, 971
. : milestone, 926,
gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7651) - mean (270ms) : 266, 274
. : milestone, 270,
master - mean (270ms) : 266, 273
. : milestone, 270,
section Baseline
This PR (7651) - mean (270ms) : 265, 275
. : milestone, 270,
master - mean (269ms) : 265, 273
. : milestone, 269,
section CallTarget+Inlining+NGEN
This PR (7651) - mean (854ms) : 831, 877
. : milestone, 854,
master - mean (857ms) : 836, 878
. : milestone, 857,
|
61ef81b to
f252a35
Compare
9dbe1e8 to
415a4f4
Compare
| { | ||
| var tags = new Dictionary<string, string>(); | ||
|
|
||
| var entrypointFullName = Assembly.GetEntryAssembly()?.EntryPoint?.DeclaringType?.FullName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I'm not mistaken there are at least 2 different entrypoint.type in dotnet, a binary or a .dll
similarly in java the split is done with entrypoint.type:jar_name/main_class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I was thinking of adding those dotnet specific details in an other PR, where we could discuss the specifics.
I can see several entry point types :
- Console app
- GUI (winforms/WPF)
- ASP net webapp
- windows service
- Azure function
- triggered by another library ? (like tests ?)
- Xamarin mobile app ?
Not sure the distinction exe/dll is super relevant, because a dll can behave the same way as an exe when launched with dotnet run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most importantly, Assembly.GetEntryAssembly() doesn't work for web apps. We have a helper to try to grab this: https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/Configuration/EntryAssemblyLocator.cs#L16
But an important part to understand and work around is that this might not be available on app startup... At some "random" later point (i.e. during a reques), we eventually will be able to access the "real" entrypoint for the application.
We already have to work around this currently by retrying multiple times if it fails the first 100 times; we probably will need to do something similar here: https://github.com/DataDog/dd-trace-dotnet/blob/master/tracer/src/Datadog.Trace/Configuration/GitMetadataTagsProvider.cs#L150-L154
I'm not sure what the implications of this are for the process tags feature in general, but it ultimately means that we probably need to add an instrumentation to make sure we trigger loading the entry assembly at the right point (inside a request for owin/aspnet apps, or just before flushing the first span for everything else).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should happen just before the first flush, because we query this in SpanMessagePackFormatter, so I'm not too worried about it not being available at app startup.
Where it could become trouble is if the value is flaky because of that. If the customer writes a rule to set the service name based on this value, but it's only here in 50% of the spans (or any % that is != 100), that no good. In that case, one could even argue it's better to not have the value at all (to not tempt users into sin)
tracer/src/Datadog.Trace/Agent/MessagePack/SpanMessagePackFormatter.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, lgtm!
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tracer/src/Datadog.Trace/Agent/MessagePack/SpanMessagePackFormatter.cs
Outdated
Show resolved
Hide resolved
|
|
||
| // NOTE: all of these can be cached in SpanMessagePackFormatter as static byte[] | ||
| // since they never change over the lifetime of a process | ||
| private static CachedBytes _processTags; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the comment above 😅
NOTE: all of these can be cached in SpanMessagePackFormatter as
static byte[]since they never change over the lifetime of a process
See _languageValueBytes and _runtimeIdValueBytes in SpanMessagePackFormatter for examples.
tracer/src/Datadog.Trace/Agent/MessagePack/SpanMessagePackFormatter.cs
Outdated
Show resolved
Hide resolved
tracer/src/Datadog.Trace/Agent/MessagePack/SpanMessagePackFormatter.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Propagate the process tags in every supported payload | ||
| /// </summary> | ||
| public const string PropagateProcessTags = "DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use the DD_EXPERIMENTAL_ prefix if it's enabled by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, it's not supposed to be enabled by default, at least not for now.
see
dd-trace-dotnet/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
Lines 105 to 108 in cc4b720
| PropagateProcessTags = config | |
| .WithKeys(ConfigurationKeys.PropagateProcessTags) | |
| .AsBool(false) | |
| || ExperimentalFeaturesEnabled.Contains(ConfigurationKeys.PropagateProcessTags); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see .AsBool(false), but also || ExperimentalFeaturesEnabled.Contains(...) which will be true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait. I was getting DefaultExperimentalFeatures and ExperimentalFeaturesEnabled mixed up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still, why EXPERIMENTAL?
Co-authored-by: Lucas Pimentel <[email protected]>
Co-authored-by: Lucas Pimentel <[email protected]>
Co-authored-by: Lucas Pimentel <[email protected]>
# Conflicts: # tracer/src/Datadog.Trace/ProcessTags.cs
BenchmarksBenchmarks Report for benchmark platform 🐌Benchmarks for #7651 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecBodyBenchmark - Unknown 🤷 Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecEncoderBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Asm.AppSecWafBenchmark - Slower
|
| Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWafRealisticBenchmark‑netcoreapp3.1 | 2.068 | 414,421.67 | 856,956.99 |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunWafRealisticBenchmark |
net6.0 | 393μs | 217ns | 840ns | 0 | 0 | 0 | 4.55 KB |
| master | RunWafRealisticBenchmark |
netcoreapp3.1 | 415μs | 96.2ns | 347ns | 0 | 0 | 0 | 4.48 KB |
| master | RunWafRealisticBenchmark |
net472 | 427μs | 27.3ns | 98.4ns | 0 | 0 | 0 | 4.66 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net6.0 | 283μs | 35.1ns | 131ns | 0 | 0 | 0 | 2.24 KB |
| master | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 298μs | 225ns | 870ns | 0 | 0 | 0 | 2.22 KB |
| master | RunWafRealisticBenchmarkWithAttack |
net472 | 310μs | 30ns | 116ns | 0 | 0 | 0 | 2.29 KB |
| #7651 | RunWafRealisticBenchmark |
net6.0 | 393μs | 38.4ns | 144ns | 0 | 0 | 0 | 4.55 KB |
| #7651 | RunWafRealisticBenchmark |
netcoreapp3.1 | 854μs | 3.46μs | 13.4μs | 0 | 0 | 0 | 4.48 KB |
| #7651 | RunWafRealisticBenchmark |
net472 | 427μs | 48.8ns | 189ns | 0 | 0 | 0 | 4.66 KB |
| #7651 | RunWafRealisticBenchmarkWithAttack |
net6.0 | 286μs | 59.2ns | 229ns | 0 | 0 | 0 | 2.24 KB |
| #7651 | RunWafRealisticBenchmarkWithAttack |
netcoreapp3.1 | 296μs | 49.8ns | 186ns | 0 | 0 | 0 | 2.22 KB |
| #7651 | RunWafRealisticBenchmarkWithAttack |
net472 | 310μs | 209ns | 808ns | 0 | 0 | 0 | 2.29 KB |
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | SendRequest |
net6.0 | 60.5μs | 55.6ns | 215ns | 0 | 0 | 0 | 14.52 KB |
| master | SendRequest |
netcoreapp3.1 | 71.7μs | 150ns | 563ns | 0 | 0 | 0 | 17.42 KB |
| master | SendRequest |
net472 | 0.00267ns | 0.00131ns | 0.00506ns | 0 | 0 | 0 | 0 b |
| #7651 | SendRequest |
net6.0 | 62.3μs | 170ns | 635ns | 0 | 0 | 0 | 14.52 KB |
| #7651 | SendRequest |
netcoreapp3.1 | 71.3μs | 275ns | 992ns | 0 | 0 | 0 | 17.42 KB |
| #7651 | SendRequest |
net472 | 0.0107ns | 0.00331ns | 0.0128ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CharSliceBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7651
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0
1 B
3 B
2 B
200.00%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑netcoreapp3.1
6 B
16 B
10 B
166.67%
Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0
4 B
5 B
1 B
25.00%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑net6.0 | 1 B | 3 B | 2 B | 200.00% |
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool‑netcoreapp3.1 | 6 B | 16 B | 10 B | 166.67% |
| Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice‑net6.0 | 4 B | 5 B | 1 B | 25.00% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | OriginalCharSlice |
net6.0 | 1.89ms | 919ns | 3.44μs | 0 | 0 | 0 | 640.01 KB |
| master | OriginalCharSlice |
netcoreapp3.1 | 2.12ms | 7.88μs | 30.5μs | 0 | 0 | 0 | 640 KB |
| master | OriginalCharSlice |
net472 | 2.55ms | 190ns | 713ns | 100 | 0 | 0 | 641.95 KB |
| master | OptimizedCharSlice |
net6.0 | 1.38ms | 243ns | 940ns | 0 | 0 | 0 | 4 B |
| master | OptimizedCharSlice |
netcoreapp3.1 | 1.66ms | 343ns | 1.28μs | 0 | 0 | 0 | 1 B |
| master | OptimizedCharSlice |
net472 | 1.93ms | 157ns | 607ns | 0 | 0 | 0 | 0 b |
| master | OptimizedCharSliceWithPool |
net6.0 | 898μs | 98.8ns | 383ns | 0 | 0 | 0 | 1 B |
| master | OptimizedCharSliceWithPool |
netcoreapp3.1 | 811μs | 166ns | 621ns | 0 | 0 | 0 | 6 B |
| master | OptimizedCharSliceWithPool |
net472 | 1.15ms | 114ns | 440ns | 0 | 0 | 0 | 0 b |
| #7651 | OriginalCharSlice |
net6.0 | 1.95ms | 252ns | 944ns | 0 | 0 | 0 | 640.01 KB |
| #7651 | OriginalCharSlice |
netcoreapp3.1 | 2.11ms | 6.38μs | 22.1μs | 0 | 0 | 0 | 640 KB |
| #7651 | OriginalCharSlice |
net472 | 2.59ms | 535ns | 2μs | 100 | 0 | 0 | 641.95 KB |
| #7651 | OptimizedCharSlice |
net6.0 | 1.48ms | 662ns | 2.56μs | 0 | 0 | 0 | 5 B |
| #7651 | OptimizedCharSlice |
netcoreapp3.1 | 1.69ms | 411ns | 1.54μs | 0 | 0 | 0 | 1 B |
| #7651 | OptimizedCharSlice |
net472 | 2.03ms | 235ns | 909ns | 0 | 0 | 0 | 0 b |
| #7651 | OptimizedCharSliceWithPool |
net6.0 | 854μs | 57.7ns | 216ns | 0 | 0 | 0 | 3 B |
| #7651 | OptimizedCharSliceWithPool |
netcoreapp3.1 | 811μs | 145ns | 563ns | 0 | 0 | 0 | 16 B |
| #7651 | OptimizedCharSliceWithPool |
net472 | 1.15ms | 87ns | 337ns | 0 | 0 | 0 | 0 b |
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️
More allocations ⚠️ in #7651
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472
55.96 KB
56.42 KB
464 B
0.83%
Fewer allocations 🎉 in #7651
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1
42.24 KB
42.02 KB
-226 B
-0.54%
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net472 | 55.96 KB | 56.42 KB | 464 B | 0.83% |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 | 42.24 KB | 42.02 KB | -226 B | -0.54% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | WriteAndFlushEnrichedTraces |
net6.0 | 742μs | 2.01μs | 7.79μs | 0 | 0 | 0 | 41.64 KB |
| master | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 726μs | 1.04μs | 4.04μs | 0 | 0 | 0 | 42.24 KB |
| master | WriteAndFlushEnrichedTraces |
net472 | 904μs | 4.57μs | 21μs | 4.46 | 0 | 0 | 55.96 KB |
| #7651 | WriteAndFlushEnrichedTraces |
net6.0 | 710μs | 4.02μs | 27μs | 0 | 0 | 0 | 41.66 KB |
| #7651 | WriteAndFlushEnrichedTraces |
netcoreapp3.1 | 756μs | 4.34μs | 33.4μs | 0 | 0 | 0 | 42.02 KB |
| #7651 | WriteAndFlushEnrichedTraces |
net472 | 945μs | 2.95μs | 11.4μs | 8.33 | 0 | 0 | 56.42 KB |
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | ExecuteNonQuery |
net6.0 | 1.95μs | 0.533ns | 1.99ns | 0 | 0 | 0 | 1.02 KB |
| master | ExecuteNonQuery |
netcoreapp3.1 | 2.6μs | 1.89ns | 6.81ns | 0 | 0 | 0 | 1.02 KB |
| master | ExecuteNonQuery |
net472 | 2.91μs | 3.49ns | 13.5ns | 0.143 | 0.0143 | 0 | 987 B |
| #7651 | ExecuteNonQuery |
net6.0 | 2.02μs | 9.55ns | 39.4ns | 0 | 0 | 0 | 1.02 KB |
| #7651 | ExecuteNonQuery |
netcoreapp3.1 | 2.63μs | 9.52ns | 36.9ns | 0 | 0 | 0 | 1.02 KB |
| #7651 | ExecuteNonQuery |
net472 | 3μs | 3.04ns | 11.8ns | 0.149 | 0.0149 | 0 | 987 B |
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | CallElasticsearch |
net6.0 | 1.68μs | 8.33ns | 36.3ns | 0 | 0 | 0 | 1.03 KB |
| master | CallElasticsearch |
netcoreapp3.1 | 2.2μs | 5.6ns | 21.7ns | 0 | 0 | 0 | 1.03 KB |
| master | CallElasticsearch |
net472 | 3.5μs | 4.22ns | 16.3ns | 0.158 | 0 | 0 | 1.04 KB |
| master | CallElasticsearchAsync |
net6.0 | 1.83μs | 5.3ns | 19.1ns | 0 | 0 | 0 | 1.01 KB |
| master | CallElasticsearchAsync |
netcoreapp3.1 | 2.36μs | 10.9ns | 42.2ns | 0 | 0 | 0 | 1.08 KB |
| master | CallElasticsearchAsync |
net472 | 3.6μs | 2.09ns | 8.08ns | 0.163 | 0 | 0 | 1.1 KB |
| #7651 | CallElasticsearch |
net6.0 | 1.72μs | 8.73ns | 40ns | 0 | 0 | 0 | 1.03 KB |
| #7651 | CallElasticsearch |
netcoreapp3.1 | 2.32μs | 9.78ns | 37.9ns | 0 | 0 | 0 | 1.03 KB |
| #7651 | CallElasticsearch |
net472 | 3.48μs | 2.62ns | 10.1ns | 0.158 | 0 | 0 | 1.04 KB |
| #7651 | CallElasticsearchAsync |
net6.0 | 1.85μs | 8.73ns | 38ns | 0 | 0 | 0 | 1.01 KB |
| #7651 | CallElasticsearchAsync |
netcoreapp3.1 | 2.35μs | 11.5ns | 50.3ns | 0 | 0 | 0 | 1.08 KB |
| #7651 | CallElasticsearchAsync |
net472 | 3.66μs | 3.31ns | 12.8ns | 0.165 | 0 | 0 | 1.1 KB |
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | ExecuteAsync |
net6.0 | 1.88μs | 8.97ns | 35.9ns | 0 | 0 | 0 | 952 B |
| master | ExecuteAsync |
netcoreapp3.1 | 2.44μs | 5.09ns | 19.7ns | 0 | 0 | 0 | 952 B |
| master | ExecuteAsync |
net472 | 2.63μs | 2.23ns | 8.63ns | 0.143 | 0 | 0 | 915 B |
| #7651 | ExecuteAsync |
net6.0 | 1.89μs | 7.49ns | 29ns | 0 | 0 | 0 | 952 B |
| #7651 | ExecuteAsync |
netcoreapp3.1 | 2.54μs | 6.42ns | 24.9ns | 0 | 0 | 0 | 952 B |
| #7651 | ExecuteAsync |
net472 | 2.62μs | 3.62ns | 14ns | 0.143 | 0 | 0 | 915 B |
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | SendAsync |
net6.0 | 6.86μs | 6.71ns | 26ns | 0 | 0 | 0 | 2.36 KB |
| master | SendAsync |
netcoreapp3.1 | 8.72μs | 26.2ns | 102ns | 0 | 0 | 0 | 2.9 KB |
| master | SendAsync |
net472 | 12.2μs | 7.61ns | 28.5ns | 0.487 | 0 | 0 | 3.18 KB |
| #7651 | SendAsync |
net6.0 | 6.82μs | 27.5ns | 103ns | 0 | 0 | 0 | 2.36 KB |
| #7651 | SendAsync |
netcoreapp3.1 | 8.57μs | 22.1ns | 85.6ns | 0 | 0 | 0 | 2.9 KB |
| #7651 | SendAsync |
net472 | 12.5μs | 11.4ns | 42.7ns | 0.497 | 0 | 0 | 3.18 KB |
Benchmarks.Trace.Iast.StringAspectsBenchmark - Faster 🎉 More allocations ⚠️
Faster 🎉 in #7651
Benchmark
base/diff
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1
2.979
1,546,250.00
519,100.00
More allocations ⚠️ in #7651
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0
250.46 KB
260.03 KB
9.58 KB
3.82%
Fewer allocations 🎉 in #7651
Benchmark
Base Allocated
Diff Allocated
Change
Change %
Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1
335.15 KB
258.65 KB
-76.5 KB
-22.83%
| Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 2.979 | 1,546,250.00 | 519,100.00 |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑net6.0 | 250.46 KB | 260.03 KB | 9.58 KB | 3.82% |
| Benchmark | Base Allocated | Diff Allocated | Change | Change % |
|---|---|---|---|---|
| Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatAspectBenchmark‑netcoreapp3.1 | 335.15 KB | 258.65 KB | -76.5 KB | -22.83% |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StringConcatBenchmark |
net6.0 | 41.7μs | 209ns | 886ns | 0 | 0 | 0 | 43.78 KB |
| master | StringConcatBenchmark |
netcoreapp3.1 | 47.8μs | 164ns | 658ns | 0 | 0 | 0 | 42.92 KB |
| master | StringConcatBenchmark |
net472 | 57μs | 261ns | 1.04μs | 0 | 0 | 0 | 57.34 KB |
| master | StringConcatAspectBenchmark |
net6.0 | 441μs | 645ns | 2.23μs | 0 | 0 | 0 | 250.46 KB |
| master | StringConcatAspectBenchmark |
netcoreapp3.1 | 1.55ms | 2.22μs | 8.32μs | 0 | 0 | 0 | 335.15 KB |
| master | StringConcatAspectBenchmark |
net472 | 411μs | 2.06μs | 8.72μs | 0 | 0 | 0 | 286.02 KB |
| #7651 | StringConcatBenchmark |
net6.0 | 41.6μs | 129ns | 446ns | 0 | 0 | 0 | 43.8 KB |
| #7651 | StringConcatBenchmark |
netcoreapp3.1 | 49.5μs | 276ns | 1.91μs | 0 | 0 | 0 | 42.9 KB |
| #7651 | StringConcatBenchmark |
net472 | 56.3μs | 279ns | 1.15μs | 0 | 0 | 0 | 57.34 KB |
| #7651 | StringConcatAspectBenchmark |
net6.0 | 467μs | 1.02μs | 3.52μs | 0 | 0 | 0 | 260.03 KB |
| #7651 | StringConcatAspectBenchmark |
netcoreapp3.1 | 520μs | 1.5μs | 5.41μs | 0 | 0 | 0 | 258.65 KB |
| #7651 | StringConcatAspectBenchmark |
net472 | 395μs | 1.21μs | 4.2μs | 0 | 0 | 0 | 286.72 KB |
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EnrichedLog |
net6.0 | 2.63μs | 4.76ns | 17.8ns | 0 | 0 | 0 | 1.7 KB |
| master | EnrichedLog |
netcoreapp3.1 | 3.54μs | 18.5ns | 90.6ns | 0 | 0 | 0 | 1.71 KB |
| master | EnrichedLog |
net472 | 3.84μs | 1.57ns | 5.87ns | 0.25 | 0 | 0 | 1.64 KB |
| #7651 | EnrichedLog |
net6.0 | 2.64μs | 5.52ns | 21.4ns | 0 | 0 | 0 | 1.7 KB |
| #7651 | EnrichedLog |
netcoreapp3.1 | 3.69μs | 17.8ns | 71.3ns | 0 | 0 | 0 | 1.7 KB |
| #7651 | EnrichedLog |
net472 | 4.2μs | 3.57ns | 13.4ns | 0.252 | 0 | 0 | 1.64 KB |
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EnrichedLog |
net6.0 | 124μs | 89.6ns | 323ns | 0 | 0 | 0 | 4.31 KB |
| master | EnrichedLog |
netcoreapp3.1 | 128μs | 25.6ns | 92.4ns | 0 | 0 | 0 | 4.31 KB |
| master | EnrichedLog |
net472 | 168μs | 42.9ns | 166ns | 0 | 0 | 0 | 4.52 KB |
| #7651 | EnrichedLog |
net6.0 | 122μs | 35.8ns | 134ns | 0 | 0 | 0 | 4.31 KB |
| #7651 | EnrichedLog |
netcoreapp3.1 | 127μs | 207ns | 802ns | 0 | 0 | 0 | 4.31 KB |
| #7651 | EnrichedLog |
net472 | 167μs | 30.7ns | 119ns | 0 | 0 | 0 | 4.52 KB |
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EnrichedLog |
net6.0 | 4.97μs | 13.2ns | 51ns | 0 | 0 | 0 | 2.26 KB |
| master | EnrichedLog |
netcoreapp3.1 | 6.8μs | 12.1ns | 46.8ns | 0 | 0 | 0 | 2.26 KB |
| master | EnrichedLog |
net472 | 7.7μs | 8.85ns | 34.3ns | 0.307 | 0 | 0 | 2.08 KB |
| #7651 | EnrichedLog |
net6.0 | 5μs | 24.4ns | 103ns | 0 | 0 | 0 | 2.26 KB |
| #7651 | EnrichedLog |
netcoreapp3.1 | 7.01μs | 11.9ns | 46ns | 0 | 0 | 0 | 2.26 KB |
| #7651 | EnrichedLog |
net472 | 7.75μs | 4.6ns | 17.8ns | 0.309 | 0 | 0 | 2.08 KB |
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | SendReceive |
net6.0 | 2μs | 0.971ns | 3.5ns | 0 | 0 | 0 | 1.2 KB |
| master | SendReceive |
netcoreapp3.1 | 2.69μs | 1.35ns | 5.25ns | 0 | 0 | 0 | 1.2 KB |
| master | SendReceive |
net472 | 3.18μs | 2.59ns | 10ns | 0.19 | 0 | 0 | 1.2 KB |
| #7651 | SendReceive |
net6.0 | 2.07μs | 10.5ns | 48.2ns | 0 | 0 | 0 | 1.2 KB |
| #7651 | SendReceive |
netcoreapp3.1 | 2.69μs | 9.47ns | 34.1ns | 0 | 0 | 0 | 1.2 KB |
| #7651 | SendReceive |
net472 | 3.16μs | 3.99ns | 15.4ns | 0.191 | 0 | 0 | 1.2 KB |
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | EnrichedLog |
net6.0 | 4.4μs | 14.9ns | 57.5ns | 0 | 0 | 0 | 1.58 KB |
| master | EnrichedLog |
netcoreapp3.1 | 5.73μs | 19.8ns | 76.5ns | 0 | 0 | 0 | 1.63 KB |
| master | EnrichedLog |
net472 | 6.71μs | 5.97ns | 22.3ns | 0.299 | 0 | 0 | 2.03 KB |
| #7651 | EnrichedLog |
net6.0 | 4.24μs | 20.4ns | 81.5ns | 0 | 0 | 0 | 1.58 KB |
| #7651 | EnrichedLog |
netcoreapp3.1 | 5.65μs | 9.19ns | 34.4ns | 0 | 0 | 0 | 1.63 KB |
| #7651 | EnrichedLog |
net472 | 6.8μs | 6.65ns | 25.7ns | 0.304 | 0 | 0 | 2.03 KB |
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StartFinishSpan |
net6.0 | 790ns | 0.365ns | 1.41ns | 0 | 0 | 0 | 576 B |
| master | StartFinishSpan |
netcoreapp3.1 | 981ns | 5.14ns | 26.2ns | 0 | 0 | 0 | 576 B |
| master | StartFinishSpan |
net472 | 920ns | 0.55ns | 2.06ns | 0.0882 | 0 | 0 | 578 B |
| master | StartFinishScope |
net6.0 | 912ns | 3.36ns | 12.6ns | 0 | 0 | 0 | 696 B |
| master | StartFinishScope |
netcoreapp3.1 | 1.17μs | 6.25ns | 31.3ns | 0 | 0 | 0 | 696 B |
| master | StartFinishScope |
net472 | 1.16μs | 1.07ns | 4.14ns | 0.105 | 0 | 0 | 658 B |
| #7651 | StartFinishSpan |
net6.0 | 786ns | 0.438ns | 1.64ns | 0 | 0 | 0 | 576 B |
| #7651 | StartFinishSpan |
netcoreapp3.1 | 962ns | 5.23ns | 29.6ns | 0 | 0 | 0 | 576 B |
| #7651 | StartFinishSpan |
net472 | 940ns | 0.0314ns | 0.109ns | 0.0895 | 0 | 0 | 578 B |
| #7651 | StartFinishScope |
net6.0 | 917ns | 1.27ns | 4.9ns | 0 | 0 | 0 | 696 B |
| #7651 | StartFinishScope |
netcoreapp3.1 | 1.18μs | 6.23ns | 33ns | 0 | 0 | 0 | 696 B |
| #7651 | StartFinishScope |
net472 | 1.17μs | 2.12ns | 8.22ns | 0.1 | 0 | 0 | 658 B |
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | RunOnMethodBegin |
net6.0 | 1.06μs | 5.42ns | 26.5ns | 0 | 0 | 0 | 697 B |
| master | RunOnMethodBegin |
netcoreapp3.1 | 1.41μs | 7.25ns | 34.8ns | 0 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
net472 | 1.43μs | 0.56ns | 2.1ns | 0.1 | 0 | 0 | 658 B |
| #7651 | RunOnMethodBegin |
net6.0 | 1.1μs | 2.36ns | 9.15ns | 0 | 0 | 0 | 696 B |
| #7651 | RunOnMethodBegin |
netcoreapp3.1 | 1.4μs | 6.49ns | 26.8ns | 0 | 0 | 0 | 696 B |
| #7651 | RunOnMethodBegin |
net472 | 1.46μs | 0.738ns | 2.86ns | 0.102 | 0 | 0 | 658 B |
| private readonly byte[] _runtimeIdValueBytes = StringEncoding.UTF8.GetBytes(Tracer.RuntimeId); | ||
|
|
||
| // using a Lazy here to make sure we don't compute the value of the process tags too early in the life of the app, | ||
| // some values may neeb a bit of time to be accessible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // some values may neeb a bit of time to be accessible. | |
| // some values may need a bit of time to be accessible. |
| isLocalRoot: isLocalRoot, | ||
| isChunkOrphan: isChunkOrphan, | ||
| isFirstSpanInChunk: isFirstSpan); | ||
| isLocalRoot, | ||
| isChunkOrphan, | ||
| isFirstSpan); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We like having the parameter names sometimes to avoid getting arguments mixed up. Since they are all bools, it would be easy to switch the order around the the code will still compile. This way as least any errors are more visible when reading the code.
| using System.Linq; | ||
| using System.Reflection; | ||
| using System.Text; | ||
| using Datadog.Trace.Processors; | ||
| using Datadog.Trace.VendoredMicrosoftCode.System.Collections.Immutable; | ||
| using Datadog.Trace.Vendors.Newtonsoft.Json.Utilities; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used.
| using System.Linq; | |
| using System.Reflection; | |
| using System.Text; | |
| using Datadog.Trace.Processors; | |
| using Datadog.Trace.VendoredMicrosoftCode.System.Collections.Immutable; | |
| using Datadog.Trace.Vendors.Newtonsoft.Json.Utilities; | |
| using System.Reflection; | |
| using System.Text; | |
| using Datadog.Trace.Processors; |
| /// <param name="samplingPriority">Optional sampling priority to override the <see cref="TraceContext"/> sampling priority.</param> | ||
| public TraceChunkModel(in ArraySegment<Span> spans, int? samplingPriority = null) | ||
| : this(spans, TraceContext.GetTraceContext(spans), samplingPriority) | ||
| /// <param name="isFirstChunkInPayload">marks if this is the first chunk being written to the buffer that then gets sent to the agent</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// <param name="isFirstChunkInPayload">marks if this is the first chunk being written to the buffer that then gets sent to the agent</param> | |
| /// <param name="isFirstChunkInPayload">Indicates if this is the first trace chunk being written to the output buffer.</param> |
| var settings = new TracerSettings(new NameValueConfigurationSource(new NameValueCollection { { ConfigurationKeys.PropagateProcessTags, enabled ? "true" : "false" } })); | ||
| var tracer = TracerHelper.Create(settings); | ||
|
|
||
| var parentContext = new SpanContext(new TraceId(Upper: 0, Lower: 1), spanId: 2, (int)SamplingPriority.UserKeep, "ServiceName1", "origin1"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not create standalone spans like this without a TraceContext anymore. We expect all spans to belong to a trace and this can mess things up. If you see a test doing this, it's an old test that we haven't updated yet 😅
|
|
||
| private static SortedDictionary<string, string> LoadBaseTags() | ||
| { | ||
| var tags = new SortedDictionary<string, string>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be List<KeyValuePair<<string, string>>, which does less working during inserts and allocates less data structures on the heap. Since we are adding the tags manually, we can ensure they 1) they are sorted and 2) they have unique keys, without the extra overhead of a SortedDictionary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, we can concatenate each key/value into a StringBuilder right here without creating any collection at all. Something like:
var serializedTags = StringBuilderCache.Acquire();
var entrypointFullName = Assembly.GetEntryAssembly()?.EntryPoint?.DeclaringType?.FullName;
if (!StringUtil.IsNullOrEmpty(entrypointFullName))
{
serializedTags.Append($"{EntrypointName}:{NormalizeTagValue(entrypointFullName)},");
}
serializedTags.Append($"{EntrypointBasedir}:{NormalizeTagValue(GetLastPathSegment(AppContext.BaseDirectory))},");
// workdir can be changed by the code, but we consider that capturing the value when this is called is good enough
serializedTags.Append($"{EntrypointWorkdir}:{NormalizeTagValue(GetLastPathSegment(Environment.CurrentDirectory))}"); // don't include last comma
return StringBuilderCache.GetStringAndRelease(serializedTags);You'll have to add the tags in the right order if you want them sorted.
Note also the use of StringBuilderCache instead of StringBuilder (we reuse builders to reduce allocations), and StringUtil.IsNullOrEmpty() instead of string.IsNullOrEmpty() (older versions of .NET don't have the nullability annotations).
Summary of changes
this is the first PR introducing process tags to dotnet, see RFC
The goal is to pass as much data as possible to the backend to enable customers to set the "correct" names for their services.
Reason for change
overarching product need
Implementation details
Test coverage
Other details