Backport unified pipeline for APM SDKs for SSI artifacts #5868
Conversation
This file contains 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
randomanderson
added
area:builds
Datadog ReportBranch report: ✅ 0 Failed, 345417 Passed, 1754 Skipped, 14h 49m 39.57s 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 (5868) - mean (74ms) : 64, 84
. : milestone, 74,
master - mean (72ms) : 64, 79
. : milestone, 72,
section CallTarget+Inlining+NGEN
This PR (5868) - mean (1,032ms) : 1003, 1061
. : milestone, 1032,
master - mean (1,072ms) : 1045, 1098
. : milestone, 1072,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5868) - mean (110ms) : 106, 114
. : milestone, 110,
master - mean (109ms) : 105, 113
. : milestone, 109,
section CallTarget+Inlining+NGEN
This PR (5868) - mean (719ms) : 697, 741
. : milestone, 719,
master - mean (747ms) : 729, 764
. : milestone, 747,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5868) - mean (92ms) : 87, 96
. : milestone, 92,
master - mean (92ms) : 89, 94
. : milestone, 92,
section CallTarget+Inlining+NGEN
This PR (5868) - mean (670ms) : 646, 693
. : milestone, 670,
master - mean (704ms) : 677, 730
. : milestone, 704,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5868) - mean (192ms) : 189, 196
. : milestone, 192,
master - mean (192ms) : 189, 194
. : milestone, 192,
section CallTarget+Inlining+NGEN
This PR (5868) - mean (1,127ms) : 1099, 1155
. : milestone, 1127,
master - mean (1,169ms) : 1144, 1195
. : milestone, 1169,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5868) - mean (276ms) : 270, 281
. : milestone, 276,
master - mean (276ms) : 272, 281
. : milestone, 276,
section CallTarget+Inlining+NGEN
This PR (5868) - mean (892ms) : 865, 919
. : milestone, 892,
master - mean (914ms) : 896, 933
. : milestone, 914,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5868) - mean (266ms) : 262, 270
. : milestone, 266,
master - mean (265ms) : 260, 270
. : milestone, 265,
section CallTarget+Inlining+NGEN
This PR (5868) - mean (872ms) : 848, 897
. : milestone, 872,
master - mean (901ms) : 879, 922
. : milestone, 901,
|
Benchmarks Report for tracer 🐌Benchmarks for #5868 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 - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Slower
|
| Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 | 1.152 | 673.39 | 775.80 |
| Benchmark | base/diff | Base Median (ns) | Diff Median (ns) | Modality |
|---|---|---|---|---|
| Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1 | 1.182 | 635.11 | 537.21 | |
| Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net6.0 | 1.169 | 473.39 | 405.09 |
Raw results
| Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---|---|---|---|---|---|---|---|---|---|
| master | StartFinishSpan |
net6.0 | 473ns | 0.256ns | 0.99ns | 0.00798 | 0 | 0 | 576 B |
| master | StartFinishSpan |
netcoreapp3.1 | 635ns | 0.284ns | 1.1ns | 0.00761 | 0 | 0 | 576 B |
| master | StartFinishSpan |
net472 | 637ns | 0.719ns | 2.79ns | 0.0916 | 0 | 0 | 578 B |
| master | StartFinishScope |
net6.0 | 484ns | 0.218ns | 0.845ns | 0.00975 | 0 | 0 | 696 B |
| master | StartFinishScope |
netcoreapp3.1 | 673ns | 0.378ns | 1.46ns | 0.00939 | 0 | 0 | 696 B |
| master | StartFinishScope |
net472 | 889ns | 0.715ns | 2.77ns | 0.104 | 0 | 0 | 658 B |
| #5868 | StartFinishSpan |
net6.0 | 405ns | 0.158ns | 0.61ns | 0.0081 | 0 | 0 | 576 B |
| #5868 | StartFinishSpan |
netcoreapp3.1 | 537ns | 0.23ns | 0.891ns | 0.00765 | 0 | 0 | 576 B |
| #5868 | StartFinishSpan |
net472 | 631ns | 0.24ns | 0.931ns | 0.0916 | 0 | 0 | 578 B |
| #5868 | StartFinishScope |
net6.0 | 481ns | 0.117ns | 0.455ns | 0.00978 | 0 | 0 | 696 B |
| #5868 | StartFinishScope |
netcoreapp3.1 | 775ns | 0.831ns | 3.22ns | 0.00925 | 0 | 0 | 696 B |
| #5868 | StartFinishScope |
net472 | 911ns | 0.506ns | 1.96ns | 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 | 647ns | 0.477ns | 1.85ns | 0.00976 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
netcoreapp3.1 | 896ns | 0.286ns | 1.11ns | 0.00925 | 0 | 0 | 696 B |
| master | RunOnMethodBegin |
net472 | 1.14μs | 0.332ns | 1.2ns | 0.104 | 0 | 0 | 658 B |
| #5868 | RunOnMethodBegin |
net6.0 | 629ns | 0.194ns | 0.752ns | 0.00977 | 0 | 0 | 696 B |
| #5868 | RunOnMethodBegin |
netcoreapp3.1 | 901ns | 2.35ns | 8.77ns | 0.00925 | 0 | 0 | 696 B |
| #5868 | RunOnMethodBegin |
net472 | 1.1μs | 1.56ns | 5.83ns | 0.104 | 0 | 0 | 658 B |
andrewlock
reviewed
Aug 9, 2024
## Summary of changes Converts the Gitlab workflow to use the unified pipeline. The changes can be summarized as follows: * Removing Github actions that are now taken over by the unified pipeline * Removing Gitlab jobs taken over by the unified pipeline ## Reason for change By using the unified pipeline across all APM libraries, there is less maintenance hassle, fewer mistakes, and a more uniform creation of artifacts. ## Implementation details Artifacts are downloaded using the `download-single-step-artifacts` script. If the pipeline is a _release_, artifacts are downloaded from the Github release, otherwise, the script attempts to download artifacts from Azure. `prepare-oci-package` takes the artifacts and creates the OCI package. Everything else is handled by the unified pipeline. ## Test coverage Tests, other than manual tests, are mostly impossible with pipelines. <!--⚠️ 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. -->
randomanderson
force-pushed
the
landerson/backport-unified-pipeline
branch
from
824e51a
to
7abc602
Compare
andrewlock
approved these changes
Aug 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This is a backport of #5818 to the 2.x release branch. The main difference is that 2.x needs the init images for musl published under the
-musltag suffixReason for change
By using the unified pipeline across all APM libraries, there is less maintenance hassle, fewer mistakes, and a more uniform creation of artifacts.
Implementation details
I kept all of the changes from the original PR isolated to a single commit for ease of review. The goal of the changes was to do the minimal without having to change
one-pipelineupstream.download-single-step-artifacts.sh :
download the musl tarball from Github releases
prepare-oci-package.sh :
if
LIBCismusluse themusltarballgitlab-ci.yml :
package-ocimatrix without having dependency/artifact issues so a new job is created to build the musl oci package. This package is only used to create the lib injection image and the OCI package is not published anywhereARCHis abused to get the tag to beCOMMIT_SHA-musl. A multiach image is unneeded because only amd64 supports musl and the single arch image is tagged correctly.masterversion overrode one-pipeline to addmusltag to everything. That override is removedCOMMIT_SHA-muslas the source