Update more things to use artifacts output#8680
Merged
Merged
Conversation
andrewlock
added
the
area:builds
Contributor
|
BenchmarksBenchmark execution time: 2026-05-29 21:51:25 Comparing candidate commit 70253f0 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 61 known flaky benchmarks, 65 flaky benchmarks without significant changes.
|
andrewlock
changed the title
andrewlock
force-pushed
the
andrew/artifacts-output
branch
from
May 26, 2026 09:02
ae4148d to
7578ab7
Compare
andrewlock
force-pushed
the
andrew/more-artifacts-output
branch
from
May 26, 2026 09:02
f03168d to
23970e8
Compare
andrewlock
force-pushed
the
andrew/artifacts-output
branch
from
May 26, 2026 11:26
7578ab7 to
7317090
Compare
andrewlock
force-pushed
the
andrew/more-artifacts-output
branch
from
May 26, 2026 11:26
23970e8 to
c227d31
Compare
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8680) 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 (8680) - mean (74ms) : 70, 78
master - mean (75ms) : 71, 79
section Bailout
This PR (8680) - mean (80ms) : 76, 84
master - mean (77ms) : 76, 79
section CallTarget+Inlining+NGEN
This PR (8680) - mean (1,115ms) : 1053, 1177
master - mean (1,111ms) : 1054, 1169
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 (8680) - mean (118ms) : 112, 125
master - mean (116ms) : 111, 121
section Bailout
This PR (8680) - mean (116ms) : 111, 120
master - mean (115ms) : 111, 119
section CallTarget+Inlining+NGEN
This PR (8680) - mean (798ms) : 777, 818
master - mean (795ms) : 772, 818
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8680) - mean (102ms) : 98, 106
master - mean (105ms) : 99, 111
section Bailout
This PR (8680) - mean (104ms) : 100, 109
master - mean (105ms) : 100, 110
section CallTarget+Inlining+NGEN
This PR (8680) - mean (958ms) : 924, 992
master - mean (960ms) : 918, 1002
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8680) - mean (102ms) : 98, 106
master - mean (101ms) : 96, 106
section Bailout
This PR (8680) - mean (101ms) : 99, 102
master - mean (105ms) : 99, 110
section CallTarget+Inlining+NGEN
This PR (8680) - mean (827ms) : 787, 866
master - mean (822ms) : 785, 858
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 (8680) - mean (199ms) : 191, 207
master - mean (199ms) : 191, 207
section Bailout
This PR (8680) - mean (202ms) : 197, 208
master - mean (201ms) : 195, 207
section CallTarget+Inlining+NGEN
This PR (8680) - mean (1,211ms) : 1151, 1271
master - mean (1,197ms) : 1158, 1236
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 (8680) - mean (287ms) : 279, 295
master - mean (286ms) : 279, 294
section Bailout
This PR (8680) - mean (286ms) : 280, 292
master - mean (286ms) : 279, 294
section CallTarget+Inlining+NGEN
This PR (8680) - mean (963ms) : 942, 984
master - mean (961ms) : 943, 980
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8680) - mean (277ms) : 272, 282
master - mean (278ms) : 270, 287
section Bailout
This PR (8680) - mean (279ms) : 274, 283
master - mean (278ms) : 273, 284
section CallTarget+Inlining+NGEN
This PR (8680) - mean (1,156ms) : 1119, 1193
master - mean (1,155ms) : 1110, 1200
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8680) - mean (276ms) : 268, 284
master - mean (278ms) : 271, 286
section Bailout
This PR (8680) - mean (276ms) : 265, 287
master - mean (279ms) : 272, 285
section CallTarget+Inlining+NGEN
This PR (8680) - mean (1,036ms) : 990, 1081
master - mean (1,038ms) : 995, 1080
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
andrewlock
force-pushed
the
andrew/more-artifacts-output
branch
2 times, most recently
from
May 27, 2026 09:26
8133c26 to
bf8bd5e
Compare
andrewlock
force-pushed
the
andrew/artifacts-output
branch
from
May 28, 2026 09:54
7317090 to
303ef9e
Compare
andrewlock
force-pushed
the
andrew/more-artifacts-output
branch
2 times, most recently
from
May 29, 2026 09:16
d733886 to
e39d93a
Compare
andrewlock
marked this pull request as ready for review
May 29, 2026 09:18
gleocadie
approved these changes
May 29, 2026
gleocadie
left a comment
gleocadie
left a comment
Collaborator
There was a problem hiding this comment.
profiler side looks good
bouwkast
approved these changes
May 29, 2026
|
|
||
| // Scratch space used by the release-tooling targets in Build.GitHub.cs to download | ||
| // upstream Azure DevOps / GitLab artifacts. Not a build output destination. | ||
| AbsolutePath ReleaseArtifactsDirectory => BuildArtifactsDirectory / "release-artifacts"; |
Collaborator
There was a problem hiding this comment.
Do we need to add this to the CreateRequiredDirectories step along with the NativeArtifactsDirectory?
Maybe not if it worked here?
Member
Author
There was a problem hiding this comment.
It's probably fine, but better to be safe
Nuke's SymbolsDirectory was at tracer/bin/symbols. Repoint it under the repo-root artifacts/ tree so symbols ship alongside the other outputs of the new layout. Update the CI YAML's symbols: variable to match. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The assembled monitoring-home was at shared/bin/monitoring-home. Move it under the repo-root artifacts/ tree so the working-directory CI artifact gets closer to "only artifacts/ matters". Update the Nuke MonitoringHomeDirectory fallback, the --monitoring-home parameter help text, and the CI YAML monitoringHome: variable. The 30+ Nuke targets that copy into / read from MonitoringHomeDirectory follow the variable so no other changes are required. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
NativeBuildDirectory was the repo-root obj/ folder used as the CMake build tree by all native cmake -B invocations (tracer native, profiler native, tests, analyzers). Move it under artifacts/ so the working-directory CI artifact only needs to ship artifacts/. All Nuke call-sites use the variable so this is purely a single-point redirect. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
CMake (Linux/macOS) and MSBuild vcxproj (Windows) both wrote native tracer .so/.dll/.a binaries into per-project bin/ folders (tracer/src/Datadog.Tracer.Native/build/bin and tracer/src/Datadog.Tracer.Native/bin/<Config>/<Arch>/ respectively). Move both under artifacts/native/<Project>/ so the working-directory CI artifact only has to ship artifacts/. - CMakeLists.txt: OUTPUT_BIN_DIR becomes a cache variable defaulting to <repo>/artifacts/native/<Project>; Nuke can still override. - vcxprojs (DLL + static-lib + Tests): rewrite OutDir/IntDir to point at artifacts/native/<Project>/<Config>/<Arch>/ and artifacts/native-obj/<Project>/<Config>/<Arch>/. - Nuke: add GetNativeOutputDirectory(projectName) helper and route all consumers (PublishNativeTracerWindows/Unix/Osx, PublishNativeSymbols, RunTracerNativeTests Linux+Windows, macOS multi-arch build, Clean) through it. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Managed.Loader's output was at tracer/src/bin/ProfilerResources/<TFM>/ via an explicit <OutputPath> in the csproj that opts out of UseArtifactsOutput. Move it under artifacts/ for consistency with the rest of the tracer's outputs. Four coordinated edits because three downstream consumers string-reference the path: - csproj <OutputPath> repointed to ..\..\..\artifacts\ProfilerResources\ - Native tracer's Resource.rc (which embeds the loader DLL/PDB into the compiled native binary at link time): all 4 paths repointed up one extra level to ..\..\..\..\artifacts\ProfilerResources\ - build/cmake/FindManagedLoader.cmake's MANAGED_LOADER_DIRECTORY repointed - Build.Steps.cs CreateTrimmingFile's GetTypeReferences glob repointed Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
profiler/* and shared/* managed projects use the legacy RepoAndBuild-DirectoryStructure.props redirection mechanism: every project's BaseOutputPath / BaseIntermediateOutputPath / PackageOutputPath hangs off a single \$(BuildOutputRoot) which previously pointed at <repo>/profiler/_build/. Repoint that root to <repo>/artifacts/profiler-build so the entire subtree moves at once, with no change to the <bin|obj|CreatedPackages>/<Config-Platform>/... internal layout. - shared/RepoAndBuild-DirectoryStructure.props and profiler/Directory.Build.props: repoint BuildOutputRoot - Build.Steps.cs ProfilerOutputDirectory: follows the same redirect - ultimate-pipeline.yml: the linux-profiler-binaries publish step picks up from the new path Every reader of ProfilerOutputDirectory follows the Nuke variable so no other call-site changes are needed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Nuke's ArtifactsDirectory is the destination for final-form Nuke artifacts (NuGet packages, MSIs, zips, dd-dotnet, FleetInstaller, telemetry-forwarder, etc.). Its local default was tracer/bin/artifacts/ and CI overrode it via env var to tracer/src/bin/artifacts/. Move it under <repo>/artifacts/output/ so it joins the rest of the consolidated tree. - Build.Steps.cs: ArtifactsDirectory fallback -> BuildArtifactsDirectory/output. ToolSourceDirectory now hangs off the new ArtifactsDirectory instead of the old OutputDirectory. The OutputDirectory variable itself is kept as a scratch space for Build.GitHub.cs release-tooling downloads (which use it as a temporary destination for upstream artifacts, not as a build output sink) and commented accordingly. - Build.cs Clean: drop the now-redundant EnsureCleanDirectory(OutputDirectory) since EnsureCleanDirectory(BuildArtifactsDirectory) and EnsureCleanDirectory(ArtifactsDirectory) cover the relevant trees. - ultimate-pipeline.yml: repoint relativeArtifacts, artifacts, relativeRunnerTool, relativeRunnerStandalone to the new location. The ~25 $(artifacts)/nuget/... $(artifacts)/dd-dotnet/... etc. publish steps follow the new value transparently. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The default-deny .artifactignore previously needed to re-include **/bin/** and **/obj/** at any depth because tracer src/test outputs, profiler outputs, shared/bin/monitoring-home, and Nuke staging dirs were scattered across the working tree. After the artifacts/ migration, the entire build graph lands in artifacts/ (and packages/ for the NuGet restore cache), so we can drop those broad re-includes. The lone carve-out is tracer/src/Datadog.Trace.Tools.Runner/bin/**: Tools.Runner has an explicit <OutputPath> for the dd-trace tool's Console/Tool sub-layout consumed by BuildStandaloneTool packaging. Migrating it would require restructuring BuildStandaloneTool's PublishDir plumbing; flagged for a follow-up. Dropping the wide **/bin/** also removes the .git/** re-exclusion we needed previously — the AzDO ignore matcher was substring-matching "bin" / "obj" inside branch names like andrew/move-bin-and-obj and leaking the corresponding .git/refs entries. Tighten the corresponding restore-working-directory.yml itemPattern to drop obj/ (none of our artifacts contain a per-project obj/ now). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The shared Datadog.Trace.ClrProfiler.Native vcxprojs (loader DLL + native-loader-tests EXE) carried explicit <OutDir>bin\<Config>\<Arch>\ overrides that bypassed the RepoAndBuild-DirectoryStructure.props redirect. After the props-level move to artifacts/profiler-build/ (see earlier commit), these vcxprojs still wrote to their per-project bin/ folders. Apply the same fix pattern as commit 4 (tracer native): - CMakeLists.txt (loader + tests): OUTPUT_BIN_DIR becomes a cache variable defaulting to <repo>/artifacts/native/<Project>; Nuke can override. - vcxprojs (loader + tests): rewrite OutDir/IntDir to point at ..\..\..\artifacts\native\<Project>\<Config>\<Arch>\ and ..\..\..\artifacts\native-obj\<Project>\<Config>\<Arch>\. - Build.Shared.Steps.cs: route all consumers (PublishNativeLoaderWindows/Unix/Osx, RunNativeLoaderTests Linux+ Windows, ValidateNativeLoaderSnapshotTestsLinux, macOS multi-arch build clean) through the shared GetNativeOutputDirectory helper. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mirrors the artifacts/native-obj/ name and makes the bin/obj symmetry explicit. No functional change; touches the NativeArtifactsDirectory Nuke literal plus the 4 CMakeLists.txt + 5 vcxproj OutDir paths that default into the directory. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
artifacts/bin/ is reserved for managed UseArtifactsOutput build outputs. The vcpkg download cache (downloaded + bootstrapped by GetVcpkg()) is a tool dependency, not a build output, so giving it its own top-level artifacts/vcpkg/ directory keeps the artifacts/bin/ semantics clean. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Make the scope explicit: this directory only holds NATIVE debug symbols (Datadog.Tracer.Native.pdb / Datadog.Profiler.Native.pdb / datadog_profiling_ffi.pdb / ddwaf.pdb / dd-dotnet.pdb on Windows, plus *.debug on Linux). Managed .pdb files continue to live next to their .dll in artifacts/bin/<Project>/<pivot>/ and ship via .snupkg / windows-tracer-home.zip. Touches SymbolsDirectory in Build.Steps.cs and the symbols: variable in ultimate-pipeline.yml. The ZipSymbols output filename stays windows-native-symbols.zip (already native-flavoured). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
After the artifacts/ tidy, only five subtrees are actually read by downstream test/pack/integration stages: artifacts/bin/ managed UseArtifactsOutput outputs artifacts/obj/ project.assets.json etc. for --no-restore artifacts/monitoring-home/ tracer loaded by integration tests artifacts/output/ nupkg/MSI/zips/tarballs/dd-dotnet packages/ NuGet restore cache Everything else under artifacts/ is build-stage-local: native-bin (consumed by PublishNative*/PublishProfiler* same-stage which copy into monitoring-home), native-obj (CMake intermediate), native-symbols (ZipSymbols packs into windows-native-symbols.zip same-stage), profiler-build, ProfilerResources (Resource.rc link consumer), publish, package (auto-pack — duplicated in output/nuget/), build_data (test results published per-job), deps, vcpkg. None of those are read by any downstream consumer. Replace .artifactignore's `!**/bin/**` / `!**/obj/**` with the precise five-line include list. Drop the Tools.Runner per-project bin carve-out (its outputs ship as the standalone `runner-dotnet-tool` AzDO artifact, not via the working-directory). Mirror the same five paths in restore-working-directory.yml's patterns. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Drop the per-project bin\{Console,Tool}\<TFM>\ OutputPath overrides and let
UseArtifactsOutput handle both variants. The two BuildStandalone settings
are distinguished via ArtifactsProjectName so they don't clobber each other:
artifacts/bin/Datadog.Trace.Tools.Runner.Console/<config>_<tfm>_<rid>/ (single-file dd-trace)
artifacts/bin/Datadog.Trace.Tools.Runner.Tool/<config>_<tfm>/ (PackAsTool global tool)
PublishDir for the Console variant is set explicitly by the Nuke target
(BuildStandaloneTool sets it to artifacts/output/tool/<rid>) so the OutputPath
move is invisible to the published archive.
Every item under artifacts/output/ (nupkg, MSI, dd-trace zips/tarballs, dd-dotnet, runnerTool, windows-tracer-home.zip) is published as its own named AzDO artifact in the same build stage and re-downloaded by downstream consumers — none read it via the working-directory artifact. Confirmed by grepping for `path: $(artifacts)` / `path: $(outputDir)` / `path: $(runnerTool)` etc.: zero downstream download sites point inside artifacts/output/, and the runner-dotnet-tool / nuget / windows-tracer-home.zip artifacts are all explicitly downloaded by their consumers. Cuts another large chunk off the build-*-working-directory artifact.
Rename the Nuke OutputDirectory property to ReleaseArtifactsDirectory and
point it at BuildArtifactsDirectory/release-artifacts (i.e.
artifacts/release-artifacts/) instead of tracer/bin/. Update the 6
release-tooling call sites in Build.GitHub.cs that download consolidated
AzDO + GitLab artifacts for the GitHub release workflow.
The release workflow consumes the path dynamically via ::set-output
(artifacts_path / gitlab_artifacts_path / sha_path), so the directory
rename is invisible to it. The new name matches the AzDO artifact name
({FullVersion}-release-artifacts) downloaded into it.
OutputDirectory is kept as a legacy alias for one commit while
CompareCodeCoverageReports still references it; the follow-up commit
moves coverage scratch under build_data and drops the alias.
CompareCodeCoverageReports downloads PR-vs-master coverage XMLs to compare and post a PR comment. Move its scratch from OutputDirectory/CodeCoverage/ (tracer/bin/CodeCoverage/) to BuildDataDirectory/CodeCoverage/ (artifacts/build_data/CodeCoverage/), alongside the sibling comparison targets CompareBenchmarksResultsBP and CompareExecutionTimeBenchmarkResults which already use BuildDataDirectory. The script invocation in .azure-pipelines/ultimate-pipeline.yml takes no path argument, so the AzDO stage is unaffected. The two reportgenerator directories used elsewhere in the coverage stage ($(Build.SourcesDirectory) /cover and /coveragereport) are unrelated AzDO-task-local scratch. With CompareCodeCoverageReports moved, the legacy OutputDirectory property has zero references and is removed.
andrewlock
force-pushed
the
andrew/more-artifacts-output
branch
from
May 29, 2026 21:00
e39d93a to
70253f0
Compare
andrewlock
added a commit
that referenced
this pull request
Jun 1, 2026
## Summary of changes Update all the launchsettings.json to point to the new output path in #8680 ## Reason for change We moved the default output folder in #8680, but there were already enough changes in that PR, so I didn't want to clutter it up with these simple changes. ## Implementation details 🤖 converted `shared\bin` to `artifacts` ## Test coverage N/A - it's only for local testing (personally I would probably remove all of these files completely, but some people like them) ## Other details Stacked on - #8610 - #8636 - #8680 --------- Co-authored-by: Claude Opus 4.7 <[email protected]>
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
3 participants
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
Follows on from #8636, moves more build outputs into the artifacts folder
Reason for change
Today, build outputs end up scattered throughput the repository, because of how the defaults work for .NET, and how we're using a very "bespoke" way of building (i.e. we're not just doing
dotnet build). That makes it tricky to track where things are built to in CI particularly, and will hamper any attempts to modify CI (e.g. to move to GitLab).Implementation details
Had 🤖 look for everything that wasn't going to the
artifacts/folder, and move it, so that we can more easily include/exclude it from artifacts. Took a relatively conservative approach in places (e.g. to the profiler assets)Final results look like this:
The important thing is that we can exclude most of these when doing the "upload working directory" step
Test coverage
This is the test - if the build still passes I think we're ok.
Other details
Stacked on
Datadog.Trace.Build.g.slnto reduce size of artifacts copied between stages #8610ArtifactsOutputin more places in the tracer build #8636