Skip to content

[ClrProfiler] Guarantee that startup hook only runs at one callsite in IIS applications#775

Merged
zacharycmontoya merged 3 commits into
masterfrom
zach/fix/iis-startup-hook
Jul 2, 2020
Merged

[ClrProfiler] Guarantee that startup hook only runs at one callsite in IIS applications#775
zacharycmontoya merged 3 commits into
masterfrom
zach/fix/iis-startup-hook

Conversation

@zacharycmontoya

Copy link
Copy Markdown
Contributor

Modify the profiler so that, when running IIS or IIS Express on .NET Framework, the startup hook is only inserted in the System.Web.Compilation.BuildManager.InvokePreStartInitMethods. This has the following effects:

  1. This removes the guess-and-check nature of valid callsites for the startup logic when running IIS. Before, System.Web.Compilation.BuildManager.InvokePreStartInitMethods was called out as a valid callsite, but it was possible for other modules to be loaded and methods to be called before this, causing the startup logic to run in the incorrect place.
  2. This leaves ASP.NET Core apps hosted on IIS and IIS Express unchanged as they will still load at the earliest possible time in the setup of the ASP.NET Core application.
  3. For ASP.NET (.NET Framework) applications, additional AppDomains that are not the main AppDomain won't be automatically instrumented. This trade-off should be acceptable as any automatic tracing there would be disconnected from the original request anyways since it is separated by the AppDomain boundary.

@DataDog/apm-dotnet

…tup hook is only inserted in the System.Web.Compilation.BuildManager.InvokePreStartInitMethods. This means ancillary app domain's created by the application will not be instrumented, but this proves safer and such instrumented methods would be disconnected from incoming requests anyways as the Tracer instance would be separate if that automatic instrumentation actually happened.
… covers the case where an ASP.NET Core site is hosted in-process and ensures the startup hook runs inside that application to receive ASP.NET Core traces.
@zacharycmontoya zacharycmontoya added the area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native) label Jul 1, 2020
@zacharycmontoya
zacharycmontoya requested a review from a team as a code owner July 1, 2020 18:08
@zacharycmontoya zacharycmontoya self-assigned this Jul 1, 2020

@lucaspimentel lucaspimentel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More startup hook madness!

…ged Loader doesn't cause another startup hook insertion itself
@zacharycmontoya
zacharycmontoya merged commit 6a1abe0 into master Jul 2, 2020
@zacharycmontoya
zacharycmontoya deleted the zach/fix/iis-startup-hook branch July 2, 2020 04:15
@zacharycmontoya zacharycmontoya added this to the 1.18.2 milestone Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:native-library Automatic instrumentation native C++ code (Datadog.Trace.ClrProfiler.Native)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants