Open andrewlock opened 4 days ago
Branch report: andrew/fix-content-encoding
Commit report: d586ade
Test service: dd-trace-dotnet
:white_check_mark: 0 Failed, 344307 Passed, 1839 Skipped, 16h 9m 14.68s Total Time :snowflake: 6 New Flaky
TestApiSecurityScan
- Datadog.Trace.Security.IntegrationTests.ApiSecurity.AspNetCore5ApiSecurityDisabled
- Last Failure
TestApiSecurityScan
- Datadog.Trace.Security.IntegrationTests.ApiSecurity.AspNetCore5ApiSecurityDisabled
- Last Failure
TestApiSecurityScan
- Datadog.Trace.Security.IntegrationTests.ApiSecurity.AspNetCore5ApiSecurityDisabled
- Last Failure
TestSecurityInitialization
- Datadog.Trace.Security.IntegrationTests.AspNetCore5AsmInitializationSecurityEnabledWithBadRuleset
- Last Failure
TestDirectoryListingLeak
- Datadog.Trace.Security.IntegrationTests.Iast.AspNetCore5IastTestsRestartedSampleIastEnabled
- Last Failure
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.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5748) - mean (75ms) : 64, 86
. : milestone, 75,
master - mean (72ms) : 63, 81
. : milestone, 72,
section CallTarget+Inlining+NGEN
This PR (5748) - mean (908ms) : 886, 930
. : milestone, 908,
master - mean (895ms) : 874, 916
. : milestone, 895,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5748) - mean (109ms) : 106, 112
. : milestone, 109,
master - mean (109ms) : 106, 113
. : milestone, 109,
section CallTarget+Inlining+NGEN
This PR (5748) - mean (632ms) : 616, 649
. : milestone, 632,
master - mean (634ms) : 618, 651
. : milestone, 634,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5748) - mean (93ms) : 90, 97
. : milestone, 93,
master - mean (93ms) : 90, 96
. : milestone, 93,
section CallTarget+Inlining+NGEN
This PR (5748) - mean (591ms) : 578, 603
. : milestone, 591,
master - mean (591ms) : 572, 610
. : milestone, 591,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5748) - mean (192ms) : 187, 196
. : milestone, 192,
master - mean (192ms) : 188, 196
. : milestone, 192,
section CallTarget+Inlining+NGEN
This PR (5748) - mean (1,013ms) : 982, 1044
. : milestone, 1013,
master - mean (1,003ms) : 974, 1031
. : milestone, 1003,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5748) - mean (276ms) : 271, 281
. : milestone, 276,
master - mean (275ms) : 270, 280
. : milestone, 275,
section CallTarget+Inlining+NGEN
This PR (5748) - mean (825ms) : 793, 857
. : milestone, 825,
master - mean (823ms) : 791, 855
. : milestone, 823,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5748) - mean (266ms) : 262, 270
. : milestone, 266,
master - mean (264ms) : 260, 269
. : milestone, 264,
section CallTarget+Inlining+NGEN
This PR (5748) - mean (804ms) : 777, 832
. : milestone, 804,
master - mean (811ms) : 780, 843
. : milestone, 811,
Benchmarks for #5748 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored.
Throughput results for AspNetCoreSimpleController comparing the following branches/commits:
Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.
Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!
gantt
title Throughput Linux x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5748) (11.786M) : 0, 11786250
master (11.941M) : 0, 11940953
benchmarks/2.9.0 (11.959M) : 0, 11959218
section Automatic
This PR (5748) (7.816M) : 0, 7815578
master (8.126M) : 0, 8125742
benchmarks/2.9.0 (8.424M) : 0, 8423539
section Trace stats
master (8.463M) : 0, 8462913
section Manual
This PR (5748) (9.964M) : 0, 9963602
master (10.295M) : 0, 10294729
section Manual + Automatic
This PR (5748) (7.445M) : 0, 7444724
master (7.548M) : 0, 7547543
section Version Conflict
master (6.848M) : 0, 6848367
gantt
title Throughput Linux arm64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5748) (9.465M) : 0, 9465234
master (9.671M) : 0, 9670503
benchmarks/2.9.0 (9.647M) : 0, 9646678
section Automatic
This PR (5748) (6.587M) : 0, 6586999
master (6.513M) : 0, 6513280
section Trace stats
master (6.824M) : 0, 6824339
section Manual
This PR (5748) (8.306M) : 0, 8305974
master (8.189M) : 0, 8189017
section Manual + Automatic
This PR (5748) (6.228M) : 0, 6228140
master (6.236M) : 0, 6235985
section Version Conflict
master (5.631M) : 0, 5631298
gantt
title Throughput Windows x64 (Total requests)
dateFormat X
axisFormat %s
section Baseline
This PR (5748) (10.295M) : 0, 10294723
master (10.108M) : 0, 10107819
benchmarks/2.9.0 (10.154M) : 0, 10153990
section Automatic
This PR (5748) (7.275M) : 0, 7275260
master (7.230M) : 0, 7230329
benchmarks/2.9.0 (7.563M) : 0, 7562893
section Trace stats
master (7.514M) : 0, 7513930
section Manual
This PR (5748) (9.020M) : 0, 9019619
master (8.979M) : 0, 8979416
section Manual + Automatic
This PR (5748) (7.071M) : 0, 7070638
master (6.821M) : 0, 6820517
section Version Conflict
master (6.181M) : 0, 6181437
LGTM, noticed the tests that broke for the latest run so was curious how you identify this bug or how users would've been affected by it.
I was working on #5747, realised I needed to know if the content-encoding header had a value, and noticed the discrepancy everywhere. I don't think anything should actually be impacted by this currently, but it's hard to be 100%
Summary of changes
IApiResponse.ContentEncoding
property was confusing and incorrectly implementedGetCharsetEncoding()
GetContentEncodingType
Reason for change
The
ContentEncoding
property was meant to return thecharset
associated with aContent-type
i.e. thecharset=utf-8
part oftext/plain;charset=utf-8
, converted into a .NETEncoding
object. However, there's also aContentEncoding
header which defines whether the content is encoded with a compression algorithm, e.g. gzip/brotli etc.This PR aims to fix the bug, clear up the confusion by renaming, add some unit tests, and make the behaviour consistent across our various
IApiResponse
implementations.Implementation details
ContentTypeHeader
andContentEncodingHeader
as values on theIApiResponse
.GetHeaders
in some cases (e.g.HttpClient
)ContentEncoding
toGetCharsetEncoding()
HttpMessage
where possible. Tweaked it slightly to always return UTF-8 by default (as our calling code wasn't resistant to it anyway and would have thrown)HttpClientResponse
GetContentEncodingType()
which returns an enum of compression valuesTest coverage
Added unit tests for the parsing logic used for decoding the
charset
and the content-encodingOther details
I initially based this on https://github.com/DataDog/dd-trace-dotnet/pull/5658 but ultimately changed a lot (and we still don't need that mime-type handling yet)