Fix reading JSON-formatted settings in remote config by lucaspimentel · Pull Request #5705 · DataDog/dd-trace-dotnet

lucaspimentel · 2024-06-17T20:29:01Z

Summary of changes

Fix bug when reading JSON-like settings from remote config (RC).

Reason for change

Remote configuration (RC) sends inline JSON object for DD_TRACE_SAMPLING_RULES. We were expecting the setting's value to be an embedded JSON string:

{ "CONFIG_NAME": "[]" }

instead of an array of JSON objects

{ "CONFIG_NAME": [] }

This breaks RC when it fails to parse the setting's value.

Implementation details

Check JToken.Type, and convert the token to string depending on its type:

null for null-like types
JToken.Value<string>() for strings to get the underlying value
JToken.ToString() for anything else. This converts arrays or objects back into json, which is what we expect.

Test coverage

fixed unit and integration tests in this PR to expect objects from RC (parsed from json) instead of the json string itself
also tested with system tests, (which revealed the issue in the first place)

tests/parametric/test_dynamic_configuration.py::TestDynamicConfigSamplingRules

Other details

In the future, we should consider converting the JToken into a collection of sampling rules directly, instead of serializing the token into a json string only to deserialize it again later.

datadog-ddstaging · 2024-06-17T20:44:54Z

Datadog Report

Branch report: lpimentel/json-settings
Commit report: 1faf0f6
Test service: dd-trace-dotnet

✅ 0 Failed, 338748 Passed, 1675 Skipped, 14h 12m 13.26s Total Time

andrewlock · 2024-06-17T20:54:15Z

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 (5705) - mean (73ms)  : 63, 82
     .   : milestone, 73,
    master - mean (76ms)  : 64, 89
     .   : milestone, 76,

    section CallTarget+Inlining+NGEN
    This PR (5705) - mean (999ms)  : 979, 1019
     .   : milestone, 999,
    master - mean (1,005ms)  : 983, 1026
     .   : milestone, 1005,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5705) - mean (108ms)  : 106, 111
     .   : milestone, 108,
    master - mean (110ms)  : 107, 113
     .   : milestone, 110,

    section CallTarget+Inlining+NGEN
    This PR (5705) - mean (713ms)  : 693, 732
     .   : milestone, 713,
    master - mean (715ms)  : 694, 737
     .   : milestone, 715,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5705) - mean (93ms)  : 91, 96
     .   : milestone, 93,
    master - mean (94ms)  : 91, 97
     .   : milestone, 94,

    section CallTarget+Inlining+NGEN
    This PR (5705) - mean (668ms)  : 639, 698
     .   : milestone, 668,
    master - mean (669ms)  : 650, 688
     .   : milestone, 669,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5705) - mean (192ms)  : 188, 195
     .   : milestone, 192,
    master - mean (194ms)  : 191, 198
     .   : milestone, 194,

    section CallTarget+Inlining+NGEN
    This PR (5705) - mean (1,103ms)  : 1080, 1126
     .   : milestone, 1103,
    master - mean (1,103ms)  : 1081, 1125
     .   : milestone, 1103,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5705) - mean (277ms)  : 272, 281
     .   : milestone, 277,
    master - mean (278ms)  : 274, 283
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (5705) - mean (906ms)  : 880, 932
     .   : milestone, 906,
    master - mean (898ms)  : 865, 932
     .   : milestone, 898,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5705) - mean (268ms)  : 261, 275
     .   : milestone, 268,
    master - mean (267ms)  : 263, 272
     .   : milestone, 267,

    section CallTarget+Inlining+NGEN
    This PR (5705) - mean (883ms)  : 854, 912
     .   : milestone, 883,
    master - mean (886ms)  : 859, 912
     .   : milestone, 886,

andrewlock · 2024-06-17T21:18:28Z

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 (5705) (11.388M)   : 0, 11387768
    master (11.536M)   : 0, 11535574
    benchmarks/2.9.0 (11.490M)   : 0, 11490473

    section Automatic
    This PR (5705) (7.710M)   : 0, 7710148
    master (7.699M)   : 0, 7699118
    benchmarks/2.9.0 (8.179M)   : 0, 8178774

    section Trace stats
    master (8.113M)   : 0, 8112968

    section Manual
    This PR (5705) (9.862M)   : 0, 9862412
    master (9.964M)   : 0, 9963530

    section Manual + Automatic
    This PR (5705) (7.229M)   : 0, 7228895
    master (7.235M)   : 0, 7234758

    section Version Conflict
    master (6.531M)   : 0, 6531478

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5705) (9.380M)   : 0, 9379548
    master (9.573M)   : 0, 9573109
    benchmarks/2.9.0 (9.629M)   : 0, 9629333

    section Automatic
    This PR (5705) (6.296M)   : 0, 6295754
    master (6.555M)   : 0, 6555285

    section Trace stats
    master (6.865M)   : 0, 6864828

    section Manual
    This PR (5705) (8.219M)   : 0, 8218600
    master (8.134M)   : 0, 8133726

    section Manual + Automatic
    This PR (5705) (6.219M)   : 0, 6218658
    master (6.017M)   : 0, 6017202

    section Version Conflict
    master (5.669M)   : 0, 5669446

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5705) (10.158M)   : 0, 10158030
    master (10.213M)   : 0, 10213235
    benchmarks/2.9.0 (10.250M)   : 0, 10250109

    section Automatic
    This PR (5705) (7.224M)   : 0, 7223584
    master (7.199M)   : 0, 7199124
    benchmarks/2.9.0 (7.524M)   : 0, 7523890

    section Trace stats
    master (7.535M)   : 0, 7535310

    section Manual
    This PR (5705) (9.055M)   : 0, 9055457
    master (8.961M)   : 0, 8961460

    section Manual + Automatic
    This PR (5705) (7.045M)   : 0, 7044973
    master (6.961M)   : 0, 6961030

    section Version Conflict
    master (6.283M)   : 0, 6282755

zacharycmontoya · 2024-06-17T21:46:26Z

                // RuntimeMetricsEnabled = settings.WithKeys(ConfigurationKeys.RuntimeMetricsEnabled).AsBool(),
                // DataStreamsMonitoringEnabled = settings.WithKeys(ConfigurationKeys.DataStreamsMonitoring.Enabled).AsBool(),
-                SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).AsString(),
+                SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAs<string>(null, null, s => s),


It's too bad we don't have a strongly typed way to convert the JArray into a result, but stringifying it right now can solve the problem in the short-term so 👍🏼

zacharycmontoya

AAAS (approval as a service) 😆

andrewlock · 2024-06-17T22:21:44Z

Benchmarks Report for tracer 🐌

Benchmarks for #5705 compared to master:

1 benchmarks are faster, with geometric mean 1.136
2 benchmarks are slower, with geometric mean 1.132
1 benchmarks have fewer 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.65μs	43.3ns	306ns	0.0248	0.0124	0	5.43 KB
master	`StartStopWithChild`	netcoreapp3.1	10.2μs	54.1ns	281ns	0.0149	0.00498	0	5.62 KB
master	`StartStopWithChild`	net472	16μs	46.8ns	181ns	1.02	0.304	0.096	6.05 KB
#5705	`StartStopWithChild`	net6.0	7.78μs	42.6ns	263ns	0.0148	0.00369	0	5.42 KB
#5705	`StartStopWithChild`	netcoreapp3.1	9.85μs	54.3ns	322ns	0.0188	0.00941	0	5.62 KB
#5705	`StartStopWithChild`	net472	16μs	42.4ns	164ns	1.02	0.302	0.103	6.08 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	490μs	298ns	1.15μs	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	624μs	187ns	700ns	0	2.7 KB
master	`WriteAndFlushEnrichedTraces`	net472	825μs	358ns	1.39μs	0.411	3.3 KB
#5705	`WriteAndFlushEnrichedTraces`	net6.0	475μs	432ns	1.62μs	0	2.7 KB
#5705	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	641μs	196ns	757ns	0	2.7 KB
#5705	`WriteAndFlushEnrichedTraces`	net472	836μs	237ns	917ns	0.417	3.3 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	170μs	256ns	990ns	0.254	18.44 KB
master	`SendRequest`	netcoreapp3.1	190μs	369ns	1.43μs	0.19	20.6 KB
master	`SendRequest`	net472	0.000422ns	0.000229ns	0.000856ns	0	0 b
#5705	`SendRequest`	net6.0	169μs	177ns	684ns	0.169	18.44 KB
#5705	`SendRequest`	netcoreapp3.1	191μs	419ns	1.62μs	0.189	20.6 KB
#5705	`SendRequest`	net472	5.21E‑05ns	5.21E‑05ns	0.000195ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #5705

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0	41.69 KB	41.41 KB	-276 B	-0.66%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	562μs	273ns	1.06μs	0.558	0	0	41.69 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	650μs	1.1μs	3.98μs	0.322	0	0	41.68 KB
master	`WriteAndFlushEnrichedTraces`	net472	859μs	3.68μs	14.3μs	8.39	2.52	0.419	53.25 KB
#5705	`WriteAndFlushEnrichedTraces`	net6.0	536μs	547ns	2.12μs	0.541	0	0	41.41 KB
#5705	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	644μs	1.34μs	5.18μs	0.322	0	0	41.8 KB
#5705	`WriteAndFlushEnrichedTraces`	net472	824μs	1.93μs	6.94μs	8.28	2.48	0.414	53.22 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`ExecuteNonQuery`	net6.0	1.09μs	0.25ns	0.937ns	0.0115	808 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.52μs	1.21ns	4.68ns	0.0106	808 B
master	`ExecuteNonQuery`	net472	1.79μs	3.62ns	14ns	0.122	770 B
#5705	`ExecuteNonQuery`	net6.0	1.12μs	1.04ns	4.01ns	0.0112	808 B
#5705	`ExecuteNonQuery`	netcoreapp3.1	1.5μs	1.24ns	4.48ns	0.0104	808 B
#5705	`ExecuteNonQuery`	net472	1.74μs	0.699ns	2.61ns	0.122	770 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net6.0	1.22μs	0.365ns	1.41ns	0.0139	0	976 B
master	`CallElasticsearch`	netcoreapp3.1	1.53μs	0.744ns	2.78ns	0.0129	0	976 B
master	`CallElasticsearch`	net472	2.55μs	1.53ns	5.74ns	0.158	0	995 B
master	`CallElasticsearchAsync`	net6.0	1.26μs	0.417ns	1.62ns	0.0132	0	952 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.68μs	1.21ns	4.68ns	0.0133	0	1.02 KB
master	`CallElasticsearchAsync`	net472	2.59μs	0.994ns	3.72ns	0.166	0.00129	1.05 KB
#5705	`CallElasticsearch`	net6.0	1.11μs	2.42ns	9.04ns	0.0138	0	976 B
#5705	`CallElasticsearch`	netcoreapp3.1	1.59μs	0.624ns	2.42ns	0.0127	0	976 B
#5705	`CallElasticsearch`	net472	2.48μs	2.19ns	8.21ns	0.157	0.00125	995 B
#5705	`CallElasticsearchAsync`	net6.0	1.36μs	0.532ns	1.99ns	0.0129	0	952 B
#5705	`CallElasticsearchAsync`	netcoreapp3.1	1.6μs	0.697ns	2.7ns	0.0136	0	1.02 KB
#5705	`CallElasticsearchAsync`	net472	2.64μs	2.77ns	10.4ns	0.167	0	1.05 KB

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

Slower ⚠️ in #5705

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync‑net6.0	1.133	1,181.54	1,339.12

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteAsync`	net6.0	1.18μs	0.356ns	1.33ns	0.0136	0	952 B
master	`ExecuteAsync`	netcoreapp3.1	1.56μs	0.907ns	3.39ns	0.0126	0	952 B
master	`ExecuteAsync`	net472	1.84μs	0.271ns	1.01ns	0.145	0.000923	915 B
#5705	`ExecuteAsync`	net6.0	1.34μs	1.99ns	7.44ns	0.0133	0	952 B
#5705	`ExecuteAsync`	netcoreapp3.1	1.63μs	0.912ns	3.53ns	0.0131	0	952 B
#5705	`ExecuteAsync`	net472	1.79μs	2.39ns	8.95ns	0.145	0	915 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	4.3μs	1.16ns	4.48ns	0.0301	2.22 KB
master	`SendAsync`	netcoreapp3.1	5μs	2.12ns	7.65ns	0.0374	2.76 KB
master	`SendAsync`	net472	7.76μs	1.81ns	7ns	0.5	3.15 KB
#5705	`SendAsync`	net6.0	4.15μs	1.97ns	7.64ns	0.031	2.22 KB
#5705	`SendAsync`	netcoreapp3.1	4.93μs	1.35ns	5.23ns	0.037	2.76 KB
#5705	`SendAsync`	net472	7.63μs	1.47ns	5.52ns	0.499	3.15 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.47μs	0.662ns	2.48ns	0.0235	1.64 KB
master	`EnrichedLog`	netcoreapp3.1	2.21μs	1.33ns	4.96ns	0.0221	1.64 KB
master	`EnrichedLog`	net472	2.42μs	1.08ns	4.2ns	0.249	1.57 KB
#5705	`EnrichedLog`	net6.0	1.44μs	0.708ns	2.65ns	0.023	1.64 KB
#5705	`EnrichedLog`	netcoreapp3.1	2.08μs	0.815ns	3.05ns	0.0227	1.64 KB
#5705	`EnrichedLog`	net472	2.61μs	3.33ns	12.9ns	0.249	1.57 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	115μs	187ns	725ns	0.0564	0	4.28 KB
master	`EnrichedLog`	netcoreapp3.1	117μs	138ns	536ns	0	0	4.28 KB
master	`EnrichedLog`	net472	146μs	66.1ns	247ns	0.656	0.219	4.46 KB
#5705	`EnrichedLog`	net6.0	113μs	163ns	630ns	0.0563	0	4.28 KB
#5705	`EnrichedLog`	netcoreapp3.1	120μs	265ns	991ns	0.0605	0	4.28 KB
#5705	`EnrichedLog`	net472	148μs	155ns	578ns	0.663	0.221	4.46 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	3.15μs	0.882ns	3.42ns	0.0298	2.2 KB
master	`EnrichedLog`	netcoreapp3.1	4.14μs	0.63ns	2.44ns	0.029	2.2 KB
master	`EnrichedLog`	net472	4.88μs	1.71ns	6.63ns	0.32	2.02 KB
#5705	`EnrichedLog`	net6.0	3.15μs	0.605ns	2.34ns	0.03	2.2 KB
#5705	`EnrichedLog`	netcoreapp3.1	4.11μs	1.76ns	6.82ns	0.0287	2.2 KB
#5705	`EnrichedLog`	net472	4.8μs	1.97ns	7.37ns	0.319	2.02 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net6.0	1.34μs	0.709ns	2.65ns	0.0162	1.14 KB
master	`SendReceive`	netcoreapp3.1	1.8μs	0.979ns	3.79ns	0.0155	1.14 KB
master	`SendReceive`	net472	2.02μs	0.945ns	3.66ns	0.183	1.16 KB
#5705	`SendReceive`	net6.0	1.26μs	0.376ns	1.41ns	0.0158	1.14 KB
#5705	`SendReceive`	netcoreapp3.1	1.73μs	2.01ns	6.97ns	0.0156	1.14 KB
#5705	`SendReceive`	net472	2.04μs	4.76ns	18.4ns	0.184	1.16 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.88μs	0.747ns	2.89ns	0.0231	1.6 KB
master	`EnrichedLog`	netcoreapp3.1	3.88μs	0.807ns	3.02ns	0.0214	1.65 KB
master	`EnrichedLog`	net472	4.28μs	1.93ns	7.47ns	0.323	2.04 KB
#5705	`EnrichedLog`	net6.0	2.67μs	0.79ns	3.06ns	0.0227	1.6 KB
#5705	`EnrichedLog`	netcoreapp3.1	3.96μs	1.16ns	4.48ns	0.0217	1.65 KB
#5705	`EnrichedLog`	net472	4.48μs	3.7ns	14.3ns	0.322	2.04 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	392ns	0.162ns	0.628ns	0.00808	576 B
master	`StartFinishSpan`	netcoreapp3.1	638ns	0.251ns	0.971ns	0.00782	576 B
master	`StartFinishSpan`	net472	630ns	0.661ns	2.56ns	0.0918	578 B
master	`StartFinishScope`	net6.0	488ns	0.191ns	0.739ns	0.00979	696 B
master	`StartFinishScope`	netcoreapp3.1	718ns	0.892ns	3.45ns	0.00942	696 B
master	`StartFinishScope`	net472	873ns	0.575ns	2.23ns	0.105	658 B
#5705	`StartFinishSpan`	net6.0	391ns	0.214ns	0.829ns	0.00806	576 B
#5705	`StartFinishSpan`	netcoreapp3.1	579ns	0.341ns	1.32ns	0.00777	576 B
#5705	`StartFinishSpan`	net472	635ns	0.442ns	1.71ns	0.0917	578 B
#5705	`StartFinishScope`	net6.0	480ns	0.248ns	0.961ns	0.00971	696 B
#5705	`StartFinishScope`	netcoreapp3.1	760ns	0.32ns	1.2ns	0.00952	696 B
#5705	`StartFinishScope`	net472	889ns	1.11ns	4.28ns	0.105	658 B

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

Slower ⚠️ in #5705

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0	1.130	589.29	665.85

Faster 🎉 in #5705

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net472	1.136	1,139.65	1,003.13

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	589ns	0.243ns	0.908ns	0.00972	696 B
master	`RunOnMethodBegin`	netcoreapp3.1	883ns	0.5ns	1.87ns	0.00937	696 B
master	`RunOnMethodBegin`	net472	1.14μs	0.683ns	2.64ns	0.104	658 B
#5705	`RunOnMethodBegin`	net6.0	666ns	0.615ns	2.38ns	0.00971	696 B
#5705	`RunOnMethodBegin`	netcoreapp3.1	969ns	1.09ns	4.2ns	0.0096	696 B
#5705	`RunOnMethodBegin`	net472	1μs	0.63ns	2.44ns	0.104	658 B

bouwkast

Looks good to me after the unit tests are fixed :)

* fix typo * clean up usings * check token type and use ToString() * use Formatting.None * use GetAs<string>() instead of AsString() * use object instead of string as config in unit test * log the json object instead of the string * use ToString() instead of Value<String>() * parse json object instead of passing string * use same logic to convert JToken to string * reduce code duplication * fix compiler error

fix typo

ecebd39

lucaspimentel added type:bug area:remote-config labels Jun 17, 2024

github-actions Bot added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Jun 17, 2024

lucaspimentel changed the title ~~Fix reading JSON-formatted settings in JsonConfigurationSource~~ Fix reading JSON-formatted settings in remote config Jun 17, 2024

zacharycmontoya reviewed Jun 17, 2024

View reviewed changes

zacharycmontoya approved these changes Jun 17, 2024

View reviewed changes

lucaspimentel marked this pull request as ready for review June 17, 2024 21:50

lucaspimentel requested a review from a team as a code owner June 17, 2024 21:50

lucaspimentel requested a review from a team June 17, 2024 21:50

zacharycmontoya approved these changes Jun 17, 2024

View reviewed changes

bouwkast approved these changes Jun 17, 2024

View reviewed changes

andrewlock reviewed Jun 18, 2024

View reviewed changes

Comment thread tracer/src/Datadog.Trace/Configuration/ConfigurationSources/JsonConfigurationSource.cs Outdated

lucaspimentel added 2 commits June 18, 2024 09:42

clean up usings

f2fcc64

check token type and use ToString()

d0b7e98

lucaspimentel force-pushed the lpimentel/json-settings branch from 1aae2b4 to d0b7e98 Compare June 18, 2024 13:43

lucaspimentel added 9 commits June 18, 2024 11:54

use Formatting.None

8ba08d2

use GetAs<string>() instead of AsString()

77ee92e

use object instead of string as config in unit test

318055f

log the json object instead of the string

407606e

use ToString() instead of Value<String>()

518d160

parse json object instead of passing string

b89c7fc

use same logic to convert JToken to string

42e857e

reduce code duplication

966e4c5

fix compiler error

1faf0f6

zacharycmontoya approved these changes Jun 18, 2024

View reviewed changes

lucaspimentel merged commit 8a14986 into master Jun 18, 2024

lucaspimentel deleted the lpimentel/json-settings branch June 18, 2024 20:33

github-actions Bot added this to the vNext-v2 milestone Jun 18, 2024

andrewlock modified the milestones: vNext-v2, 2.53.2 Jun 19, 2024

Uh oh!

Conversation

lucaspimentel commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

Uh oh!

datadog-ddstaging Bot commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

andrewlock commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Execution-Time Benchmarks Report ⏱️

Uh oh!

andrewlock commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Throughput/Crank Report:zap:

Uh oh!

zacharycmontoya Jun 17, 2024

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Report for tracer 🐌

Benchmark details

Raw results

Raw results

Raw results

Fewer allocations 🎉 in #5705

Raw results

Raw results

Raw results

Slower ⚠️ in #5705

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #5705

Faster 🎉 in #5705

Raw results

Uh oh!

bouwkast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lucaspimentel commented Jun 17, 2024 •

edited

Loading

datadog-ddstaging Bot commented Jun 17, 2024 •

edited

Loading

andrewlock commented Jun 17, 2024 •

edited

Loading

andrewlock commented Jun 17, 2024 •

edited

Loading

andrewlock commented Jun 17, 2024 •

edited

Loading