-
Notifications
You must be signed in to change notification settings - Fork 151
Build and run tests against .NET 10 RC 1 #7499
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
Conversation
andrewlock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far - I would expect there to be find+replace changes we need to NuGet packages in the samples too though?
bouwkast
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a find all / replace with the wrong value for NuGet versions
tracer/test/Datadog.Trace.IntegrationTests/Datadog.Trace.IntegrationTests.csproj
Outdated
Show resolved
Hide resolved
tracer/test/Datadog.Trace.IntegrationTests/Datadog.Trace.IntegrationTests.csproj
Outdated
Show resolved
Hide resolved
.../test-applications/security/Samples.Security.AspNetCore5/Samples.Security.AspNetCore5.csproj
Outdated
Show resolved
Hide resolved
profiler/src/Demos/Samples.Computer01/Samples.Computer01.csproj
Outdated
Show resolved
Hide resolved
b073a00 to
816f2ce
Compare
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:
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.8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (72ms) : 71, 73
. : milestone, 72,
master - mean (72ms) : 71, 73
. : milestone, 72,
section Baseline
This PR (7499) - mean (68ms) : 65, 72
. : milestone, 68,
master - mean (68ms) : 66, 70
. : milestone, 68,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (1,001ms) : 968, 1034
. : milestone, 1001,
master - mean (998ms) : 970, 1026
. : milestone, 998,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (106ms) : 105, 107
. : milestone, 106,
master - mean (106ms) : 105, 107
. : milestone, 106,
section Baseline
This PR (7499) - mean (106ms) : 103, 108
. : milestone, 106,
master - mean (105ms) : 102, 108
. : milestone, 105,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (703ms) : 682, 724
. : milestone, 703,
master - mean (704ms) : 685, 723
. : milestone, 704,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (94ms) : 93, 95
. : milestone, 94,
master - mean (94ms) : 93, 95
. : milestone, 94,
section Baseline
This PR (7499) - mean (93ms) : 91, 95
. : milestone, 93,
master - mean (93ms) : 91, 94
. : milestone, 93,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (661ms) : 640, 683
. : milestone, 661,
master - mean (665ms) : 649, 681
. : milestone, 665,
gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (92ms) : 91, 94
. : milestone, 92,
master - mean (93ms) : 91, 94
. : milestone, 93,
section Baseline
This PR (7499) - mean (92ms) : 89, 94
. : milestone, 92,
master - mean (91ms) : 89, 93
. : milestone, 91,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (592ms) : 582, 602
. : milestone, 592,
master - mean (599ms) : 587, 610
. : milestone, 599,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (193ms) : 190, 197
. : milestone, 193,
master - mean (194ms) : 190, 199
. : milestone, 194,
section Baseline
This PR (7499) - mean (192ms) : 187, 197
. : milestone, 192,
master - mean (192ms) : 184, 200
. : milestone, 192,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (1,101ms) : 1059, 1144
. : milestone, 1101,
master - mean (1,095ms) : 1059, 1130
. : milestone, 1095,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (281ms) : 276, 286
. : milestone, 281,
master - mean (274ms) : 268, 280
. : milestone, 274,
section Baseline
This PR (7499) - mean (277ms) : 272, 281
. : milestone, 277,
master - mean (273ms) : 266, 280
. : milestone, 273,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (902ms) : 869, 935
. : milestone, 902,
master - mean (890ms) : 852, 929
. : milestone, 890,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (269ms) : 265, 274
. : milestone, 269,
master - mean (267ms) : 259, 275
. : milestone, 267,
section Baseline
This PR (7499) - mean (269ms) : 264, 274
. : milestone, 269,
master - mean (266ms) : 259, 273
. : milestone, 266,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (875ms) : 834, 915
. : milestone, 875,
master - mean (876ms) : 833, 919
. : milestone, 876,
gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat X
axisFormat %s
todayMarker off
section Bailout
This PR (7499) - mean (268ms) : 265, 271
. : milestone, 268,
master - mean (265ms) : 260, 271
. : milestone, 265,
section Baseline
This PR (7499) - mean (269ms) : 263, 275
. : milestone, 269,
master - mean (265ms) : 259, 271
. : milestone, 265,
section CallTarget+Inlining+NGEN
This PR (7499) - mean (790ms) : 770, 810
. : milestone, 790,
master - mean (786ms) : 762, 811
. : milestone, 786,
|
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback". |
## Summary of changes Fix issues that cause failure to compile if you try to use RC1 ## Reason for change @bouwkast found these issues in #7499, but some people have already installed RC1 and are getting local failures, so this should unblock them ## Implementation details Copy the fixes from #7499 ## Test coverage Covered by existing (hopefully) ## Other details <!-- Fixes #{issue} --> <!--⚠️ Note: Where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. MergeQueue is NOT enabled in this repository. If you have write access to the repo, the PR has 1-2 approvals (see above), and all of the required checks have passed, you can use the Squash and Merge button to merge the PR. If you don't have write access, or you need help, reach out in the #apm-dotnet channel in Slack. -->
|
| bool expandRouteParameters = false) | ||
| where T : class | ||
| { | ||
| #pragma warning disable ASPDEPR004 // WebHostBuilder is deprecated but we need it for net core 2.1 FIXME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh, don't actually need to FIXME 😄
andrewlock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
WebHostBuilder is deprecated but we need it for net core 2.1 seems to only be in this one case where we use it so just ignoring for the time being. New to .NET 10
Was trying to resolve a NU1510 error, but went away with VS 26
Can't get any other means to disable them.
This reverts commit 32d9d64. Caused different issues
We will need to re-update the VMs after pushing new docker images for them. Planning on doing that at a later point
4adbcc2 to
2db1fc6
Compare
## Summary of changes Fixes the nuget tool smoke tests ## Reason for change The nuget tool smoke tests runs in the _sdk_ image, which means we need an sdk version number (10.0.100) not a runtime version number (10.0.0) ## Implementation details Simple typo fix ## Test coverage Will do a full test to make sure ## Other details Related to #7499
Summary of changes
This updates our build and testing to work with .NET 10 RC 1, updating from .NET 10 Preview 7 to it.
Reason for change
https://devblogs.microsoft.com/dotnet/dotnet-10-rc-1/
https://github.com/dotnet/core/blob/main/release-notes/10.0/preview/rc1/libraries.md
Implementation details
MemoryExtensions public static void Reverse<T>(this Span<T> span)- I just did aAsEnumerablefor the time beingASPDEPR008in which theWebHostBuilderis now deprecated in one of the test filesTest coverage
Other details
I don't have permissions to do most of the stuff with the managed pools :(
I initially had more instances of
NU1510, but I am only getting those in VS 22, they don't show up in VS 26 🤷https://datadoghq.atlassian.net/browse/LANGPLAT-758