[Tracer] add support for trace-level tags (vertical propagation) by lucaspimentel · Pull Request #2432 · DataDog/dd-trace-dotnet

lucaspimentel · 2022-02-14T05:34:33Z

Continuation of #2178.

Add support for trace-level tags that are propagated via x-datadog-tags header. Trace-level tags are now sent to the Agent by adding them to root spans during serialization. (aka vertical propagation)

add new TraceTagNames for well-known trace-level tags (leaving Tags for span-level tags)
- add new trace-level tag _dd.propagation_error to denote propagation errors (e.g. header too long)
add new class TraceTagsCollection with support for parsing from and serializing to the x-datadog-tags propagation header
replace string TraceContext.DatadogTags with TraceTagsCollection TraceContext.Tags
- during incoming calls from upstream services, parse the x-datadog-tags header to populate TraceContext.Tags
- during outgoing calls to downstream services, inject the x-datadog-tags header with values from TraceContext.Tags
during trace serialization, if a span is the root span, add the tags from TraceContext.Tags

Future work:

next PRs
- add the concept of "sampling mechanism", and track sampling priority, sampling mechanism, and sampling rate throughout so we can propagate this info later
- add special support for the _dd.p.upstream_services trace-level tag
handle adding trace tags to other spans besides root when the root is not present in the same tace chunk (find the first span with no parent?)
someday: migrate existing trace-level tags to use this mechanism? (_sampling_priority_v1, _dd.origin, _dd.tracer_kr, and many more...)

zacharycmontoya

LGTM with some minor comments

pierotibou

Nice work. I have a few questions but LGTM.

lucaspimentel · 2022-02-16T14:46:55Z

Actually, you don't have to implement IEnumerable to use it with foreach, only the GetEnumerator method. 😊

Update: The "good thing" to only have the method without the interface is that you don't need to implement the boxing methods to IEnumerator that would cause a heap allocation in case it get called.

Good idea. I removed the IEnumerable<...> implementation in 129e0c5 so we don't use it accidentally.

tonyredondo

I already can feel the pain resolving conflicts in my agentless PR with the TagsList file modification 🥲

andrewlock · 2022-02-17T02:23:53Z

Code Coverage Report 📊

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

	master	#2432	Change
Lines	`11540` / `15880`	`11621` / `15970`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`6198` / `9276`	`6256` / `9338`
Branches %	`67%`	`67%`	`0%` ✔️
Complexity	`10480`	`10548`	`68` ⛔

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#2432	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`67%`	`67%`	`0%` ✔️
Complexity	`10480`	`10548`	`68` ⛔

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change
Datadog.Trace.ClrProfiler.AutoInstrumentation.MongoDb.MongoDbIntegration	`-7%` ⛔	`0%` ✔️	`0` ✔️

The following classes were added in #2432:

File	Line coverage	Branch coverage	Complexity
Datadog.Trace.Tagging.TraceTagCollection	`94%`	`92%`	`58`

View the full reports for further details:

andrewlock · 2022-02-17T04:22:41Z

Benchmarks Report 🐌

Benchmarks for #2432 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	891μs	4.65μs	31.9μs	3.16 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	665μs	3.62μs	28.7μs	2.57 KB
#2432	`WriteAndFlushEnrichedTraces`	net472	871μs	4.24μs	17.5μs	3.16 KB
#2432	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	671μs	2.34μs	8.76μs	2.57 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	287μs	1.05μs	4.06μs	0.142	19.71 KB
#2432	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#2432	`SendRequest`	netcoreapp3.1	292μs	1.44μs	6.62μs	0.142	19.71 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.69μs	7.51ns	29.1ns	0.0929	0.000852	586 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.45μs	5.91ns	25.7ns	0.00896	0	624 B
#2432	`ExecuteNonQuery`	net472	1.62μs	5.17ns	19.4ns	0.0928	0.000793	586 B
#2432	`ExecuteNonQuery`	netcoreapp3.1	1.48μs	5.91ns	22.1ns	0.00868	0	624 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net472	2.48μs	8.88ns	32ns	0.125	0	794 B
master	`CallElasticsearch`	netcoreapp3.1	1.69μs	9.09ns	48.9ns	0.0104	0	784 B
master	`CallElasticsearchAsync`	net472	2.65μs	6.77ns	25.3ns	0.147	0	931 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.84μs	10.1ns	60.6ns	0.0123	0	904 B
#2432	`CallElasticsearch`	net472	2.5μs	13.9ns	85.6ns	0.126	0	794 B
#2432	`CallElasticsearch`	netcoreapp3.1	1.7μs	9.47ns	57.6ns	0.0114	0	784 B
#2432	`CallElasticsearchAsync`	net472	2.8μs	14.1ns	80.9ns	0.148	0.00139	931 B
#2432	`CallElasticsearchAsync`	netcoreapp3.1	1.95μs	10.9ns	72.6ns	0.013	0	904 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteAsync`	net472	2.99μs	15.8ns	83.6ns	0.167	0.00147	1.05 KB
master	`ExecuteAsync`	netcoreapp3.1	1.93μs	8.72ns	32.6ns	0.0139	0	1.02 KB
#2432	`ExecuteAsync`	net472	2.86μs	15.2ns	87.6ns	0.165	0	1.05 KB
#2432	`ExecuteAsync`	netcoreapp3.1	1.93μs	6.27ns	23.5ns	0.0138	0	1.02 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	6.81μs	32.5ns	130ns	0.364	2.3 KB
master	`SendAsync`	netcoreapp3.1	4.81μs	26.8ns	173ns	0.0305	2.2 KB
#2432	`SendAsync`	net472	6.79μs	34.8ns	167ns	0.364	2.3 KB
#2432	`SendAsync`	netcoreapp3.1	4.62μs	14.7ns	55.1ns	0.0297	2.2 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.27μs	15.4ns	61.6ns	0.225	1.44 KB
master	`EnrichedLog`	netcoreapp3.1	3.08μs	13.6ns	52.8ns	0.0208	1.52 KB
#2432	`EnrichedLog`	net472	3.2μs	16.6ns	81.2ns	0.225	1.44 KB
#2432	`EnrichedLog`	netcoreapp3.1	2.96μs	7.24ns	28ns	0.0209	1.52 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	285μs	1.01μs	3.79μs	0.421	0.14	4.31 KB
master	`EnrichedLog`	netcoreapp3.1	229μs	800ns	3.1μs	0	0	4.19 KB
#2432	`EnrichedLog`	net472	284μs	970ns	3.76μs	0.43	0.143	4.31 KB
#2432	`EnrichedLog`	netcoreapp3.1	230μs	911ns	3.53μs	0	0	4.19 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	7.2μs	32.9ns	127ns	0.502	3.21 KB
master	`EnrichedLog`	netcoreapp3.1	6.68μs	16.5ns	63.9ns	0.0499	3.58 KB
#2432	`EnrichedLog`	net472	7.69μs	44.1ns	342ns	0.504	3.21 KB
#2432	`EnrichedLog`	netcoreapp3.1	6.18μs	13.3ns	46ns	0.0497	3.58 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.3μs	11.6ns	65.6ns	0.157	1 KB
master	`SendReceive`	netcoreapp3.1	2.1μs	11.9ns	89.4ns	0.0133	1 KB
#2432	`SendReceive`	net472	2.33μs	11.6ns	47.8ns	0.159	1 KB
#2432	`SendReceive`	netcoreapp3.1	1.96μs	5.36ns	20.7ns	0.0136	1 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	6.02μs	28.5ns	111ns	0.29	1.85 KB
master	`EnrichedLog`	netcoreapp3.1	5.01μs	19.2ns	74.5ns	0.0192	1.47 KB
#2432	`EnrichedLog`	net472	5.78μs	29.4ns	128ns	0.289	1.85 KB
#2432	`EnrichedLog`	netcoreapp3.1	4.95μs	8.33ns	32.3ns	0.0199	1.47 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	904ns	4.51ns	19.1ns	0.0704	449 B
master	`StartFinishSpan`	netcoreapp3.1	889ns	3.86ns	14.9ns	0.00603	448 B
master	`StartFinishScope`	net472	1.05μs	5.14ns	22.4ns	0.0831	530 B
master	`StartFinishScope`	netcoreapp3.1	1.02μs	4.21ns	15.8ns	0.00767	568 B
#2432	`StartFinishSpan`	net472	874ns	2.67ns	10.4ns	0.0707	449 B
#2432	`StartFinishSpan`	netcoreapp3.1	894ns	2.13ns	8.26ns	0.00616	448 B
#2432	`StartFinishScope`	net472	1.04μs	3.67ns	14.2ns	0.0833	530 B
#2432	`StartFinishScope`	netcoreapp3.1	1.03μs	3.55ns	12.3ns	0.00768	568 B

pierotibou · 2022-02-17T14:12:41Z

            return _tags.GetEnumerator();
        }
+
+        /// <summary>


nit - if this is for testing only we could imagine having an extension in the test assembly rather than in the production code.

This comment has been minimized.

Sign in to view

lucaspimentel added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:new-feature area:tests unit tests, integration tests labels Feb 14, 2022

lucaspimentel force-pushed the lpimentel/trace-tags branch 4 times, most recently from 499a5a7 to db91b0c Compare February 14, 2022 18:23

lucaspimentel marked this pull request as ready for review February 14, 2022 18:27

lucaspimentel requested a review from a team as a code owner February 14, 2022 18:27

This comment has been minimized.

Sign in to view

lucaspimentel added 3 commits February 14, 2022 15:04

add TraceTags and tests

0913b29

add Tracer.Tags

d9c24c0

add trace-level tags to root spans during serialization

332149e

lucaspimentel force-pushed the lpimentel/trace-tags branch from db91b0c to 332149e Compare February 14, 2022 20:04

This comment has been minimized.

Sign in to view

zacharycmontoya reviewed Feb 14, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/TraceTagNames.cs Outdated

zacharycmontoya reviewed Feb 14, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs Outdated

zacharycmontoya approved these changes Feb 14, 2022

View reviewed changes

pierotibou approved these changes Feb 15, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs

Comment thread tracer/src/Datadog.Trace/Tagging/TagsList.cs Outdated

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs Outdated

lucaspimentel added 9 commits February 15, 2022 11:53

fix xml-doc comment

cb62948

make one ctor private

9754382

fix off-by-one error

942aeb2

add comments

2619d48

fix xml-doc comment, redux

e471b53

keep only tags with "_dd.p.*" prefix

dc3db2a

don't allocation the string if not needed

849ca79

comments

ded0514

no need for volatile read

f589fd5

This comment has been minimized.

Sign in to view

improved validation, caching, and tests

c6e2434

lucaspimentel requested review from pierotibou and zacharycmontoya February 16, 2022 04:14

This comment has been minimized.

Sign in to view

pierotibou approved these changes Feb 16, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs Outdated

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs Outdated

Comment thread tracer/src/Datadog.Trace/TraceContext.cs

This comment has been minimized.

Sign in to view

implement IEnumerable, add GetEnumerator() and Count

9f602c4

tonyredondo reviewed Feb 16, 2022

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Tagging/TraceTagCollection.cs

lucaspimentel added 4 commits February 16, 2022 09:25

don't use AsList(), rename overloads

41004cf

remove AsList()

59ff634

remove IEnumerable to prevent accidental allocations, keep GetEnumerator

129e0c5

add missing lock

bb42761

lucaspimentel added 3 commits February 16, 2022 11:17

expand comments

57727ef

check for prefix earlier

b55285a

more comments

383b5ff

tonyredondo approved these changes Feb 16, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

fix build errors in tests

abae1c4

lucaspimentel merged commit 47dfba2 into master Feb 17, 2022

lucaspimentel deleted the lpimentel/trace-tags branch February 17, 2022 14:01

github-actions Bot added this to the vNext milestone Feb 17, 2022

pierotibou reviewed Feb 17, 2022

View reviewed changes

This was referenced Feb 18, 2022

[wip] track sampling mechanism and sampling rate #2459

Closed

[Tracer] stop propagating x-datadog-tags header #2506

Merged

lucaspimentel changed the title ~~add support for trace-level tags (vertical propagation)~~ [Tracer] add support for trace-level tags (vertical propagation) Aug 25, 2022

Uh oh!

Conversation

lucaspimentel commented Feb 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

zacharycmontoya left a comment

Choose a reason for hiding this comment

Uh oh!

pierotibou 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.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

lucaspimentel commented Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonyredondo left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

andrewlock commented Feb 17, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

andrewlock commented Feb 17, 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

Uh oh!

pierotibou Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lucaspimentel commented Feb 14, 2022 •

edited

Loading

lucaspimentel commented Feb 16, 2022 •

edited

Loading