-
Notifications
You must be signed in to change notification settings - Fork 147
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
Create an "installer" executable for use with the Datadog installer #6643
Conversation
64a4c7b
to
29a2ff8
Compare
Datadog ReportBranch report: ✅ 0 Failed, 241328 Passed, 2373 Skipped, 18h 16m 9.47s Total Time |
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.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6643) - mean (69ms) : 66, 71
. : milestone, 69,
master - mean (69ms) : 66, 71
. : milestone, 69,
section CallTarget+Inlining+NGEN
This PR (6643) - mean (993ms) : 970, 1016
. : milestone, 993,
master - mean (997ms) : 975, 1018
. : milestone, 997,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6643) - mean (102ms) : 100, 104
. : milestone, 102,
master - mean (102ms) : 100, 104
. : milestone, 102,
section CallTarget+Inlining+NGEN
This PR (6643) - mean (670ms) : 648, 691
. : milestone, 670,
master - mean (674ms) : 658, 690
. : milestone, 674,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6643) - mean (90ms) : 88, 91
. : milestone, 90,
master - mean (89ms) : 87, 92
. : milestone, 89,
section CallTarget+Inlining+NGEN
This PR (6643) - mean (630ms) : 615, 646
. : milestone, 630,
master - mean (633ms) : 618, 647
. : milestone, 633,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6643) - mean (191ms) : 185, 197
. : milestone, 191,
master - mean (191ms) : 186, 196
. : milestone, 191,
section CallTarget+Inlining+NGEN
This PR (6643) - mean (1,102ms) : 1070, 1133
. : milestone, 1102,
master - mean (1,105ms) : 1078, 1133
. : milestone, 1105,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6643) - mean (270ms) : 266, 274
. : milestone, 270,
master - mean (271ms) : 266, 275
. : milestone, 271,
section CallTarget+Inlining+NGEN
This PR (6643) - mean (862ms) : 831, 894
. : milestone, 862,
master - mean (871ms) : 829, 913
. : milestone, 871,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6643) - mean (262ms) : 257, 266
. : milestone, 262,
master - mean (263ms) : 259, 266
. : milestone, 263,
section CallTarget+Inlining+NGEN
This PR (6643) - mean (845ms) : 817, 872
. : milestone, 845,
master - mean (853ms) : 813, 893
. : milestone, 853,
|
Benchmarks Report for tracer 🐌Benchmarks for #6643 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.ILoggerBenchmark.EnrichedLog‑net472 | 1.123 | 2,408.77 | 2,704.77 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | EnrichedLog |
net6.0 | 1.48μs | 0.657ns | 2.46ns | 0.023 | 0 | 0 | 1.64 KB |
master | EnrichedLog |
netcoreapp3.1 | 2.18μs | 1.01ns | 3.79ns | 0.0218 | 0 | 0 | 1.64 KB |
master | EnrichedLog |
net472 | 2.41μs | 0.67ns | 2.41ns | 0.249 | 0 | 0 | 1.57 KB |
#6643 | EnrichedLog |
net6.0 | 1.48μs | 1.88ns | 6.79ns | 0.0228 | 0 | 0 | 1.64 KB |
#6643 | EnrichedLog |
netcoreapp3.1 | 2.18μs | 0.608ns | 2.19ns | 0.0219 | 0 | 0 | 1.64 KB |
#6643 | EnrichedLog |
net472 | 2.71μs | 2.24ns | 8.08ns | 0.249 | 0 | 0 | 1.57 KB |
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | EnrichedLog |
net6.0 | 112μs | 97.9ns | 379ns | 0 | 0 | 0 | 4.28 KB |
master | EnrichedLog |
netcoreapp3.1 | 117μs | 216ns | 837ns | 0.0586 | 0 | 0 | 4.28 KB |
master | EnrichedLog |
net472 | 150μs | 86.7ns | 324ns | 0.674 | 0.225 | 0 | 4.46 KB |
#6643 | EnrichedLog |
net6.0 | 112μs | 138ns | 534ns | 0.0559 | 0 | 0 | 4.28 KB |
#6643 | EnrichedLog |
netcoreapp3.1 | 117μs | 275ns | 1.07μs | 0 | 0 | 0 | 4.28 KB |
#6643 | EnrichedLog |
net472 | 151μs | 154ns | 597ns | 0.673 | 0.224 | 0 | 4.46 KB |
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | EnrichedLog |
net6.0 | 2.94μs | 0.786ns | 3.05ns | 0.031 | 0 | 0 | 2.2 KB |
master | EnrichedLog |
netcoreapp3.1 | 4.1μs | 1.96ns | 7.34ns | 0.0288 | 0 | 0 | 2.2 KB |
master | EnrichedLog |
net472 | 4.92μs | 0.984ns | 3.68ns | 0.321 | 0 | 0 | 2.02 KB |
#6643 | EnrichedLog |
net6.0 | 2.98μs | 0.853ns | 3.08ns | 0.0312 | 0 | 0 | 2.2 KB |
#6643 | EnrichedLog |
netcoreapp3.1 | 4.27μs | 1.19ns | 4.44ns | 0.028 | 0 | 0 | 2.2 KB |
#6643 | EnrichedLog |
net472 | 4.82μs | 1.46ns | 5.66ns | 0.32 | 0 | 0 | 2.02 KB |
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | SendReceive |
net6.0 | 1.35μs | 0.852ns | 3.3ns | 0.0162 | 0 | 0 | 1.14 KB |
master | SendReceive |
netcoreapp3.1 | 1.78μs | 0.645ns | 2.5ns | 0.015 | 0 | 0 | 1.14 KB |
master | SendReceive |
net472 | 2.14μs | 1.51ns | 5.43ns | 0.183 | 0 | 0 | 1.16 KB |
#6643 | SendReceive |
net6.0 | 1.25μs | 0.75ns | 2.91ns | 0.0162 | 0 | 0 | 1.14 KB |
#6643 | SendReceive |
netcoreapp3.1 | 1.8μs | 1.12ns | 4.35ns | 0.0153 | 0 | 0 | 1.14 KB |
#6643 | SendReceive |
net472 | 2.07μs | 4.02ns | 15.6ns | 0.184 | 0 | 0 | 1.16 KB |
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | EnrichedLog |
net6.0 | 2.81μs | 0.819ns | 3.07ns | 0.0224 | 0 | 0 | 1.6 KB |
master | EnrichedLog |
netcoreapp3.1 | 3.9μs | 1.5ns | 5.6ns | 0.0215 | 0 | 0 | 1.65 KB |
master | EnrichedLog |
net472 | 4.49μs | 3.72ns | 14.4ns | 0.323 | 0 | 0 | 2.04 KB |
#6643 | EnrichedLog |
net6.0 | 2.69μs | 0.737ns | 2.66ns | 0.0217 | 0 | 0 | 1.6 KB |
#6643 | EnrichedLog |
netcoreapp3.1 | 3.98μs | 3.3ns | 12.3ns | 0.022 | 0 | 0 | 1.65 KB |
#6643 | EnrichedLog |
net472 | 4.6μs | 4.08ns | 15.3ns | 0.322 | 0 | 0 | 2.04 KB |
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️
Slower ⚠️ in #6643
Benchmark
diff/base
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1
1.156
552.32
638.36
Faster 🎉 in #6643
Benchmark
base/diff
Base Median (ns)
Diff Median (ns)
Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0
1.165
461.61
396.06
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net472
1.148
889.15
774.68
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0
1.115
540.73
485.13
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1 | 1.156 | 552.32 | 638.36 |
Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 | 1.165 | 461.61 | 396.06 | |
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net472 | 1.148 | 889.15 | 774.68 | |
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0 | 1.115 | 540.73 | 485.13 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StartFinishSpan |
net6.0 | 463ns | 0.636ns | 2.46ns | 0.00808 | 0 | 0 | 576 B |
master | StartFinishSpan |
netcoreapp3.1 | 553ns | 0.772ns | 2.99ns | 0.00779 | 0 | 0 | 576 B |
master | StartFinishSpan |
net472 | 609ns | 1.24ns | 4.78ns | 0.0916 | 0 | 0 | 578 B |
master | StartFinishScope |
net6.0 | 540ns | 0.8ns | 3.1ns | 0.00978 | 0 | 0 | 696 B |
master | StartFinishScope |
netcoreapp3.1 | 738ns | 0.965ns | 3.74ns | 0.00938 | 0 | 0 | 696 B |
master | StartFinishScope |
net472 | 888ns | 1.47ns | 5.7ns | 0.104 | 0 | 0 | 658 B |
#6643 | StartFinishSpan |
net6.0 | 395ns | 0.629ns | 2.43ns | 0.00813 | 0 | 0 | 576 B |
#6643 | StartFinishSpan |
netcoreapp3.1 | 637ns | 0.734ns | 2.84ns | 0.00795 | 0 | 0 | 576 B |
#6643 | StartFinishSpan |
net472 | 586ns | 2.22ns | 8.58ns | 0.0917 | 0 | 0 | 578 B |
#6643 | StartFinishScope |
net6.0 | 484ns | 0.978ns | 3.79ns | 0.0097 | 0 | 0 | 696 B |
#6643 | StartFinishScope |
netcoreapp3.1 | 687ns | 1.96ns | 7.61ns | 0.00931 | 0 | 0 | 696 B |
#6643 | StartFinishScope |
net472 | 774ns | 1.04ns | 4.01ns | 0.104 | 0 | 0 | 658 B |
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | RunOnMethodBegin |
net6.0 | 657ns | 0.752ns | 2.91ns | 0.00979 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
netcoreapp3.1 | 956ns | 1.04ns | 4.02ns | 0.00917 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
net472 | 1.1μs | 1.64ns | 6.34ns | 0.104 | 0 | 0 | 658 B |
#6643 | RunOnMethodBegin |
net6.0 | 626ns | 0.685ns | 2.65ns | 0.00987 | 0 | 0 | 696 B |
#6643 | RunOnMethodBegin |
netcoreapp3.1 | 872ns | 1.77ns | 6.84ns | 0.00931 | 0 | 0 | 696 B |
#6643 | RunOnMethodBegin |
net472 | 1.06μs | 1.86ns | 7.22ns | 0.104 | 0 | 0 | 658 B |
9ea0872
to
16cda8e
Compare
@@ -603,6 +603,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Samples.AzureFunctions.V4Is | |||
EndProject | |||
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Samples.AzureFunctions.V4Isolated.AspNetCore", "tracer\test\test-applications\azure-functions\Samples.AzureFunctions.V4Isolated.AspNetCore\Samples.AzureFunctions.V4Isolated.AspNetCore.csproj", "{0F8EAB52-0C5B-4F60-92C5-42FAC21F4E77}" | |||
EndProject | |||
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Datadog.FleetInstaller", "tracer\src\Datadog.FleetInstaller\Datadog.FleetInstaller.csproj", "{47C1970A-0098-45A2-9D65-7790607D9A68}" |
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.
Adding even more projects to the solution that don't really need to be there makes me sad 😔
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.
Adding even more projects to the solution that don't really need to be there makes me sad 😔
It depends on your definition of "need", surely? 😅 I don't need half of the projects in the sln day-to-day 😅 What is the sln for anymore? I'm only half joking tbh, seeing as we don't (and can't) do a simple dotnet build
on it, is there any point in it?
More importantly, if I don't add this to the solution even if I don't explicitly try to build it, then it breaks the build 😩 Because of horrible NuGet restore reasons.
FWIW, I initially had this in /shared
next to the MSI, but given I want to use our existing test projects in the tracer to unit/integration test this (in a subsequent PR), I moved it here as this seemed like the correct place, and at that point I had to add it to the sln 🤷♂️
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.
Because of horrible NuGet restore reasons
Right. I always forget this.
I have my own tiny sln for everyday work now so I don't know why I care about Datadog.Trace.sln
anymore 😝
... although, restoring everything does make nuke builds slower. We should consider having some things in their own standalone sln. Definitely not blocking here.
// The order of these values is important, as they are used to determine the exit code of the process | ||
// We should always add new error values to the end and not re-order them |
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.
Given this comment, should we given them explicit values?
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.
🤷♂️ That was my initial thought, but I've seen more cases of that causing copy-pasta duplication errors than actually avoiding it personally 😄 Happy to if you have feelings stronger than my "mild disinclination" 😅
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.
Level of care low. The comment is probably good enough.
FilesToAddToGac = | ||
[ | ||
Path.Combine(tracerHomeDirectory, "net461", "Datadog.Trace.dll"), | ||
Path.Combine(tracerHomeDirectory, "net461", "Datadog.Trace.MSBuild.dll"), |
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.
Is SSI supported for CI Visibility?
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.
Not explicitly, but omitting this seemed likely to cause confusion down the line without any real benefit.
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.
👍🏽
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.
Nice!
# Conflicts: # tracer/src/Datadog.FleetInstaller/.editorconfig # tracer/src/Datadog.FleetInstaller/Directory.Build.props
…ry, given we have the dll path
…w why it's necessary
16cda8e
to
d77915f
Compare
…6644) ## Summary of changes - Build the fleet installer exe in GitLab - Include the fleet installer exe in Windows OCI - Produce Windows OCI images in Staging ## Reason for change We need to build the fleet installer executable, and include it in a Windows OI image ## Implementation details - Add a stage `BuildFleetInstaller` to explicitly publish the fleet installer - Update the `download-single-step-artifacts` to grab the `windows-tracer-home.zip` and fleet installer artifact from the `Build` stage - Update the OCI image For tagged releases this flow is a bit more convoluted. We need to grab the fleet-instller.exe artifact from the s3 upload that's done in the `publish` step, because we don't store it on the GitHub release. It's a bit ugly, but it works. ## Test coverage This is the test - if it builds, and the OCI image contains the expected files, we're good ## Other details Note that _currently_, the final OCI images will _not_ contain Windows layers, until we unblock that both in our gitlab YAML, and in the libdatadog-build one pipeline. Part of a stack of PRs: - #6643 - #6644 👈 - #6645
## Summary of changes Adds a new smoke test stage, for running inside Windows using the fleet installer ## Reason for change Emulates an end-to-end smoke test for the fleet installer executable. ## Implementation details - Build the fleet installer in the AzDo pipeline, as part of the `package-windows` stage - Adds a new `fleet_installer_smoke_tests` stage for running the smoke tests - Fix issues in ASP.NET Core test app, so that it works when hosted in IIS - Add a dockerfile for running the ASP.NET Core app with the fleet installer, hosted in IIS That latter point proved to be somewhat of a nightmare. The resulting dockerfile feels kind of horrible, but I couldn't find another way to have: - Delay the start of the app pool until the container image is running (i.e. not during build time) - Start the worker process when the app pool starts, _without_ an incoming request - Shut down the app pool when the worker process exits (which is the way the aspnetcore app works) - Exit the container The workaround, using the entrypoint script, makes a 404 request to the app to spin up the worker process, manually shuts down the app pool (to make sure the app definitely shuts down and flushes), and then exits. This causes an additional span in the traces, so needed to create new snapshots. ## Test coverage This gives basic snapshot testing on 2022 which is the only version I could get to work given the hosted images on AzDo and the available python base images for the test agent. I think that is sufficient - we are going to have additional end-to-end testing of Windows images in system tests anyway. ## Other details Part of a stack of PRs: - #6643 - #6644 - #6645 👈
Summary of changes
Creates an "installer" executable, designed to be executed by the fleet installer as part of configuring Windows SSI.
Reason for change
We don't want to use the existing MSI, primarily because it requires stopping and starting IIS when you update it, otherwise we risk causing crashes.
The datadog-installer is responsible for copying the files added to the Windows OCI image, and running the Datadog.FleetInstaller.exe executable to configure the application. Similarly, it calls this exe when uninstalling a tracer version or removing the product
Implementation details
environmentVariables
section in applicationHost.config. Uses the Microsoft.Web.Administration NuGet to interact with the native API.Test coverage
Manually tested. Subsequent PRs in the stack will add smoke and integration tests.
Other details
Explicitly designed to run on Windows Server 2016+, so targets .NET FX for simplicity. The Microsoft.Web.Administration nuget is a .NET Standard 1.x dll, which creates an annoying number of dlls in the output, but it's just ugly, it works fine.
There's a fair amount of duplication in the Fusion API/Microsoft.Web.Administration code with what the
dd-dotnet
/dd-trace
tool currently does. We could certainly look at consolidating that down the line, and potentially rewriting the fleet installer tool to be a NativeAOT executable, but it's probably not worth the effort at this point.The explicit
<RuntimeIdentifier>win-x64</RuntimeIdentifier>
in the .csproj is an odd one. Without it, for some reason, the gitlab build restores for x86 but tries to build for x64 which is... weird. Also, can't repro the issue locally. Really don't understand what's happening there, but this works so... 🙈Part of a stack of PRs: