Skip to content

Merge auto-instrumentation code into Datadog.Trace.dll#1443

Merged
lucaspimentel merged 26 commits into
masterfrom
lpimentel/merge-projects
Aug 9, 2021
Merged

Merge auto-instrumentation code into Datadog.Trace.dll#1443
lucaspimentel merged 26 commits into
masterfrom
lpimentel/merge-projects

Conversation

@lucaspimentel

@lucaspimentel lucaspimentel commented May 3, 2021

Copy link
Copy Markdown
Member

I recommend looking at this PR one commit at a time. They are batched into big changes: move files, delete files, upgrade projects and pipelines, fix regressions, add tests, etc)

Goals

  • prevent future changes to internal APIs from becoming breaking changes when applications mix different versions of the installers and nuget packages
    • allows safer changes to internal APIs
    • unblocks several pending PRs which would otherwise incur breaking changes
    • enables further improvements to version mismatch scenarios (e.g. shared storage of active scope)
  • small bonus: method calls that previously crossed assembly boundaries are now candidates for method inlining

Changes

Merge the following projects into the Datadog.Trace project:

  • Datadog.Trace.ClrProfiler.Managed
  • Datadog.Trace.ClrProfiler.Managed.Core
  • Datadog.Trace.AspNet (note: we are leaving an empty Datadog.Trace.AspNet.dll with type-forwarding for backwards compatibility)
  • Datadog.Trace.Ci.Shared

This PR moves all code files below, while keeping the same namespaces to avoid changing type names.

Source directory Destination directory Namespace (unchanged)
src/Datadog.Trace.ClrProfiler.Managed src/Datadog.Trace/ClrProfiler Datadog.Trace.ClrProfiler
src/Datadog.Trace.ClrProfiler.Managed.Core src/Datadog.Trace/ClrProfiler Datadog.Trace.ClrProfiler
src/Datadog.Trace.AspNet src/Datadog.Trace/AspNet Datadog.Trace.AspNet
src/Datadog.Trace.Ci.Shared src/Datadog.Trace/Ci Datadog.Trace.Ci

Other changes to fix regressions caused by changes in this PR:

  • GenerateIntegrationDefinitions: ignore types that cannot be loaded. In particular, TracingHttpModule inherits from IHttpModule, which is not available to the nuke builds because they run on net5.0. (8ff2bd9)
  • In .NET Core, allow newer versions of Datadog.Trace.dll from native C++ code (6aa89f9)

Test results

I added some test apps and script to /test/test-applications/regression/MismatchedTracerVersions (link). The scripts can build multiple tracer versions with a single command, and run one of the apps by loading the specific tracer home version and nuget version.

For test results, see testing_results.md.

  • updated url to point to master, since this PR's branch was deleted
  • 2021-10-08 updated url after all tracer files were moved into /tracer folder

@lucaspimentel lucaspimentel added status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. type:refactor labels May 3, 2021
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 2 times, most recently from 9e63ba9 to f3af087 Compare May 5, 2021 23:04
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 5 times, most recently from 3466b98 to 7aaf656 Compare May 17, 2021 22:21
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 2 times, most recently from bddf4cc to 04fed8f Compare May 20, 2021 23:40
@lucaspimentel lucaspimentel added the area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) label May 20, 2021
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 3 times, most recently from 47597bd to 6ca8424 Compare May 26, 2021 03:50
@lucaspimentel lucaspimentel added status:do-not-merge Work is done. Can review, but do not merge yet. and removed status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. labels May 26, 2021
@lucaspimentel lucaspimentel changed the title [WIP] consolidate assemblies Merge projects and assemblies May 26, 2021
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 2 times, most recently from a90dd38 to 3f0a59d Compare May 26, 2021 04:03
Comment thread src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.NetCore.cs Outdated
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 4 times, most recently from 66c2e90 to 07bef5a Compare June 2, 2021 14:03
@lucaspimentel lucaspimentel force-pushed the lpimentel/merge-projects branch 3 times, most recently from 4f2fa21 to 915b6dc Compare June 3, 2021 20:29

@zacharycmontoya zacharycmontoya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Left a couple of comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) area:builds project files, build scripts, pipelines, versioning, releases, packages type:refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants