Skip to content

Fix checking for changes to generated files#6412

Merged
andrewlock merged 2 commits into
masterfrom
andrew/ci/fix-calltarget-checks
Dec 9, 2024
Merged

Fix checking for changes to generated files#6412
andrewlock merged 2 commits into
masterfrom
andrew/ci/fix-calltarget-checks

Conversation

@andrewlock

Copy link
Copy Markdown
Member

Summary of changes

  • Fixes the verify_source_generated_changes_are_persisted.yml workflow
  • Adds the missing changes

Reason for change

This workflow was broken when we stopped using a source generator to generate the call target integrations, so it was no longer flagging cases where the call target files have not been correctly updated and need to be rebuilt using Nuke

Implementation details

Add the additional paths to include in the diff comparison

Test coverage

Did a manual test locally. Also committing this in 2 parts, to confirm it flags the currently broken master.

Other details

@andrewlock andrewlock added the area:builds project files, build scripts, pipelines, versioning, releases, packages label Dec 9, 2024
@andrewlock
andrewlock requested review from a team as code owners December 9, 2024 14:24
@datadog-ddstaging

datadog-ddstaging Bot commented Dec 9, 2024

Copy link
Copy Markdown

Datadog Report

Branch report: andrew/ci/fix-calltarget-checks
Commit report: a51d034
Test service: dd-trace-dotnet

✅ 0 Failed, 470202 Passed, 3685 Skipped, 32h 16m 56.45s Total Time

@andrewlock

andrewlock commented Dec 9, 2024

Copy link
Copy Markdown
Member Author

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 (6412) - mean (69ms)  : 66, 73
     .   : milestone, 69,
    master - mean (70ms)  : 67, 72
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (6412) - mean (980ms)  : 957, 1003
     .   : milestone, 980,
    master - mean (988ms)  : 963, 1014
     .   : milestone, 988,

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

    section CallTarget+Inlining+NGEN
    This PR (6412) - mean (680ms)  : 666, 694
     .   : milestone, 680,
    master - mean (687ms)  : 669, 704
     .   : milestone, 687,

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

    section CallTarget+Inlining+NGEN
    This PR (6412) - mean (633ms)  : 617, 650
     .   : milestone, 633,
    master - mean (643ms)  : 626, 660
     .   : milestone, 643,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6412) - mean (191ms)  : 186, 197
     .   : milestone, 191,
    master - mean (190ms)  : 186, 194
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6412) - mean (1,093ms)  : 1060, 1127
     .   : milestone, 1093,
    master - mean (1,091ms)  : 1063, 1119
     .   : milestone, 1091,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6412) - mean (276ms)  : 272, 281
     .   : milestone, 276,
    master - mean (276ms)  : 272, 280
     .   : milestone, 276,

    section CallTarget+Inlining+NGEN
    This PR (6412) - mean (868ms)  : 839, 897
     .   : milestone, 868,
    master - mean (869ms)  : 839, 900
     .   : milestone, 869,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6412) - mean (266ms)  : 261, 271
     .   : milestone, 266,
    master - mean (266ms)  : 262, 269
     .   : milestone, 266,

    section CallTarget+Inlining+NGEN
    This PR (6412) - mean (848ms)  : 810, 885
     .   : milestone, 848,
    master - mean (850ms)  : 818, 882
     .   : milestone, 850,

Loading

@andrewlock
andrewlock force-pushed the andrew/ci/fix-calltarget-checks branch from a20a558 to a51d034 Compare December 9, 2024 16:53
@andrewlock
andrewlock merged commit ee9d184 into master Dec 9, 2024
@andrewlock
andrewlock deleted the andrew/ci/fix-calltarget-checks branch December 9, 2024 18:42
@github-actions github-actions Bot added this to the vNext-v3 milestone Dec 9, 2024
veerbia pushed a commit that referenced this pull request Dec 16, 2024
## Summary of changes

- Fixes the `verify_source_generated_changes_are_persisted.yml` workflow
- Adds the missing changes

## Reason for change

This workflow was broken when we stopped using a source generator to
generate the call target integrations, so it was no longer flagging
cases where the call target files have _not_ been correctly updated and
need to be rebuilt using Nuke

## Implementation details

Add the additional paths to include in the diff comparison

## Test coverage

Did a manual test locally. Also committing this in 2 parts, to confirm
it flags the currently broken master.

## Other details

This missing file updates should have been added in #6397 but were missed because this was broken
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:builds project files, build scripts, pipelines, versioning, releases, packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants