[SymDB] Fixed SymDB multipart event.json#8194
Merged
dudikeleti merged 2 commits intomasterfrom Feb 13, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes SymDB multipart event.json generation so the payload is always valid JSON, and adds a unit test to validate the generated metadata (including proper quoting/escaping).
Changes:
- Replaced string-interpolated JSON with a
JsonTextWriter-based JSON writer forevent.json. - Introduced
CreateEventMetadata(serviceName, runtimeId)to centralize metadata generation. - Added a unit test to validate JSON parsing and expected fields/values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tracer/src/Datadog.Trace/Debugger/Upload/SymbolUploadApi.cs | Generates event.json via JsonTextWriter to ensure valid JSON and proper quoting. |
| tracer/test/Datadog.Trace.Tests/Debugger/SymbolsTests/SymbolUploadApiTests.cs | Adds test coverage to ensure the metadata payload parses as JSON and contains expected fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+34
| var dict = JsonConvert.DeserializeObject<Dictionary<string, object?>>(json); | ||
|
|
||
| Assert.NotNull(dict); | ||
| Assert.Equal("dd_debugger", dict!["ddsource"]); | ||
| Assert.Equal(serviceName, dict["service"]); | ||
| Assert.Equal(runtimeId, dict["runtimeId"]); | ||
| Assert.Equal("symdb", dict["debugger.type"]); |
There was a problem hiding this comment.
JsonConvert.DeserializeObject<Dictionary<string, object?>> will typically materialize values as JToken/JValue (because the target value type is object), so these equality assertions may fail even when the JSON is correct. To make the test reliable and validate quoting, consider parsing to a JObject and asserting both the value and token type (e.g., JTokenType.String for debugger.type), or deserialize to Dictionary<string, string> if all values are expected to be strings.
7be009c to
3ee5fb7
Compare
BenchmarksBenchmark execution time: 2026-02-12 20:35:52 Comparing candidate commit 3ee5fb7 in PR branch Found 7 performance improvements and 10 performance regressions! Performance is the same for 158 metrics, 17 unstable metrics. scenario:Benchmarks.Trace.AgentWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody net6.0
scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net6.0
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net6.0
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSlice net6.0
scenario:Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery net6.0
scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync net472
scenario:Benchmarks.Trace.ILoggerBenchmark.EnrichedLog net6.0
scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark net6.0
scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishScope netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishTwoScopes netcoreapp3.1
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8194) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-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 highlighted 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). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (70ms) : 68, 73
master - mean (69ms) : 67, 70
section Bailout
This PR (8194) - mean (75ms) : 73, 76
master - mean (72ms) : 71, 74
section CallTarget+Inlining+NGEN
This PR (8194) - mean (1,048ms) : 984, 1113
master - mean (1,032ms) : 991, 1072
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (117ms) : 114, 121
master - mean (114ms) : 111, 116
section Bailout
This PR (8194) - mean (118ms) : 116, 120
master - mean (114ms) : 113, 116
section CallTarget+Inlining+NGEN
This PR (8194) - mean (786ms) : 749, 823
master - mean (769ms) : 714, 824
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (104ms) : 100, 107
master - mean (100ms) : 97, 103
section Bailout
This PR (8194) - mean (104ms) : 103, 106
master - mean (101ms) : 100, 102
section CallTarget+Inlining+NGEN
This PR (8194) - mean (759ms) : 739, 780
master - mean (765ms) : 736, 793
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (96ms) : 93, 98
master - mean (92ms) : 90, 94
section Bailout
This PR (8194) - mean (97ms) : 95, 99
master - mean (93ms) : 91, 95
section CallTarget+Inlining+NGEN
This PR (8194) - mean (639ms) : 627, 652
master - mean (634ms) : 617, 651
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (206ms) : 194, 219
master - mean (214ms) : 196, 232
section Bailout
This PR (8194) - mean (209ms) : 197, 222
master - mean (215ms) : 198, 233
section CallTarget+Inlining+NGEN
This PR (8194) - mean (1,194ms) : 1131, 1256
master - mean (1,222ms) : 1155, 1290
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (307ms) : 289, 326
master - mean (315ms) : 294, 336
section Bailout
This PR (8194) - mean (310ms) : 291, 328
master - mean (318ms) : 295, 341
section CallTarget+Inlining+NGEN
This PR (8194) - mean (1,015ms) : 967, 1063
master - mean (1,043ms) : 977, 1108
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (289ms) : 272, 306
master - mean (299ms) : 276, 322
section Bailout
This PR (8194) - mean (290ms) : 272, 308
master - mean (298ms) : 279, 317
section CallTarget+Inlining+NGEN
This PR (8194) - mean (977ms) : 926, 1029
master - mean (997ms) : 938, 1055
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8194) - mean (301ms) : 282, 320
master - mean (309ms) : 276, 341
section Bailout
This PR (8194) - mean (304ms) : 283, 325
master - mean (305ms) : 283, 326
section CallTarget+Inlining+NGEN
This PR (8194) - mean (947ms) : 840, 1054
master - mean (1,008ms) : 910, 1107
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tylfin
approved these changes
Feb 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Fixed SymDB multipart event.json generation to always produce valid JSON.
Added a unit test validating the generated event.json payload.
Reason for change
The generated event.json was malformed ("debugger.type": symdb without quotes), which cause SymDB uploads to fail or be rejected by intake.
Test coverage
Added
SymbolUploadApiTests.EventMetadata_IsValidJson_AndDebuggerTypeIsQuotedto assert:Payload parses as JSON
ddsource, service, runtimeId, and debugger.type have expected values (including escaping behavior).