Skip to content
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

Fix ContentEncoding in IApiResponse #5748

Merged
merged 6 commits into from
Jul 3, 2024
Merged

Conversation

andrewlock
Copy link
Member

Summary of changes

  • The IApiResponse.ContentEncoding property was confusing and incorrectly implemented
  • Refactored as GetCharsetEncoding()
  • Added GetContentEncodingType

Reason for change

The ContentEncoding property was meant to return the charset associated with a Content-type i.e. the charset=utf-8 part of text/plain;charset=utf-8, converted into a .NET Encoding object. However, there's also a ContentEncoding 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

  • Expose ContentTypeHeader and ContentEncodingHeader as values on the IApiResponse.
    • This is necessary because you can't necessarily get these values directly from GetHeaders in some cases (e.g. HttpClient)
  • Rename ContentEncoding to GetCharsetEncoding()
    • Made it a method so we don't bother processing the header until we need it. In most cases we never use it (and use it at most once)
    • Re-used the optimized implementation we were using in 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)
    • Made it fallback to "any" charset, as was previously always happening for HttpClientResponse
  • Add GetContentEncodingType() which returns an enum of compression values

Test coverage

Added unit tests for the parsing logic used for decoding the charset and the content-encoding

Other details

I initially based this on #5658 but ultimately changed a lot (and we still don't need that mime-type handling yet)

@andrewlock andrewlock added type:bug area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) labels Jun 28, 2024
@andrewlock andrewlock requested a review from a team as a code owner June 28, 2024 10:53
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Jun 28, 2024

Datadog Report

Branch report: andrew/fix-content-encoding
Commit report: d586ade
Test service: dd-trace-dotnet

✅ 0 Failed, 344307 Passed, 1839 Skipped, 16h 9m 14.68s Total Time
❄️ 6 New Flaky

New Flaky Tests (6)

  • TestApiSecurityScan - Datadog.Trace.Security.IntegrationTests.ApiSecurity.AspNetCore5ApiSecurityDisabled - Last Failure

    Expand for error
     Unable to determine port application is listening on
    
  • TestApiSecurityScan - Datadog.Trace.Security.IntegrationTests.ApiSecurity.AspNetCore5ApiSecurityDisabled - Last Failure

    Expand for error
     Expected relevantSpans to contain at least 2 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.
    
  • TestApiSecurityScan - Datadog.Trace.Security.IntegrationTests.ApiSecurity.AspNetCore5ApiSecurityDisabled - Last Failure

    Expand for error
     Expected relevantSpans to contain at least 2 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.
    
  • TestSecurityInitialization - Datadog.Trace.Security.IntegrationTests.AspNetCore5AsmInitializationSecurityEnabledWithBadRuleset - Last Failure

    Expand for error
     Unable to determine port application is listening on
    
  • TestDirectoryListingLeak - Datadog.Trace.Security.IntegrationTests.Iast.AspNetCore5IastTestsRestartedSampleIastEnabled - Last Failure

    Expand for error
     Unable to determine port application is listening on
    

@andrewlock
Copy link
Member Author

andrewlock commented Jun 28, 2024

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:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

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,

Loading
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,

Loading
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,

Loading
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,

Loading
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,

Loading
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,

Loading

@andrewlock
Copy link
Member Author

andrewlock commented Jun 28, 2024

Benchmarks Report for tracer 🐌

Benchmarks for #5748 compared to master:

  • All benchmarks have the same speed
  • 1 benchmarks have more 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.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 7.85μs 43.2ns 248ns 0.0193 0.00773 0 5.42 KB
master StartStopWithChild netcoreapp3.1 10μs 55.4ns 354ns 0.0205 0.0102 0 5.61 KB
master StartStopWithChild net472 16.7μs 62.2ns 241ns 1.02 0.298 0.0994 6.06 KB
#5748 StartStopWithChild net6.0 7.88μs 42.6ns 237ns 0.0155 0.00775 0 5.42 KB
#5748 StartStopWithChild netcoreapp3.1 10.1μs 55ns 306ns 0.0154 0.00514 0 5.62 KB
#5748 StartStopWithChild net472 17.2μs 52.1ns 195ns 1.02 0.308 0.0942 6.07 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 482μs 195ns 755ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 620μs 188ns 727ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 830μs 294ns 1.14μs 0.408 0 0 3.3 KB
#5748 WriteAndFlushEnrichedTraces net6.0 483μs 274ns 1.03μs 0 0 0 2.7 KB
#5748 WriteAndFlushEnrichedTraces netcoreapp3.1 643μs 292ns 1.13μs 0 0 0 2.7 KB
#5748 WriteAndFlushEnrichedTraces net472 822μs 155ns 601ns 0.408 0 0 3.3 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 172μs 187ns 726ns 0.169 0 0 18.45 KB
master SendRequest netcoreapp3.1 190μs 275ns 1.03μs 0.191 0 0 20.61 KB
master SendRequest net472 0.00135ns 0.000409ns 0.00159ns 0 0 0 0 b
#5748 SendRequest net6.0 171μs 157ns 609ns 0.253 0 0 18.45 KB
#5748 SendRequest netcoreapp3.1 192μs 200ns 748ns 0.188 0 0 20.61 KB
#5748 SendRequest net472 0.00134ns 0.000377ns 0.00146ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #5748

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.56 KB 41.81 KB 254 B 0.61%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 549μs 529ns 2.05μs 0.553 0 0 41.56 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 647μs 1.57μs 6.1μs 0.322 0 0 41.79 KB
master WriteAndFlushEnrichedTraces net472 856μs 4.14μs 18μs 8.19 2.59 0.431 53.24 KB
#5748 WriteAndFlushEnrichedTraces net6.0 548μs 405ns 1.52μs 0.553 0 0 41.81 KB
#5748 WriteAndFlushEnrichedTraces netcoreapp3.1 666μs 1.26μs 4.87μs 0.332 0 0 41.77 KB
#5748 WriteAndFlushEnrichedTraces net472 861μs 3.94μs 15.3μs 8.45 2.53 0.422 53.38 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.2μs 0.606ns 2.27ns 0.0143 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.66μs 1.01ns 3.89ns 0.0138 0 0 1.02 KB
master ExecuteNonQuery net472 1.92μs 0.803ns 2.9ns 0.156 0 0 987 B
#5748 ExecuteNonQuery net6.0 1.32μs 0.496ns 1.79ns 0.0145 0 0 1.02 KB
#5748 ExecuteNonQuery netcoreapp3.1 1.62μs 0.993ns 3.85ns 0.0137 0 0 1.02 KB
#5748 ExecuteNonQuery net472 1.9μs 0.971ns 3.63ns 0.157 0 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.15μs 0.472ns 1.77ns 0.0138 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.49μs 0.57ns 2.21ns 0.0133 0 0 976 B
master CallElasticsearch net472 2.52μs 2.43ns 9.4ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.27μs 0.771ns 2.99ns 0.0134 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.61μs 0.757ns 2.83ns 0.0137 0 0 1.02 KB
master CallElasticsearchAsync net472 2.59μs 1.34ns 5.21ns 0.166 0 0 1.05 KB
#5748 CallElasticsearch net6.0 1.13μs 3.27ns 12.7ns 0.0135 0 0 976 B
#5748 CallElasticsearch netcoreapp3.1 1.55μs 1.08ns 4.16ns 0.0133 0 0 976 B
#5748 CallElasticsearch net472 2.53μs 2.22ns 8.62ns 0.157 0 0 995 B
#5748 CallElasticsearchAsync net6.0 1.24μs 0.391ns 1.46ns 0.0129 0 0 952 B
#5748 CallElasticsearchAsync netcoreapp3.1 1.64μs 0.527ns 1.9ns 0.0139 0 0 1.02 KB
#5748 CallElasticsearchAsync net472 2.67μs 1.11ns 3.99ns 0.167 0 0 1.05 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.31μs 3.73ns 14.5ns 0.0135 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.58μs 0.964ns 3.61ns 0.0127 0 0 952 B
master ExecuteAsync net472 1.83μs 0.751ns 2.91ns 0.145 0 0 915 B
#5748 ExecuteAsync net6.0 1.29μs 0.853ns 3.3ns 0.0129 0 0 952 B
#5748 ExecuteAsync netcoreapp3.1 1.6μs 1.05ns 3.93ns 0.0128 0 0 952 B
#5748 ExecuteAsync net472 1.78μs 0.938ns 3.51ns 0.145 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 4.17μs 2.1ns 7.87ns 0.03 0 0 2.22 KB
master SendAsync netcoreapp3.1 5.07μs 4.24ns 16.4ns 0.0355 0 0 2.76 KB
master SendAsync net472 7.63μs 2.83ns 10.9ns 0.496 0 0 3.15 KB
#5748 SendAsync net6.0 4.15μs 2.99ns 11.6ns 0.0312 0 0 2.22 KB
#5748 SendAsync netcoreapp3.1 5.15μs 3.2ns 12.4ns 0.0361 0 0 2.76 KB
#5748 SendAsync net472 7.75μs 2.95ns 11.4ns 0.497 0 0 3.15 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 1.43μs 1.03ns 4ns 0.0231 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.13μs 1.15ns 4.32ns 0.0217 0 0 1.64 KB
master EnrichedLog net472 2.43μs 0.87ns 3.26ns 0.25 0 0 1.57 KB
#5748 EnrichedLog net6.0 1.5μs 1.16ns 4.35ns 0.0232 0 0 1.64 KB
#5748 EnrichedLog netcoreapp3.1 2.25μs 1.47ns 5.31ns 0.0214 0 0 1.64 KB
#5748 EnrichedLog net472 2.58μs 1.21ns 4.69ns 0.25 0 0 1.57 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 113μs 144ns 556ns 0.0569 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 120μs 204ns 792ns 0.0596 0 0 4.28 KB
master EnrichedLog net472 146μs 73.9ns 267ns 0.655 0.218 0 4.46 KB
#5748 EnrichedLog net6.0 116μs 245ns 949ns 0 0 0 4.28 KB
#5748 EnrichedLog netcoreapp3.1 119μs 79.2ns 286ns 0 0 0 4.28 KB
#5748 EnrichedLog net472 147μs 196ns 732ns 0.659 0.22 0 4.46 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 3.07μs 1ns 3.89ns 0.0311 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.09μs 1.17ns 4.39ns 0.0305 0 0 2.2 KB
master EnrichedLog net472 4.75μs 1.96ns 7.61ns 0.32 0 0 2.02 KB
#5748 EnrichedLog net6.0 2.88μs 0.844ns 3.16ns 0.0315 0 0 2.2 KB
#5748 EnrichedLog netcoreapp3.1 4.09μs 1.96ns 7.32ns 0.0306 0 0 2.2 KB
#5748 EnrichedLog net472 4.88μs 0.899ns 3.24ns 0.321 0 0 2.02 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 1.32μs 0.358ns 1.34ns 0.0158 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.67μs 4.75ns 18.4ns 0.015 0 0 1.14 KB
master SendReceive net472 2.14μs 1.36ns 5.27ns 0.183 0.00107 0 1.16 KB
#5748 SendReceive net6.0 1.29μs 0.343ns 1.28ns 0.0161 0 0 1.14 KB
#5748 SendReceive netcoreapp3.1 1.76μs 1.29ns 5.01ns 0.015 0 0 1.14 KB
#5748 SendReceive net472 1.99μs 1.62ns 6.26ns 0.183 0 0 1.16 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 2.64μs 0.668ns 2.5ns 0.0224 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.82μs 1.25ns 4.84ns 0.0211 0 0 1.65 KB
master EnrichedLog net472 4.35μs 3.34ns 12.9ns 0.323 0 0 2.04 KB
#5748 EnrichedLog net6.0 2.73μs 3.67ns 14.2ns 0.0218 0 0 1.6 KB
#5748 EnrichedLog netcoreapp3.1 3.86μs 0.909ns 3.52ns 0.0211 0 0 1.65 KB
#5748 EnrichedLog net472 4.48μs 2.33ns 9.04ns 0.323 0 0 2.04 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 410ns 0.0999ns 0.387ns 0.00799 0 0 576 B
master StartFinishSpan netcoreapp3.1 613ns 0.188ns 0.728ns 0.00771 0 0 576 B
master StartFinishSpan net472 680ns 0.281ns 1.09ns 0.0915 0 0 578 B
master StartFinishScope net6.0 479ns 0.15ns 0.579ns 0.00989 0 0 696 B
master StartFinishScope netcoreapp3.1 761ns 0.409ns 1.58ns 0.00951 0 0 696 B
master StartFinishScope net472 830ns 0.418ns 1.62ns 0.104 0 0 658 B
#5748 StartFinishSpan net6.0 407ns 0.11ns 0.427ns 0.00803 0 0 576 B
#5748 StartFinishSpan netcoreapp3.1 598ns 0.243ns 0.911ns 0.00782 0 0 576 B
#5748 StartFinishSpan net472 658ns 0.41ns 1.53ns 0.0915 0 0 578 B
#5748 StartFinishScope net6.0 496ns 0.211ns 0.788ns 0.00976 0 0 696 B
#5748 StartFinishScope netcoreapp3.1 734ns 0.24ns 0.9ns 0.00945 0 0 696 B
#5748 StartFinishScope net472 860ns 0.784ns 3.04ns 0.104 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 583ns 0.227ns 0.879ns 0.00963 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 927ns 0.212ns 0.765ns 0.00925 0 0 696 B
master RunOnMethodBegin net472 1.05μs 0.544ns 2.11ns 0.104 0 0 658 B
#5748 RunOnMethodBegin net6.0 642ns 0.176ns 0.658ns 0.0097 0 0 696 B
#5748 RunOnMethodBegin netcoreapp3.1 912ns 0.398ns 1.43ns 0.00953 0 0 696 B
#5748 RunOnMethodBegin net472 1.09μs 0.542ns 2.03ns 0.104 0 0 658 B

_response.Content.Headers.ContentEncoding.Count switch
{
0 => ContentEncodingType.None,
1 => ApiResponseExtensions.GetContentEncodingType(_response.Content.Headers.ContentEncoding.First()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit-picking, but it feels weird to call a non-extension method that lives in ApiResponseExtensions from here. Also that ApiResponseExtensions lives in a file called IApiResponse.cs, which I would expect to contain only that interface. Should we try to reorg the code a little? 😅 (maybe later)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(maybe later)

😺

@andrewlock
Copy link
Member Author

andrewlock commented Jun 28, 2024

Throughput/Crank Report:zap:

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

Loading
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

Loading
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

Loading

Copy link
Contributor

@link04 link04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

In an attempt to disambiguate with the ContentEncoding header
There's no reason we _should_ default to utf-8 if they've explicitly specified a charset
@andrewlock andrewlock force-pushed the andrew/fix-content-encoding branch from b4bfee4 to a1413b7 Compare July 2, 2024 10:57
@andrewlock
Copy link
Member Author

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%

@andrewlock andrewlock merged commit dae1d15 into master Jul 3, 2024
57 checks passed
@andrewlock andrewlock deleted the andrew/fix-content-encoding branch July 3, 2024 15:06
@github-actions github-actions bot added this to the vNext-v2 milestone Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants