Use the non truncated UserAgent property in .netfx webapi by pierotibou · Pull Request #2128 · DataDog/dd-trace-dotnet

pierotibou · 2021-12-06T11:23:50Z

We are propagating headers when asked. But, in webapi, the useragent collection is handled differently and the first header value returns only a substring of the UA, until the first space.
I added support to the UserAgent property provided by web api and relied on this one instead.

Jira: AIT-321

@DataDog/apm-dotnet

- Change the code to pass the useragent when needed - Impact webapi integration tests

andrewlock

Looks good, just a few questions

Removing useless log line Co-authored-by: Andrew Lock <[email protected]>

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

andrewlock · 2021-12-06T15:14:08Z

Benchmarks Report 🐌

Benchmarks for #2128 compared to master:

1 benchmarks are slower, 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	Error	StdDev	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	923.3 μs	18.27 μs	24.39 μs	3.09 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	704.7 μs	8.35 μs	7.40 μs	2.51 KB
#2128	`WriteAndFlushEnrichedTraces`	net472	944.9 μs	18.75 μs	36.12 μs	3.09 KB
#2128	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	682.0 μs	8.46 μs	7.91 μs	2.51 KB

Benchmarks.Trace.AspNetCoreBenchmark - Unknown 🤷 Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`SendRequest`	net472	0.0000 ns	0.0000 ns	0.0000 ns	0.0000	0 B
master	`SendRequest`	netcoreapp3.1	283,277.1612 ns	5,567.0640 ns	6,836.8545 ns	0.2854	20289 B
#2128	`SendRequest`	net472	0.0000 ns	0.0000 ns	0.0000 ns	0.0000	0 B
#2128	`SendRequest`	netcoreapp3.1	276,393.3015 ns	4,378.5053 ns	4,095.6565 ns	0.2772	20289 B

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev
master	`ExecuteNonQuery`	net472	316.7 ns	3.57 ns	2.78 ns
master	`ExecuteNonQuery`	netcoreapp3.1	279.2 ns	5.04 ns	4.71 ns
#2128	`ExecuteNonQuery`	net472	318.4 ns	4.51 ns	4.22 ns
#2128	`ExecuteNonQuery`	netcoreapp3.1	277.4 ns	2.85 ns	2.38 ns

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`CallElasticsearch`	net472	2.930 μs	0.0354 μs	0.0332 μs	0.1412	907 B
master	`CallElasticsearch`	netcoreapp3.1	1.677 μs	0.0190 μs	0.0169 μs	0.0122	928 B
master	`CallElasticsearchAsync`	net472	3.336 μs	0.0639 μs	0.0627 μs	0.1630	1043 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.729 μs	0.0215 μs	0.0201 μs	0.0148	1048 B
#2128	`CallElasticsearch`	net472	3.055 μs	0.0600 μs	0.1051 μs	0.1407	907 B
#2128	`CallElasticsearch`	netcoreapp3.1	1.674 μs	0.0218 μs	0.0204 μs	0.0124	928 B
#2128	`CallElasticsearchAsync`	net472	3.313 μs	0.0360 μs	0.0300 μs	0.1625	1043 B
#2128	`CallElasticsearchAsync`	netcoreapp3.1	1.768 μs	0.0202 μs	0.0168 μs	0.0150	1048 B

Benchmarks.Trace.GraphQLBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2128

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net472	1.158	3,503.10	4,056.40

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`ExecuteAsync`	net472	3.533 μs	0.0672 μs	0.0825 μs	0.1809	1.14 KB
master	`ExecuteAsync`	netcoreapp3.1	1.887 μs	0.0222 μs	0.0208 μs	0.0160	1.14 KB
#2128	`ExecuteAsync`	net472	4.059 μs	0.0799 μs	0.0785 μs	0.1810	1.14 KB
#2128	`ExecuteAsync`	netcoreapp3.1	1.990 μs	0.0349 μs	0.0512 μs	0.0163	1.14 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	6.030 μs	0.1199 μs	0.1937 μs	0.3621	2.26 KB
master	`SendAsync`	netcoreapp3.1	4.414 μs	0.0770 μs	0.0643 μs	0.0319	2.25 KB
#2128	`SendAsync`	net472	6.054 μs	0.1196 μs	0.1118 μs	0.3630	2.26 KB
#2128	`SendAsync`	netcoreapp3.1	4.304 μs	0.0546 μs	0.0511 μs	0.0314	2.25 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	4.307 μs	0.0846 μs	0.0974 μs	0.3665	2.27 KB
master	`EnrichedLog`	netcoreapp3.1	4.126 μs	0.0524 μs	0.0465 μs	0.0370	2.58 KB
#2128	`EnrichedLog`	net472	4.309 μs	0.0857 μs	0.0880 μs	0.3657	2.27 KB
#2128	`EnrichedLog`	netcoreapp3.1	4.265 μs	0.0556 μs	0.0520 μs	0.0370	2.58 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	180.6 μs	3.57 μs	5.77 μs	0.6897	0.2586	5.43 KB
master	`EnrichedLog`	netcoreapp3.1	146.5 μs	2.27 μs	3.32 μs	0.0721	0.0000	5.42 KB
#2128	`EnrichedLog`	net472	172.8 μs	2.39 μs	2.00 μs	0.6849	0.2568	5.43 KB
#2128	`EnrichedLog`	netcoreapp3.1	147.4 μs	2.23 μs	1.98 μs	0.0741	0.0000	5.42 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	12.25 μs	0.166 μs	0.148 μs	0.8956	5.58 KB
master	`EnrichedLog`	netcoreapp3.1	10.25 μs	0.168 μs	0.149 μs	0.0983	6.92 KB
#2128	`EnrichedLog`	net472	11.59 μs	0.156 μs	0.138 μs	0.8975	5.58 KB
#2128	`EnrichedLog`	netcoreapp3.1	10.63 μs	0.206 μs	0.211 μs	0.0993	6.92 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.294 μs	0.0440 μs	0.0489 μs	0.1678	1.04 KB
master	`SendReceive`	netcoreapp3.1	1.965 μs	0.0361 μs	0.0320 μs	0.0155	1.12 KB
#2128	`SendReceive`	net472	2.253 μs	0.0421 μs	0.0351 μs	0.1669	1.04 KB
#2128	`SendReceive`	netcoreapp3.1	2.046 μs	0.0261 μs	0.0244 μs	0.0161	1.12 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	7.838 μs	0.1493 μs	0.1396 μs	0.5283	3.28 KB
master	`EnrichedLog`	netcoreapp3.1	8.483 μs	0.1695 μs	0.1884 μs	0.0462	3.25 KB
#2128	`EnrichedLog`	net472	7.923 μs	0.0721 μs	0.0602 μs	0.5255	3.28 KB
#2128	`EnrichedLog`	netcoreapp3.1	7.684 μs	0.1009 μs	0.0944 μs	0.0460	3.25 KB

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

Raw results

Branch	Method	Toolchain	Mean	Error	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	886.5 ns	10.39 ns	9.72 ns	0.0681	433 B
master	`StartFinishSpan`	netcoreapp3.1	878.4 ns	17.05 ns	25.52 ns	0.0060	432 B
master	`StartFinishScope`	net472	1,159.1 ns	12.20 ns	10.82 ns	0.0930	594 B
master	`StartFinishScope`	netcoreapp3.1	1,151.3 ns	10.82 ns	9.59 ns	0.0097	712 B
#2128	`StartFinishSpan`	net472	899.6 ns	17.17 ns	16.86 ns	0.0680	433 B
#2128	`StartFinishSpan`	netcoreapp3.1	856.8 ns	8.79 ns	8.22 ns	0.0060	432 B
#2128	`StartFinishScope`	net472	1,182.7 ns	23.22 ns	38.15 ns	0.0930	594 B
#2128	`StartFinishScope`	netcoreapp3.1	1,134.0 ns	9.91 ns	9.27 ns	0.0098	712 B

andrewlock · 2021-12-06T15:51:08Z

Code Coverage Report 📊

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

	master	#2128	Change
Lines	`9107` / `12624`	`9096` / `12627`
Lines %	`72%`	`72%`	`0%` ✔️
Branches	`4094` / `6117`	`4093` / `6121`
Branches %	`67%`	`67%`	`0%` ✔️
Complexity	`7283`	`7288`	`5` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#2128	Change
Lines %	`72%`	`72%`	`0%` ✔️
Branches %	`67%`	`67%`	`0%` ✔️
Complexity	`7283`	`7288`	`5` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.ClrProfiler.AutoInstrumentation.Couchbase.IIOServiceExecuteIntegrationBis	`-100%` ⛔	`0%` ✔️	`0` ✔️
Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.MsTestV2.TestMethodRunnerExecuteIntegration	`-33%` ⛔	`-19%` ⛔	`0` ✔️
Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCompositeWorkItemSkipChildrenIntegration	`-8%` ⛔	`-8%` ⛔	`0` ✔️

View the full reports for further details:

lucaspimentel · 2021-12-07T16:38:52Z

        public const string Origin = "x-datadog-origin";
+
+        /// <summary>
+        /// Origin of the distributed trace.


Oops, wrong comment.

* Fix Typo * Use the non truncated UserAgent property in .netfx webapi - Change the code to pass the useragent when needed - Impact webapi integration tests * Update tracer/src/Datadog.Trace/SpanContextPropagator.cs Removing useless log line Co-authored-by: Andrew Lock <[email protected]> * Update tracer/src/Datadog.Trace/SpanContextPropagator.cs Co-authored-by: Andrew Lock <[email protected]> * Implement one comment * Fix Public Api Tests and add a UTest Co-authored-by: Andrew Lock <[email protected]>

* Use the non truncated UserAgent property in .netfx webapi (#2128) * Fix Typo * Use the non truncated UserAgent property in .netfx webapi - Change the code to pass the useragent when needed - Impact webapi integration tests * Update tracer/src/Datadog.Trace/SpanContextPropagator.cs Removing useless log line Co-authored-by: Andrew Lock <[email protected]> * Update tracer/src/Datadog.Trace/SpanContextPropagator.cs Co-authored-by: Andrew Lock <[email protected]> * Implement one comment * Fix Public Api Tests and add a UTest Co-authored-by: Andrew Lock <[email protected]> * Do not normalize periods in header tags if specified by the customer (#2205) * Fix normalization following Anna's spec * Add Feature Flag * change feature flag name * Add CallTarget Verified files They are actually the call site ones, the naming is inverted in the test Co-authored-by: Andrew Lock <[email protected]>

pierotibou added 2 commits December 3, 2021 05:16

Fix Typo

d265802

Use the non truncated UserAgent property in .netfx webapi

9b8d0c9

- Change the code to pass the useragent when needed - Impact webapi integration tests

pierotibou requested a review from a team as a code owner December 6, 2021 11:23

andrewlock approved these changes Dec 6, 2021

View reviewed changes

pierotibou and others added 3 commits December 6, 2021 04:08

Update tracer/src/Datadog.Trace/SpanContextPropagator.cs

50d9cdf

Removing useless log line Co-authored-by: Andrew Lock <[email protected]>

Update tracer/src/Datadog.Trace/SpanContextPropagator.cs

5389d58

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

Implement one comment

fff3052

This comment has been minimized.

Sign in to view

Fix Public Api Tests and add a UTest

02f75ef

lucaspimentel added area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) labels Dec 6, 2021

lucaspimentel approved these changes Dec 6, 2021

View reviewed changes

pierotibou merged commit d233ed2 into master Dec 7, 2021

pierotibou deleted the pierre/get-correct-ua-string branch December 7, 2021 10:58

github-actions Bot added this to the vNext milestone Dec 7, 2021

andrewlock modified the milestones: vNext, 2.0 Dec 7, 2021

lucaspimentel reviewed Dec 7, 2021

View reviewed changes

pierotibou modified the milestones: 2.0, vNext Dec 10, 2021

pierotibou mentioned this pull request Dec 27, 2021

Port header fixes from master #2242

Merged

andrewlock mentioned this pull request Feb 22, 2022

[Version Bump] v1.31.2 #2486

Merged

Conversation

pierotibou commented Dec 6, 2021

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

andrewlock commented Dec 6, 2021

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #2128

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

andrewlock commented Dec 6, 2021

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

lucaspimentel Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants