[CI Visibility] - Enable snapshot testing of current testing framework implementations by tonyredondo · Pull Request #5226 · DataDog/dd-trace-dotnet

tonyredondo · 2024-02-22T15:43:11Z

Summary of changes

This PR adds snapshot testing on the testing framework integration tests.

Reason for change

There's some new features for the testing framework instrumentation and I want to make sure I don't change the existing behavior in the output of those integrations.

andrewlock

LGTM in principal, but I suggest:

You scrub the library version
You do a run that sets RunAllTestFrameworks
You do three "comprehensive" runs, one for each framework, so that you test against all package versions

tracer/test/snapshots/MsTestV2Tests.SubmitTraces_packageVersion=.verified.txt

tracer/test/snapshots/XUnitTests.SubmitTraces_packageVersion=2.6.6.verified.txt

tracer/src/Datadog.Trace/Ci/TestSession.cs

andrewlock · 2024-03-11T13:33:53Z

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/MsTestV2EvpTests.cs


                    using (ProcessResult processResult = await RunDotnetTestSampleAndWaitForExit(agent, packageVersion: packageVersion))
                    {
+                        var settings = VerifyHelper.GetCIVisibilitySpanVerifierSettings(expectedTestCount == 15 ? "old" : "new", null, null);


I would advice not using "old" and "new" unless you want "new new" and "newer" to be suffixes down the line 😛 You'd be better off with things like Pre_2 etc

I'd also suggest you don't base it on the expectedTestCount, but instead have a bool/enum that controls both the number of exepected tests and the suffix. Otherwise, when you add a new test later to check some functionality you're going to have to remember to update all these

changed the name in 1a39b53

andrewlock · 2024-03-11T13:36:37Z

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/CI/TestingFrameworkTest.cs

+// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
+// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
+// </copyright>
+#nullable enable


Just FYI, we don't mandate #nullable enable in test code, as it tends to be more of a pain than it's worth. Not saying you can't, just FYI

andrewlock · 2024-03-11T13:39:07Z

...Tests.SubmitTraces_packageVersion=old_evpVersionToRemove=null_expectedGzip=null.verified.txt

I assume you've checked these, and that seeing them all as individual traces is what you expect:

…nvironment to avoid collision with System.Environment class

datadog-ddstaging · 2024-03-11T22:22:56Z

Datadog Report

Branch report: tony/civisibility-snapshot-testing
Commit report: 62c8796
Test service: dd-trace-dotnet

✅ 0 Failed, 330047 Passed, 1582 Skipped, 45m 58.22s Wall Time

andrewlock · 2024-03-11T22:31:53Z

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 (5226) - mean (75ms)  : 67, 84
     .   : milestone, 75,
    master - mean (74ms)  : 65, 83
     .   : milestone, 74,

    section CallTarget+Inlining+NGEN
    This PR (5226) - mean (995ms)  : 970, 1019
     .   : milestone, 995,
    master - mean (985ms)  : 961, 1008
     .   : milestone, 985,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5226) - mean (112ms)  : 108, 116
     .   : milestone, 112,
    master - mean (118ms)  : 96, 140
     .   : milestone, 118,

    section CallTarget+Inlining+NGEN
    This PR (5226) - mean (723ms)  : 701, 745
     .   : milestone, 723,
    master - mean (713ms)  : 691, 735
     .   : milestone, 713,

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

    section CallTarget+Inlining+NGEN
    This PR (5226) - mean (673ms)  : 647, 699
     .   : milestone, 673,
    master - mean (671ms)  : 647, 694
     .   : milestone, 671,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5226) - mean (188ms)  : 186, 191
     .   : milestone, 188,
    master - mean (188ms)  : 182, 193
     .   : milestone, 188,

    section CallTarget+Inlining+NGEN
    This PR (5226) - mean (1,075ms)  : 1049, 1100
     .   : milestone, 1075,
    master - mean (1,063ms)  : 1038, 1089
     .   : milestone, 1063,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5226) - mean (272ms)  : 268, 276
     .   : milestone, 272,
    master - mean (269ms)  : 262, 276
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (5226) - mean (869ms)  : 848, 890
     .   : milestone, 869,
    master - mean (861ms)  : 839, 884
     .   : milestone, 861,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5226) - mean (261ms)  : 258, 265
     .   : milestone, 261,
    master - mean (258ms)  : 250, 265
     .   : milestone, 258,

    section CallTarget+Inlining+NGEN
    This PR (5226) - mean (860ms)  : 839, 881
     .   : milestone, 860,
    master - mean (855ms)  : 834, 876
     .   : milestone, 855,

commit 832de4b Author: Flavien Darche <11708575+e-n-0@users.noreply.github.com> Date: Tue Mar 12 20:24:21 2024 +0000 [ASM][IAST] Configure maximum IAST Ranges (#5292) * Add configuration key * Use a RangeList in some case to not exceed the max number * Revert some code + implem correct merge * Fix + Add unit and integration tests * Usual macos fix for snapshot * Fix snapshots hashs * Update snapshots (remove other tests as they can't apply different env var values in same run) * Apply comment * Re-integrate integration tests with multiple processes (new fixture) * Add test case for setting MaxRangeCount to zero commit 83f6ab1 Author: Tony Redondo <tony.redondo@datadoghq.com> Date: Tue Mar 12 21:20:39 2024 +0100 [CI Visibility] - Enable snapshot testing of current testing framework implementations (#5226) commit 233695a Author: Daniel Romano <108014683+daniel-romano-DD@users.noreply.github.com> Date: Tue Mar 12 17:06:06 2024 +0100 [IAST] Vulnerability and Evidence truncation (#5302) * Initial implementation * Updated test bundle * Fix test compilation error * Fix snapshot (from rebase) * Fix typo in config value. Updated tests * Fix typo * Refactor converters initialization commit ea31cf5 Author: Anna <anna.yafi@datadoghq.com> Date: Tue Mar 12 16:39:09 2024 +0100 Deactivate benchmark for legacy encoder (#5299) commit d0d713a Author: NachoEchevarria <53266532+NachoEchevarria@users.noreply.github.com> Date: Tue Mar 12 09:25:27 2024 +0100 Set big regex timeouts for tests (#5297) commit d5388d6 Author: Lucas Pimentel <lucas.pimentel@datadoghq.com> Date: Mon Mar 11 15:20:58 2024 -0400 [Tracing] Support configuring `DD_TRACE_ENABLED` remotely (#5181) * add support for remote TraceEnabled setting * fix unrelated typo * add ApmTracingEnabled capability 19 * add missing RCM capability 18 * add mapping * add unit test * add comments to unit test * rename property to match RCM constant * include config in integration tests * fix test json * rewrite tests to use raw values instead of strings commit 2b95f46 Author: Flavien Darche <11708575+e-n-0@users.noreply.github.com> Date: Mon Mar 11 17:47:55 2024 +0100 [ASM][IAST] Support manual JSON deserialisation (Newtonsoft.Json) (#5238) * Add Newtonsoft.Json (non -working yet) * Refactor the tainting proces + add tests * Add the JToken Parse aspect + test * Rename Aspects class + Duck orignal method call * Add integration test * Fix nullability * Fix compilation issue for netfx * Change JSON formatting in ParseTests * Fix a test json format * Refactor NewtonsoftJsonAspects to static constructor commit 0d511f9 Author: Igor Kravchenko <21974069+kr-igor@users.noreply.github.com> Date: Mon Mar 11 09:35:23 2024 -0500 [DSM] - Fixes for IbmMq instrumentation (#5271) * Use byte properties instead of strings * Fixed nullability files * Added some debug info * Fixed lint issues * Added a bit more logs * Using slow byte->sbyte conversion * Added noop headers adapter * Fixed nullability files * Added more logs * Cleaned up debug logs * Removed symlink * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Removed debug code Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Using Unsafe.As instead of BlockCopy Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Update tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/IbmMq/IbmMqHeadersAdapter.cs Use Unsafe.As instead of BlockCopy Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Addressed some of the comments * Removed context propagation options --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> commit 5684a72 Author: Zach Montoya <zach.montoya@datadoghq.com> Date: Fri Mar 8 20:56:30 2024 -0800 [Tracing] Update instrumentation point for DD_TRACE_DELAY_WCF_INSTRUMENTATION_ENABLED=true (#5206) Updates the instrumentation point for `DD_TRACE_DELAY_WCF_INSTRUMENTATION_ENABLED=true` so that now a server span is created immediately before IDispatchMessageInspector implementations are run, so application code can access the root span from inside a IDispatchMessageInspector.AfterReceiveRequest callback. This PR also does some cleanup to remove unused Wcf files and it makes the entire Wcf instrumentation use nullable reference types. commit ca1bb6e Author: Andrew Lock <andrew.lock@datadoghq.com> Date: Fri Mar 8 17:43:57 2024 +0000 Fix errors identified from telemetry (#5279) * Try to avoid MongoDb exception We're seeing exceptions like this: ``` System.FieldAccessException at REDACTED at Datadog.Trace.ClrProfiler.AutoInstrumentation.MongoDb.MongoDbIntegration.CreateScope[TConnection](Object wireProtocol, TConnection connection) at REDACTED at MongoDB.Driver.Core.WireProtocol.CommandWireProtocol`1.ExecuteAsync(IConnection connection, CancellationToken cancellationToken) ``` and the only explanation I can think of is a duck-chaining issue, so stopped doing duck chaining and being explicit instead * Add local functions to try to isolate problems * Fix ArgumentNullException in AWS SQS integration

tonyredondo self-assigned this Feb 22, 2024

github-actions bot added area:tests unit tests, integration tests area:ci-visibility labels Feb 22, 2024

DataDog deleted a comment from andrewlock Feb 23, 2024

DataDog deleted a comment from datadog-ddstaging bot Mar 4, 2024

DataDog deleted a comment from andrewlock Mar 4, 2024

DataDog deleted a comment from datadog-ddstaging bot Mar 4, 2024

DataDog deleted a comment from andrewlock Mar 4, 2024

tonyredondo marked this pull request as ready for review March 4, 2024 17:22

tonyredondo requested review from a team as code owners March 4, 2024 17:22

andrewlock approved these changes Mar 5, 2024

View reviewed changes

tracer/test/snapshots/MsTestV2Tests.SubmitTraces_packageVersion=.verified.txt Show resolved Hide resolved

tracer/test/snapshots/XUnitTests.SubmitTraces_packageVersion=2.6.6.verified.txt Outdated Show resolved Hide resolved

link04 mentioned this pull request Mar 6, 2024

[Tracer] Setting parallelizeTestCollections to false to Triage CI Run #5270

Closed

tonyredondo added 5 commits March 6, 2024 23:26

Initial changes

eaa30aa

Changes

69be28a

fix formatting

509319a

add missing snapshots

7698e3c

changes

cd43b46

tonyredondo force-pushed the tony/civisibility-snapshot-testing branch from 8a5d126 to cd43b46 Compare March 6, 2024 22:55

DataDog deleted a comment from datadog-ddstaging bot Mar 6, 2024

DataDog deleted a comment from andrewlock Mar 6, 2024

tonyredondo added 3 commits March 7, 2024 00:04

changes

6a1c3ba

CIEnvironmentValues refactor

6760edc

fixes

e82a8d6

update snapshots

ab03de5

DataDog deleted a comment from datadog-ddstaging bot Mar 7, 2024

DataDog deleted a comment from andrewlock Mar 7, 2024

tonyredondo added 4 commits March 7, 2024 22:31

more snapshots

708634b

more snapshots

e346306

Test a single snapshot for all NUnit versions

40d2590

fixes and reduce the number of snapshots

c431e00

DataDog deleted a comment from datadog-ddstaging bot Mar 8, 2024

DataDog deleted a comment from andrewlock Mar 8, 2024

Fixes

519eb21

DataDog deleted a comment from datadog-ddstaging bot Mar 8, 2024

DataDog deleted a comment from andrewlock Mar 8, 2024

Merge branch 'master' into tony/civisibility-snapshot-testing

2392802

anmarchenko approved these changes Mar 8, 2024

View reviewed changes

Merge branch 'master' into tony/civisibility-snapshot-testing

bfc04e8

andrewlock approved these changes Mar 11, 2024

View reviewed changes

tonyredondo added 3 commits March 11, 2024 17:02

change Datadog.Trace.Ci.Environment namespace to Datadog.Trace.Ci.CiE…

8b8b3f1

…nvironment to avoid collision with System.Environment class

Avoid using old and new as a snapshot name.

1a39b53

fix

53d4961

DataDog deleted a comment from datadog-ddstaging bot Mar 11, 2024

DataDog deleted a comment from andrewlock Mar 11, 2024

Merge branch 'master' into tony/civisibility-snapshot-testing

685df38

Merge branch 'master' into tony/civisibility-snapshot-testing

62c8796

tonyredondo merged commit 83f6ab1 into master Mar 12, 2024

tonyredondo deleted the tony/civisibility-snapshot-testing branch March 12, 2024 20:20

github-actions bot added this to the vNext milestone Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI Visibility] - Enable snapshot testing of current testing framework implementations#5226

[CI Visibility] - Enable snapshot testing of current testing framework implementations#5226
tonyredondo merged 24 commits intomasterfrom
tony/civisibility-snapshot-testing

tonyredondo commented Feb 22, 2024 •

edited

Loading

Uh oh!

andrewlock left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewlock Mar 11, 2024

Uh oh!

tonyredondo Mar 11, 2024

Uh oh!

andrewlock Mar 11, 2024

Uh oh!

andrewlock Mar 11, 2024

Uh oh!

datadog-ddstaging bot commented Mar 11, 2024 •

edited

Loading

Uh oh!

andrewlock commented Mar 11, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tonyredondo commented Feb 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewlock Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

tonyredondo Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

andrewlock Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

andrewlock Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

datadog-ddstaging bot commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

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

Uh oh!

Execution-Time Benchmarks Report ⏱️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tonyredondo commented Feb 22, 2024 •

edited

Loading

datadog-ddstaging bot commented Mar 11, 2024 •

edited

Loading

andrewlock commented Mar 11, 2024 •

edited

Loading