[Release] Remove Tracer MSI and Improve release notes categorization by pierotibou · Pull Request #2607 · DataDog/dd-trace-dotnet

pierotibou · 2022-03-28T09:13:43Z

Summary of changes

I added a new split in the release notes, in order to categorize by components (ie Tracer, Ci App..., by historical order). It's not perfect as it mixes fixes and changes when there is a component, but I wanted to get feedback before doing more complicated.
Doing so, I realized that we still had the Tracer Msi in the code base and that we needed to update the release processe with a few last changes.

Reason for change

Last release notes were loooonnnngggg. I really think we need a better categorization so I rewrote it manually: https://github.com/DataDog/dd-trace-dotnet/releases/tag/v2.5.1

Implementation details

The categorization relies on the label that is present on the PR.
Tracer msi removed and 'UpdateMsiContent' target reworked

Test coverage

None

Other details

Not sure I'm not missing something for the profiler-beta msi :)

andrewlock

Thanks, I like the look of this 🙂

andrewlock · 2022-03-30T12:28:03Z

      inputs:
        artifact: windows-msi-$(targetPlatform)
-        patterns: '**/*profiler-beta.msi'
+        patterns: '**/*.msi'


Note that previously this matched a single .msi file, and now it matches two, the x64 and x86 msi. I think we should either

Restrict this to the x64 installer, e.g. '**/*-x64.msi'

Use the platform-specific installer, e.g. '**/*$(targetPlatform).msi'

My preference is for the former, as I think it's what we were doing before, it's what most people will use (even when running in 32 bit mode), and the yaml will definitely work 🙂

Taking the currently in the windows-msi-x64 artifact as an example, we currently have:

datadog-dotnet-apm-2.6.0-x64-profiler-beta.msi

datadog-dotnet-apm-2.6.0-x64.msi

After the changes in this PR, only the datadog-dotnet-apm-2.6.0-x64.msi should remain so I think this is fine. I would feel more comfortable if this PR also removed the BuildMsi step from Nuke and the pipeline steps so only one step is responsible for building the installer. As the changes currently stand, both the BuildMsi and BuildMsiBeta both build the same output file so the last one to run overwrites the contents of the file.

Thanks for your inputs. I've fixed both in 6dc6dda.
That said, I've also added a new commit 9d28d5a where I remove completely the tracer (I should have separated all that from the release notes from the beginning). I'm not entirely sure if what I'm doing is correct, especially updating the UpdateMsiContentstarget but it felt right.
(Note: I haven't tested yet as I was on my Mac, I'll test the targets on my windows laptop now)

These changes look good. One way to test the UpdateMsiContents target is to delete the Tracer's .wxs files and run the target and observe no diff. You can remove the condition WIX_IS_NETFRAMEWORK_461_OR_LATER_INSTALLED which we determined was no longer necessary and you'll need to update the ComponentGroup which is generated by SyncMsiContent.cs

Ok I think we're good now

andrewlock

LGTM, well spotted on the MSI files update

zacharycmontoya · 2022-04-05T16:33:31Z

      inputs:
        artifact: windows-msi-$(targetPlatform)
-        patterns: '**/*profiler-beta.msi'
+        patterns: '**/*-x64.msi'


I think this should be /**/*-$(targetPlatform).msi or /**/*.msi because there should only be one MSI in the windows-msi-x64/windows-msi-x86 artifact folder now, and it ends with the same architecture suffix as the folder

zacharycmontoya

LGTM

and remove references to it. I've also upcated the `UpdateMsiContents`target. This part I'm not familiar of and I wonder if it was an issue not to be updated beforehand

andrewlock · 2022-04-05T23:04:11Z

Code Coverage Report 📊

✔️ Merging #2607 into master will not change line coverage
✔️ Merging #2607 into master will not change branch coverage
✔️ Merging #2607 into master will not change complexity

	master	#2607	Change
Lines	`13420` / `18467`	`13455` / `18467`
Lines %	`73%`	`73%`	`0%` ✔️
Branches	`7746` / `11018`	`7763` / `11018`
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`12309`	`12309`	`0` ✔️

View the full report for further details:

Datadog.Trace Breakdown ✔️

	master	#2607	Change
Lines %	`73%`	`73%`	`0%` ✔️
Branches %	`70%`	`70%`	`0%` ✔️
Complexity	`12309`	`12309`	`0` ✔️

The following classes have significant coverage changes.

File	Line coverage change	Branch coverage change	Complexity change

View the full reports for further details:

andrewlock · 2022-04-06T02:25:22Z

Benchmarks Report 🐌

Benchmarks for #2607 compared to master:

1 benchmarks are slower, with geometric mean 1.173
All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

Mann–Whitney U test with statistical test for significance of 5%
Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Allocated
master	`WriteAndFlushEnrichedTraces`	net472	946μs	5.23μs	31.4μs	3.16 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	706μs	3.36μs	15μs	2.57 KB
#2607	`WriteAndFlushEnrichedTraces`	net472	915μs	2.65μs	9.92μs	3.16 KB
#2607	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	703μs	2.75μs	12μs	2.57 KB

Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net472	293ns	1.4ns	5.59ns	0.0519	0	329 B
master	`AllCycleSimpleBody`	netcoreapp3.1	379ns	1.77ns	6.85ns	0.00454	0	328 B
master	`AllCycleMoreComplexBody`	net472	292ns	1ns	3.75ns	0.0481	0	305 B
master	`AllCycleMoreComplexBody`	netcoreapp3.1	372ns	2.03ns	13.8ns	0.00411	0	304 B
master	`BodyExtractorSimpleBody`	net472	437ns	2.15ns	8.86ns	0.057	0	361 B
master	`BodyExtractorSimpleBody`	netcoreapp3.1	447ns	1.96ns	7.58ns	0.00379	0	272 B
master	`BodyExtractorMoreComplexBody`	net472	24.3μs	119ns	534ns	1.18	0.0121	7.62 KB
master	`BodyExtractorMoreComplexBody`	netcoreapp3.1	21.3μs	115ns	683ns	0.0941	0	6.75 KB
#2607	`AllCycleSimpleBody`	net472	292ns	1ns	3.88ns	0.052	0	329 B
#2607	`AllCycleSimpleBody`	netcoreapp3.1	369ns	1.01ns	3.91ns	0.00449	0	328 B
#2607	`AllCycleMoreComplexBody`	net472	286ns	0.817ns	3.16ns	0.0482	0	305 B
#2607	`AllCycleMoreComplexBody`	netcoreapp3.1	359ns	1.01ns	3.91ns	0.00429	0	304 B
#2607	`BodyExtractorSimpleBody`	net472	432ns	1.44ns	5.38ns	0.057	0	361 B
#2607	`BodyExtractorSimpleBody`	netcoreapp3.1	432ns	2ns	8.01ns	0.00384	0	272 B
#2607	`BodyExtractorMoreComplexBody`	net472	24.2μs	118ns	470ns	1.19	0.0121	7.62 KB
#2607	`BodyExtractorMoreComplexBody`	netcoreapp3.1	20.7μs	58.8ns	228ns	0.0923	0	6.75 KB

Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
master	`SendRequest`	netcoreapp3.1	296μs	1.48μs	6.46μs	0.147	19.74 KB
#2607	`SendRequest`	net472	0ns	0ns	0ns	0	0 b
#2607	`SendRequest`	netcoreapp3.1	293μs	1.27μs	4.93μs	0.291	19.74 KB

Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net472	1.79μs	9.02ns	39.3ns	0.0936	0	594 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.51μs	5.46ns	21.1ns	0.00891	0	632 B
#2607	`ExecuteNonQuery`	net472	1.81μs	7.03ns	25.4ns	0.0935	0.000954	594 B
#2607	`ExecuteNonQuery`	netcoreapp3.1	1.54μs	7.34ns	41.5ns	0.00885	0	632 B

Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net472	2.89μs	10.7ns	40.1ns	0.128	0	803 B
master	`CallElasticsearch`	netcoreapp3.1	1.78μs	7.24ns	28ns	0.0113	0	792 B
master	`CallElasticsearchAsync`	net472	3.07μs	12.5ns	43.3ns	0.148	0.00159	939 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.81μs	5.03ns	19.5ns	0.0126	0	912 B
#2607	`CallElasticsearch`	net472	2.86μs	15.8ns	101ns	0.125	0	802 B
#2607	`CallElasticsearch`	netcoreapp3.1	1.75μs	6.41ns	22.2ns	0.0112	0	792 B
#2607	`CallElasticsearchAsync`	net472	2.9μs	13.7ns	56.6ns	0.149	0.00148	939 B
#2607	`CallElasticsearchAsync`	netcoreapp3.1	1.91μs	5.34ns	19.2ns	0.0131	0	912 B

Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteAsync`	net472	3.23μs	16ns	99.9ns	0.166	0	1.06 KB
master	`ExecuteAsync`	netcoreapp3.1	2μs	9.06ns	35.1ns	0.0146	0	1.03 KB
#2607	`ExecuteAsync`	net472	3.12μs	16.1ns	82ns	0.168	0.00149	1.06 KB
#2607	`ExecuteAsync`	netcoreapp3.1	2.04μs	10ns	40ns	0.0138	0	1.03 KB

Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net472	6.9μs	35.9ns	212ns	0.354	2.28 KB
master	`SendAsync`	netcoreapp3.1	5.07μs	27.7ns	164ns	0.0294	2.21 KB
#2607	`SendAsync`	net472	7.06μs	36.9ns	222ns	0.355	2.28 KB
#2607	`SendAsync`	netcoreapp3.1	4.86μs	17ns	66ns	0.0303	2.21 KB

Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	3.48μs	18.9ns	112ns	0.228	1.45 KB
master	`EnrichedLog`	netcoreapp3.1	3.09μs	11ns	41.2ns	0.0202	1.53 KB
#2607	`EnrichedLog`	net472	3.44μs	16.2ns	68.7ns	0.228	1.45 KB
#2607	`EnrichedLog`	netcoreapp3.1	3.09μs	7.67ns	29.7ns	0.0215	1.53 KB

Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net472	292μs	1.23μs	4.76μs	0.437	0.146	4.33 KB
master	`EnrichedLog`	netcoreapp3.1	233μs	1.11μs	4.46μs	0	0	4.21 KB
#2607	`EnrichedLog`	net472	291μs	930ns	3.48μs	0.31	0.155	4.33 KB
#2607	`EnrichedLog`	netcoreapp3.1	235μs	932ns	3.23μs	0	0	4.21 KB

Benchmarks.Trace.NLogBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #2607

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.NLogBenchmark.EnrichedLog‑net472	1.173	7,798.08	9,149.87

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	7.8μs	35.8ns	134ns	0.506	3.23 KB
master	`EnrichedLog`	netcoreapp3.1	6.78μs	28.7ns	111ns	0.0475	3.6 KB
#2607	`EnrichedLog`	net472	9.11μs	46.3ns	278ns	0.505	3.23 KB
#2607	`EnrichedLog`	netcoreapp3.1	6.42μs	12.5ns	46.9ns	0.0515	3.6 KB

Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendReceive`	net472	2.37μs	7.54ns	29.2ns	0.161	1.01 KB
master	`SendReceive`	netcoreapp3.1	2.16μs	12.2ns	85.7ns	0.0139	1.01 KB
#2607	`SendReceive`	net472	2.49μs	12.6ns	61.8ns	0.161	1.01 KB
#2607	`SendReceive`	netcoreapp3.1	2.16μs	10.4ns	43ns	0.0135	1.01 KB

Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net472	5.96μs	28.7ns	111ns	0.291	1.87 KB
master	`EnrichedLog`	netcoreapp3.1	5.22μs	25.5ns	102ns	0.0181	1.49 KB
#2607	`EnrichedLog`	net472	6.07μs	18ns	67.5ns	0.292	1.87 KB
#2607	`EnrichedLog`	netcoreapp3.1	5.22μs	26.1ns	111ns	0.0208	1.49 KB

Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net472	911ns	4.21ns	17.3ns	0.0717	457 B
master	`StartFinishSpan`	netcoreapp3.1	878ns	4.51ns	21.1ns	0.00603	456 B
master	`StartFinishScope`	net472	1.15μs	6.32ns	35.8ns	0.0843	538 B
master	`StartFinishScope`	netcoreapp3.1	1.06μs	5.22ns	22.1ns	0.00802	576 B
#2607	`StartFinishSpan`	net472	896ns	3.9ns	15.1ns	0.0719	457 B
#2607	`StartFinishSpan`	netcoreapp3.1	891ns	4.08ns	15.3ns	0.00631	456 B
#2607	`StartFinishScope`	net472	1.15μs	6.71ns	58.9ns	0.0842	538 B
#2607	`StartFinishScope`	netcoreapp3.1	1.04μs	5.06ns	20.3ns	0.00815	576 B

Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net472	1.27μs	3.9ns	15.1ns	0.084	538 B
master	`RunOnMethodBegin`	netcoreapp3.1	1.22μs	4.47ns	16.7ns	0.00816	576 B
#2607	`RunOnMethodBegin`	net472	1.31μs	6.12ns	30.6ns	0.0844	538 B
#2607	`RunOnMethodBegin`	netcoreapp3.1	1.28μs	15.8ns	158ns	0.00776	576 B

pierotibou requested review from a team as code owners March 28, 2022 09:13

This comment has been minimized.

Sign in to view

andrewlock approved these changes Mar 30, 2022

View reviewed changes

andrewlock reviewed Mar 30, 2022

View reviewed changes

Comment thread tracer/build/_build/Build.GitHub.cs

andrewlock reviewed Mar 30, 2022

View reviewed changes

Comment thread tracer/build/_build/Build.GitHub.cs

lucaspimentel approved these changes Mar 30, 2022

View reviewed changes

andrewlock approved these changes Apr 5, 2022

View reviewed changes

zacharycmontoya reviewed Apr 5, 2022

View reviewed changes

pierotibou changed the title ~~[Release] Improve release notes categorization and remove profiler-beta from the MSI name~~ [Release] Remove Tracer MSI and Improve release notes categorization Apr 5, 2022

pierotibou added 4 commits April 5, 2022 23:38

Add Components to release notes

3991bf6

remove profiler-beta from the suffix of the msi

bd968bc

remove unused var

c86663a

Remove Build MSI and specify what MSI to use in IIS tests

12d09eb

zacharycmontoya approved these changes Apr 5, 2022

View reviewed changes

pierotibou added 3 commits April 5, 2022 23:43

Remove the tracer installer

af1f586

and remove references to it. I've also upcated the `UpdateMsiContents`target. This part I'm not familiar of and I wonder if it was an issue not to be updated beforehand

implement Zach's feedback

ca37c4c

Fix after testing on windows

5e7a797

pierotibou force-pushed the pierre/rel-process-change-for-debugger branch from df11f77 to 5e7a797 Compare April 5, 2022 21:43

pierotibou merged commit a908775 into master Apr 6, 2022

pierotibou deleted the pierre/rel-process-change-for-debugger branch April 6, 2022 08:25

github-actions Bot added this to the vNext milestone Apr 6, 2022

pierotibou added area:builds project files, build scripts, pipelines, versioning, releases, packages area:installers labels Apr 6, 2022

pierotibou mentioned this pull request Apr 9, 2022

Add simplistic labeller #2662

Merged

Uh oh!

Conversation

pierotibou commented Mar 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

Uh oh!

This comment has been minimized.

This comment has been minimized.

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewlock Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Apr 4, 2022

Choose a reason for hiding this comment

Uh oh!

pierotibou Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Apr 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierotibou Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andrewlock left a comment

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Apr 5, 2022

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya left a comment

Choose a reason for hiding this comment

Uh oh!

andrewlock commented Apr 5, 2022

Code Coverage Report 📊

Datadog.Trace Breakdown ✔️

Uh oh!

andrewlock commented Apr 6, 2022

Benchmarks Report 🐌

Benchmark details

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #2607

Raw results

Raw results

Raw results

Raw results

Raw results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pierotibou commented Mar 28, 2022 •

edited

Loading

zacharycmontoya Apr 5, 2022 •

edited

Loading