Add NuGet packages

[pjanotti]

Why

Distribute automatic instrumentation as NuGet packages: it simplifies deployment for some scenarios and ensure assembly compatibility.

Fixes #1541 - Note: this PR doesn't take care of publishing the packages.

What

• Add build targets and CI jobs to build NuGet packages on every PR. The packages built from .csproj are built on a Windows runner to avoid requiring targeting packs for .NET Framework on the other runner.
• The packages are published as a CI artifact named OpenTelemetry.AutoInstrumentation.NuGet.Packages.
• Only two packages are intended for direct consumption by the user applications. The others are to facilitate bringing the correct dependencies and the build process. The two packages for application consumption are:
  • OpenTelemetry.AutoInstrumentation: brings all that is needed to automatically instrument an application.
  • OpenTelemetry.AutoInstrumentation.Dependencies: all dependencies used by automatic instrumentation. Intended for self-contained applications sharing an installation of OTel automatic instrumentation.

Follow-ups

• Refactor TestHelper: this change uses a temporary hack for the TestNuGetPackages target.
• Add a test using tracer-home and the Dependencies NuGet package (simulate shared deployment). This test was done manually, but, it will be good to have it on the normal integration tests.
• Add tests using the NuGet packages from Dockerfile.
• Publish packages directly to nuget.org.

Tests

• Added tests for the NuGet packages, separate from integration tests (that use bin/tracer-home) to ensure that only the NuGet packages are being used to instrument the test applications.

Checklist

- [ ] CHANGELOG.md is updated.

• Documentation is updated.
• New features are covered by tests.

[theletterf]

Some suggestions!

[Kielek]

I will back to this PR next week. For now only less important comments after scrolling through it.

[Kielek]

Before publishing, check if we are using the same file as SDK/contrib

[pjanotti]

Definitely, before going public we should review all of the NuGet properties. For instance: PackageIconUrl is actually deprecated (see open-telemetry/opentelemetry-dotnet#325). Related: @pellared suggested using a validation tool to ensure some properties on the NuGet packages.

[rajkumar-rangaraj]

I started with PR review and got few questions.

1. Is there a reason to pack OpenTelemetry.AutoInstrumentation.dll as a part of OpenTelemetry.AutoInstrumentation.Runtime.Managed package.
2. What design goal are we trying to achieve using OpenTelemetry.AutoInstrumentation.Runtime.Managed and OpenTelemetry.AutoInstrumentation.Dependencies. I see it is similar with additional libraries.
3. Does all libraries gets added to application bin directory. I did not try it yet.

[pjanotti]

@rajkumar-rangaraj

1. Is there a reason to pack OpenTelemetry.AutoInstrumentation.dll as a part of OpenTelemetry.AutoInstrumentation.Runtime.Managed package.

Two reasons: first is that I want the name OpenTelemetry.AutoInstrumentation to be the uber package that includes everything needed for automatic instrumentation. The second is that when dealing with the native runtime files (the CLR profiler native binaries) and arbitrary content files is easier to use .nuspec files while managing package dependencies is easier via csproj files. So I wanted to use OpenTelemetry.AutoInstrumentation.csproj to build the managed part but wanted the name for the uber package. Notice that only the 2 packages are intended to be used directly by the users: OpenTelemetry.AutoInstrumentation and OpenTelemetry.AutoInstrumentation.Dependencies, the Runtime packages are a kind of build artifact, similar to packages like this one.

2. What design goal are we trying to achieve using OpenTelemetry.AutoInstrumentation.Runtime.Managed and OpenTelemetry.AutoInstrumentation.Dependencies. I see it is similar with additional libraries.

When using NuGet packages there should be no need for the AdditionalDeps and SharedStore. I agree that these two packages are almost redundant with each other: the small difference is that the Dependencies one doesn't include the assemblies built by in our repo (small size savings for some scenarios). If we opt to keep them separate at a minimum we should change the projects in a way to have the dependencies listed in a single place - perhaps build Dependencies as a csproj and have OpenTelemetry.AutoInstrumentation reference it. We can address this issue we start building the packages, but, ideally before we publish them to nuget.org (NuGet packages are forever :)).

3. Does all libraries gets added to application bin directory. I did not try it yet.

Yes, all libraries get added to the application bin directory. This is good for usability but a bit wasteful from the "size" perspective.

[rajkumar-rangaraj]

It's still unclear to me why we need OpenTelemetry.AutoInstrumentation.Runtime.Managed as the Nuget package OpenTelemetry.AutoInstrumentation.Dependencies will have indirect reference to all the packages listed out in OpenTelemetry.AutoInstrumentation.Runtime.Managed. I guess we might be trying to resolve assembly redirect with .NET framework.

We are not making these Nuget as public yet. It's good to keep both of them and figure out later if we need both of these Nuget.

Excellent work!

[RassK]

@pjanotti, my only conclusive thought. Should we add an output cleanup task also to the follow-ups?
My issue was that, as an OTel user I would be very concerned if my output is filled with dlls I don't use in my app.
For eg finding MongoDB dlls from a usual ASP.NET Core + MSSQL app would rise a few questions in my mind.

Maybe we can:

1. Do compile time analysis which dlls to deploy.
2. Move OTel stuff to opentelemetry subfolder.

[pjanotti]

For eg finding MongoDB dlls from a usual ASP.NET Core + MSSQL app would rise a few questions in my mind.

That's a good point @RassK: I think we need to document and make explicit that by default libraries related to all possible instrumentations are going to be added when using the current NuGet packages. Then see if there is actual demand to reduce the size of the packages. Assuming that there is let's do a quick brainstorming to see what can be done to remove/control these extra dependencies when using the NuGet packages.

• User can explicitly exclude packages:

    <PackageReference Include="OpenTelemetry.Instrumentation.StackExchangeRedis" Version="1.0.0-rc9.8" ExcludeAssets="all" />
    <PackageReference Include="StackExchange.Redis" Version="2.1.58" ExcludeAssets="all" />

This is pretty tedious to be done manually given that most NuGet packages only have one asset and it requires identifying all items that one wants to exclude. At some level automation can help here, perhaps some build elements on the NuGet package could be helpful, i.e.: your suggestion number 1, but seems tricky.

• Re-org the packages;
  We could change the way we are packaging our assemblies and dependencies. A simple way would be to ship a package that only contains the auto-instrumentation assembly and no reference to the instrumentation packages. To keep things simple for both scenarios we should still ship an uber instrumentation package for users that are not worried about the extra assemblies, but, let the ones that want fine control over what assemblies are deployed specify which instrumentation packages they want to add. In a sense, it becomes a way to control which instrumentations are going to be enabled. It requires some re-work, but, seems a promising long-term direction.

Moving stuff to opentelemetry subfolder

This seems a bit trickier: would we have to put these in the NuGet package as content, and anyway still need to figure out what to remove add. So at first sounds like the trouble of idea number 1 without clear advantages.

[pjanotti]

@RassK your Q prompted me to open #2328