Skip to content

Add support for specifying a Content-Encoding in IApiRequest#3238

Merged
andrewlock merged 1 commit into
masterfrom
andrew/add-content-encoding-to-api-request
Sep 22, 2022
Merged

Add support for specifying a Content-Encoding in IApiRequest#3238
andrewlock merged 1 commit into
masterfrom
andrew/add-content-encoding-to-api-request

Conversation

@andrewlock

@andrewlock andrewlock commented Sep 21, 2022

Copy link
Copy Markdown
Member

Summary of changes

Adds the ability to set Content-Encoding on a POST request.

Reason for change

For data streams monitoring we need to set the Content-Encoding header. This isn't currently possible with our IApiRequest implementation (HttpClient is finicky about where you set the header).

Implementation details

Adds a new overload to IApiRequest.PostAsync() for setting the content encoding (similar to the way we set the content type). Content-Encoding needs to be set on the request content in HttpClient, not on the client itself, so the normal AddHeader() approach doesn't work here.

Test coverage

The no-encoding pathways are tested by existing tests. I haven't added explicit paths testing the setting of encoding in this PR; it's tested indirectly in a follow-up PR for DSM.

Other details

  • Did a minor cleanup of the request reuse in ApiWebRequest. The existing code feels a bit icky, but this doesn't change anything fundamental.
  • I didn't add extra unit tests for encoding because it's annoyingly hard to test this code. It feels like we should be able to say "send this request", and verify the HTTP request sent. That'll take a lot of refactoring though, so I've punted for now

@andrewlock
Add support for specifying a Content-Encoding in IApiRequest
eead0ad 
Content-Encoding needs to be set on the request _content_ in HttpClient, not on the client itself, so the normal AddHeader() approach doesn't work.

This provides an overload to PostAsync for providing the content encoding
@andrewlock andrewlock added area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) area:data-streams-monitoring labels Sep 21, 2022
@andrewlock
andrewlock requested a review from a team as a code owner September 21, 2022 10:44
@andrewlock

andrewlock commented Sep 21, 2022

Copy link
Copy Markdown
Member Author

Benchmarks Report 🐌

Benchmarks for #3238 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 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net472 713μs 386ns 1.44μs 0.355 0 0 3.18 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 461μs 156ns 582ns 0 0 0 2.58 KB
#3238 WriteAndFlushEnrichedTraces net472 710μs 694ns 2.6μs 0.355 0 0 3.18 KB
#3238 WriteAndFlushEnrichedTraces netcoreapp3.1 460μs 208ns 750ns 0 0 0 2.58 KB
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master AllCycleSimpleBody net472 191ns 0.16ns 0.578ns 0.0675 0 0 425 B
master AllCycleSimpleBody netcoreapp3.1 235ns 0.266ns 0.994ns 0.00587 0 0 424 B
master AllCycleMoreComplexBody net472 180ns 0.159ns 0.614ns 0.0638 0 0 401 B
master AllCycleMoreComplexBody netcoreapp3.1 236ns 0.217ns 0.841ns 0.00545 0 0 400 B
master BodyExtractorSimpleBody net472 259ns 0.321ns 1.2ns 0.0573 0 0 361 B
master BodyExtractorSimpleBody netcoreapp3.1 219ns 0.17ns 0.658ns 0.00374 0 0 272 B
master BodyExtractorMoreComplexBody net472 14.7μs 11.4ns 42.5ns 1.2 0.0219 0 7.62 KB
master BodyExtractorMoreComplexBody netcoreapp3.1 11.9μs 10.8ns 40.4ns 0.0894 0 0 6.75 KB
#3238 AllCycleSimpleBody net472 183ns 0.128ns 0.478ns 0.0676 9.17E-05 0 425 B
#3238 AllCycleSimpleBody netcoreapp3.1 236ns 0.154ns 0.595ns 0.00594 0 0 424 B
#3238 AllCycleMoreComplexBody net472 180ns 0.124ns 0.482ns 0.0637 0 0 401 B
#3238 AllCycleMoreComplexBody netcoreapp3.1 234ns 0.314ns 1.21ns 0.00551 0 0 400 B
#3238 BodyExtractorSimpleBody net472 268ns 0.29ns 1.12ns 0.0574 0 0 361 B
#3238 BodyExtractorSimpleBody netcoreapp3.1 219ns 0.244ns 0.943ns 0.00365 0 0 272 B
#3238 BodyExtractorMoreComplexBody net472 14.2μs 12.7ns 49.2ns 1.21 0.0214 0 7.62 KB
#3238 BodyExtractorMoreComplexBody netcoreapp3.1 12.1μs 11.1ns 41.6ns 0.0912 0 0 6.75 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 net472 0ns 0ns 0ns 0 0 0 0 b
master SendRequest netcoreapp3.1 179μs 134ns 517ns 0.268 0 0 20.6 KB
#3238 SendRequest net472 0ns 0ns 0ns 0 0 0 0 b
#3238 SendRequest netcoreapp3.1 179μs 107ns 414ns 0.179 0 0 20.58 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 net472 1.86μs 1.05ns 4.08ns 0.15 0.000939 0 947 B
master ExecuteNonQuery netcoreapp3.1 1.39μs 0.796ns 2.98ns 0.0127 0 0 936 B
#3238 ExecuteNonQuery net472 1.88μs 1.28ns 4.94ns 0.15 0.000941 0 947 B
#3238 ExecuteNonQuery netcoreapp3.1 1.38μs 0.713ns 2.57ns 0.0124 0 0 936 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 net472 2.59μs 2.43ns 9.43ns 0.184 0.00129 0 1.16 KB
master CallElasticsearch netcoreapp3.1 1.53μs 0.414ns 1.49ns 0.0145 0 0 1.1 KB
master CallElasticsearchAsync net472 2.66μs 2.09ns 8.11ns 0.205 0 0 1.29 KB
master CallElasticsearchAsync netcoreapp3.1 1.64μs 0.735ns 2.65ns 0.0164 0 0 1.22 KB
#3238 CallElasticsearch net472 2.52μs 1.52ns 5.9ns 0.183 0 0 1.16 KB
#3238 CallElasticsearch netcoreapp3.1 1.54μs 0.543ns 2.03ns 0.0146 0 0 1.1 KB
#3238 CallElasticsearchAsync net472 2.69μs 2.1ns 8.15ns 0.204 0 0 1.29 KB
#3238 CallElasticsearchAsync netcoreapp3.1 1.62μs 0.629ns 2.35ns 0.0163 0 0 1.22 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 net472 2.87μs 4.48ns 17.3ns 0.225 0 0 1.42 KB
master ExecuteAsync netcoreapp3.1 1.8μs 0.769ns 2.98ns 0.018 0 0 1.34 KB
#3238 ExecuteAsync net472 2.94μs 2.4ns 9.28ns 0.225 0 0 1.42 KB
#3238 ExecuteAsync netcoreapp3.1 1.69μs 1.79ns 6.94ns 0.0177 0 0 1.34 KB
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net472 5.9μs 2.99ns 11.2ns 0.44 0 0 2.77 KB
master SendAsync netcoreapp3.1 3.77μs 1.82ns 7.05ns 0.0358 0 0 2.6 KB
#3238 SendAsync net472 5.84μs 13.6ns 52.8ns 0.44 0 0 2.77 KB
#3238 SendAsync netcoreapp3.1 3.69μs 3.45ns 13.3ns 0.0345 0 0 2.6 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 net472 3.25μs 15.5ns 59.8ns 0.287 0 0 1.81 KB
master EnrichedLog netcoreapp3.1 2.48μs 13.5ns 109ns 0.0242 0 0 1.85 KB
#3238 EnrichedLog net472 3.04μs 2.13ns 7.68ns 0.288 0 0 1.81 KB
#3238 EnrichedLog netcoreapp3.1 2.5μs 1.37ns 5.3ns 0.025 0 0 1.85 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 net472 152μs 199ns 770ns 0.682 0.227 0 4.65 KB
master EnrichedLog netcoreapp3.1 119μs 138ns 535ns 0 0 0 4.49 KB
#3238 EnrichedLog net472 151μs 97.3ns 364ns 0.679 0.226 0 4.65 KB
#3238 EnrichedLog netcoreapp3.1 120μs 153ns 592ns 0.0599 0 0 4.49 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 net472 5.73μs 12.3ns 47.6ns 0.57 0.00292 0 3.59 KB
master EnrichedLog netcoreapp3.1 4.35μs 1.73ns 6.48ns 0.0531 0 0 3.91 KB
#3238 EnrichedLog net472 5.96μs 3.11ns 12ns 0.568 0.00299 0 3.59 KB
#3238 EnrichedLog netcoreapp3.1 4.56μs 3.28ns 12.7ns 0.0524 0 0 3.91 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 net472 2.23μs 2.36ns 9.15ns 0.217 0 0 1.37 KB
master SendReceive netcoreapp3.1 1.82μs 0.922ns 3.57ns 0.0174 0 0 1.32 KB
#3238 SendReceive net472 2.25μs 3.49ns 13.1ns 0.217 0 0 1.37 KB
#3238 SendReceive netcoreapp3.1 1.81μs 0.791ns 2.96ns 0.0179 0 0 1.32 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 net472 5.13μs 1.7ns 6.37ns 0.353 0 0 2.23 KB
master EnrichedLog netcoreapp3.1 4.33μs 1.36ns 5.08ns 0.0239 0 0 1.8 KB
#3238 EnrichedLog net472 4.98μs 1.84ns 7.14ns 0.353 0 0 2.23 KB
#3238 EnrichedLog netcoreapp3.1 4.32μs 1.38ns 5.18ns 0.0239 0 0 1.8 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 net472 1.24μs 0.732ns 2.74ns 0.129 0 0 810 B
master StartFinishSpan netcoreapp3.1 966ns 1.16ns 4.18ns 0.01 0 0 760 B
master StartFinishScope net472 1.41μs 1.06ns 4.12ns 0.141 0 0 891 B
master StartFinishScope netcoreapp3.1 1.05μs 0.383ns 1.38ns 0.0119 0 0 880 B
#3238 StartFinishSpan net472 1.18μs 0.55ns 2.13ns 0.129 0 0 810 B
#3238 StartFinishSpan netcoreapp3.1 974ns 0.874ns 3.27ns 0.0102 0 0 760 B
#3238 StartFinishScope net472 1.39μs 1.98ns 7.67ns 0.141 0 0 891 B
#3238 StartFinishScope netcoreapp3.1 1.06μs 0.441ns 1.71ns 0.0122 0 0 880 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 net472 1.51μs 1.08ns 4.17ns 0.141 0 0 891 B
master RunOnMethodBegin netcoreapp3.1 1.16μs 0.387ns 1.45ns 0.0122 0 0 880 B
#3238 RunOnMethodBegin net472 1.49μs 0.92ns 3.56ns 0.141 0 0 891 B
#3238 RunOnMethodBegin netcoreapp3.1 1.24μs 0.54ns 2.02ns 0.0121 0 0 880 B

@andrewlock

andrewlock commented Sep 21, 2022

Copy link
Copy Markdown
Member Author

Code Coverage Report 📊

⛔ Merging #3238 into master will will decrease line coverage by 5%
⚠️ Merging #3238 into master will will decrease branch coverage by 3%
⛔ Merging #3238 into master will will increase complexity by 24

master #3238 Change
Lines 17963 / 24889 16749 / 24914
Lines % 72% 67% -5% ⛔
Branches 10472 / 15254 9994 / 15278
Branches % 69% 65% -3% ⚠️
Complexity 16477 16501 24 ⛔

View the full report for further details:

Datadog.Trace Breakdown ⛔

master #3238 Change
Lines % 72% 67% -5% ⛔
Branches % 69% 65% -3% ⚠️
Complexity 16477 16501 24 ⛔

The following classes have significant coverage changes.

File Line coverage change Branch coverage change Complexity change
Datadog.Trace.Debugger.PInvoke.NativeMethodProbeDefinition -100% ⛔ -100% ⛔ 0 ✔️
Datadog.Trace.Debugger.Sink.SnapshotSink -100% ⛔ -100% ⛔ 0 ✔️
Datadog.Trace.Debugger.Snapshots.LazySnapshotSerializerFieldsAndPropsSelector -100% ⛔ -100% ⛔ 0 ✔️
Datadog.Trace.Debugger.Snapshots.DebuggerSnapshotCreator -100% ⛔ -68% ⛔ 1 ⛔
BoundLineProbeLocation -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.CapturedLines -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Instrumentation.DebuggerReturn -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Instrumentation.MethodMetadataInfo -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.Models.LineProbeResolveResult -100% ⛔ 0% ✔️ 0 ✔️
Datadog.Trace.Debugger.PInvoke.NativeLineProbeDefinition -100% ⛔ 0% ✔️ 0 ✔️
...And 49 more

View the full reports for further details:

@andrewlock
andrewlock merged commit 5852d82 into master Sep 22, 2022
@andrewlock
andrewlock deleted the andrew/add-content-encoding-to-api-request branch September 22, 2022 09:09
@github-actions github-actions Bot added this to the vNext milestone Sep 22, 2022
@andrewlock andrewlock mentioned this pull request Sep 22, 2022
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bouwkast bouwkast bouwkast approved these changes
@pierotibou pierotibou pierotibou approved these changes

Assignees

No one assigned

Labels

area:data-streams-monitoring area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations)

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

3 participants

@andrewlock @bouwkast @pierotibou