Don't cache the process instance for runtime metrics by kevingosse · Pull Request #1698 · DataDog/dd-trace-dotnet

kevingosse · 2021-08-24T11:55:03Z

A textbook case of premature optimization. Keeping the process instance doesn't save much (almost everything is recomputed when calling Refresh), yet adds significant memory pressure on applications with a large number of appdomains and threads.

This PR is set to merge into the `release/1.28.4` branch, which is based on the 1.28.2 release (set at the `v1.28.2` tag). To minimize risk while deploying fixes for customers, this PR is only porting the following changes to be released: - #1662 - #1651 - #1652 - #1666 - #1677 - #1698 - #1700 - #1702

Fixes crank pipeline on PR merge commits. (DataDog/dd-trace-dotnet#1669) * show folder content * changes. * Copies the managed profiler to the crank runners. Removes Datadog.Trace.ClrProfiler.Managed from the throughput project. * fix pipeline. * Fix app. * list loaded assembly. * Enable debug mode. * Force loader injection in calltarget scenarios. * Fixes assembly name. Disable CallSite scenario from Throughput tests. (DataDog/dd-trace-dotnet#1674) Add instrumentation for Begin/EndGetResponse to WebRequest (DataDog/dd-trace-dotnet#1658) * Add overload to scope factory that allows providing a start time * Add HttpWebRequest integration from BeginGetRequestStream, BeginGetResponse, and EndGetResponse BeginGetRequestStream can be used in both .NET Core and .NET Framework, but BeginGetResponse and EndGetResponse are .NET Framework only, because the .NET Core implementation uses HttpClient under the covers (which we already instrument) * Update WebRequestTests to handle additional scenarios * Fix rename service tests * Fix WebRequest20Tests to reflect additional instrumentation * Add a test for BeginGetResponse when Task.Factory.FromAsync is used * Simplify Task.Factory.FromAsync implementation * Update integrations json after rebase The v0 version of App Sec (DataDog/dd-trace-dotnet#1647) This PR implements a first version of App Sec. Please forgive the message history, it will be squashed away :) The App Sec module introduces two new environment variables to activate App Sec features: `DD_APPSEC_ENABLED` – enables the App Sec feature, any HTTP requests will be examined by the WAF (Web Application Firewall). By default, this feature is disabled. `DD_APPSEC_BLOCKING_ENABLED` – this controls whether the App Sec feature will block suspicious requests. By default, this feature is disabled. The app sec uses the ‘call target’ instrumentation to gather the data about the HTTP request that will be passed to the WAF. It is has not been implemented for ‘call site’ instrumentation and currently there is not plan to support this style of instrumentation. Data is gathered in the instrumentations for ASP.NET and ASP.NET Core and then passed to the ‘instrumentation gateway’ which starts the security flow. Currently the instrumentation gateway simply creates and event with the incoming data. In future version this component will be used to route the data to different security components based on the addresses associated with the data, but this functionality is not necessary for the first version. After the instrumentation gateway the data flow is as follows: ``` InstrumentationGateway -> Security -> PowerWaf -> Native -> Sqreen.dll ``` `Security` – this class coordinates the reaction to security events `PowerWaf` – this class provides .NET idiomatic wrapper to the WAF, as the WAF is implemented in C++ with C style interface `Native` – this class is responsible for finding, loading and providing low level bindings with the WAF `[lib]Sqreen.[dll|so|dylib]` – this are the native binaries containing the WAF implementation. Security events are reported to the agent and backend by `AgentWriter` class, which implements a basic queuing system. Two new samples are provided, that form the basis for the security integration tests. These are `Samples.AspNetMvc5` and `Samples.AspNetCore5` which are used to test ASP.NET and ASP.NET Core respectively. Integration tests based on similar principals to `Datadog.Trace.ClrProfiler.IntegrationTests` have been provided in `Datadog.Trace.Security.IntegrationTests` and are run as part of the integration tests in the `consolidated-pipeline` CI. ARM based Linux systems are not supported yet, but all other environment supported by the .NET libraries are. ASP.NET Core running on .NET Framework is not supported. The instrumentation is a no-op in these scenarios. This is because it though this unlikely to work, since we rely on referencing several `Microsoft.AspNetCore.*` dlls to be able instrument the ASP.NET Core pipeline. Co-authored-by: Anna Yafi <anna.yafi@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Native logger error fix (DataDog/dd-trace-dotnet#1677) Adds NGEN scenario on Throughput test. (DataDog/dd-trace-dotnet#1678) Removes code for Callsite scenario from the throughput tests. (DataDog/dd-trace-dotnet#1679) Small build improvements (DataDog/dd-trace-dotnet#1646) * Allow running an arbitrary dotnet sample app from Nuke i.e. it doesn't have to be part of the solution * Automatically build the "multiPackageProject" list from PackageVersionsGeneratorDefinitions.json There's no point building these twice, so previously we had a list of sample projects to not build. This simplifies things somewhat by loading the list dynamically * Try using a unique docker network per build to avoid issues with concurrent builds on arm64 * Upload artifacts to azure (for use in AAS extension automation) The latest file list can be downloaded from: https://apmdotnetci.blob.core.windows.net/apm-dotnet-ci-artifiacts-master/index.txt and the latest commit sha of the uploads can be downloaded from https://apmdotnetci.blob.core.windows.net/apm-dotnet-ci-artifiacts-master/sha.txt The blobs are stored in a subdirectory * Only push assets to Azure from master * Pull Azure storage location from variable Update CI Visibility specification (DataDog/dd-trace-dotnet#1682) * Update CI Visibility specification * refactor and fixes. replace "minimal" solution with a solution filter (DataDog/dd-trace-dotnet#1631) Add a custom test framework to monitor execution of unit tests in ducktyping library (DataDog/dd-trace-dotnet#1680) Add tests for changes to Datadog.Trace's Public API (DataDog/dd-trace-dotnet#1681) * Add tests for public API and public browsable (intellisense) API Note that we can't use Verify in Datadog.Trace.Tests because it uses v5.0.0 of the Logging abstractions library, which is unfortunately incompatible with v2.x of the aspnetcore libraries. Resorted to a poor man's version instead. * Add public API snapshots based on v1.28.2 (pre assembly merge) * Update snapshots _after_ assembly merge. This adds a _lot_ of surface area * Update snapshots for master * Make Security class internal * Add [Browsable(false)] and [EditorBrowsable(EditorBrowsableState.Never)] to all public APIs in the ClrProfiler library * Exclude non-browsable APIs from "public API" * Try making some things internal Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> * More updates * Fix test visibility Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> [Version Bump] 1.28.3-prerelease (DataDog/dd-trace-dotnet#1688) Co-authored-by: zacharycmontoya <zacharycmontoya@users.noreply.github.com> Adds Execution time benchmarks (DataDog/dd-trace-dotnet#1687) * Adds initial execution benchmarks * fixes * Adds sleep to the end of the pipeline. Produce NuGet package to deploy automatic instrumentation (DataDog/dd-trace-dotnet#1661) Produces a NuGet package that contains all of the automatic instrumentation assets that will be deployed to the application output directory. The current name is "Datadog.Instrumentation" Details: - TargetFrameworks: net45, net461, netstandard2.0, netcoreapp3.1 (same as Datadog.Trace) - Dependencies: Datadog.Trace - This means customers who try to restore an earlier version of `Datadog.Trace` will face a build-time NU1605 error (https://docs.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1605) - Contents: Publishes the following to application output |-- datadog | |-- linux-arm64 | | |-- Datadog.Trace.ClrProfiler.Native.so | |-- linux-musl-x64 | | |-- Datadog.Trace.ClrProfiler.Native.so | | |-- libSqreen.so | |-- linux-x64 | | |-- Datadog.Trace.ClrProfiler.Native.so | | |-- libSqreen.so | |-- net45 | | |-- Managed assemblies | |-- net461 | | |-- Managed assemblies | |-- netcoreapp3.1 | | |-- Managed assemblies | |-- netstandard2.0 | | |-- Managed assemblies | |-- osx-x64 | | |-- Datadog.Trace.ClrProfiler.Native.dylib | | |-- libSqreen.dylib | |-- win-x64 | | |-- Datadog.Trace.ClrProfiler.Native.dll | | |-- Sqreen.dll | |-- win-x86 | | |-- Datadog.Trace.ClrProfiler.Native.dll | | |-- Sqreen.dll | |-- createLogPath.sh | |-- integrations.json Additional changes: - Nuke changes: build BuildTracerHome [CreateInstrumentationHome] BuildInstrumentationNuget - CreateInstrumentationHomebuild target: Copies automatic instrumentation binaries from tracer home to the src/Datadog.Instrumentation/home directory, to be used by the Instrumentation, RunnerTool, and StandaloneTool projects - BuildInstrumentationNuget build target: Builds the Datadog.Instrumentation NuGet package with the binaries in the src/Datadog.Instrumentation/home directory - Removes src/Datadog.Trace.Tools.Runner/Datadog.Trace.Tools.Runner.proj - Add a sample application to samples/NugetDeployment directory with a docker-compose file to run the application on each platform supported by automatic instrumentation Appsec/anna/v0 integration/crankscenarios (DataDog/dd-trace-dotnet#1684) * crank scenarios for appsec * Reset sln configs * Include new yaml in sln fix json filename fix name of file for linux profile Scenarios according to acceptance criterias increase timeout limit logging rate restore ultimate pipeline increase timeout change order of tests to check increase timeout for linux Sharing profiles in another file to import Proxy calls to dl (DataDog/dd-trace-dotnet#1686) * Proxy calls to dl Some Linux distros don't have a version of libdl.so without a version prefix. Call from manage code will fail to find the library on these distros. Proxying via a C++ library should cure this, because the C libraries will handle loading in this case. * Add linker command for libdil * Rename exported fuctions to avoid strange issue * Rewrite PInvoke tables for the native methods used by App Sec too * Correct build error on windows Co-authored-by: Robert Pickering <robertfpickering@fastmail.com> Add end-to-end integration tests for logs injection (DataDog/dd-trace-dotnet#1637) New Tests - `LogsInjection.Log4Net`, `LogsInjection.NLog`, `LogsInjection.Serilog` - Behaviors - Logs injection occurs with raw logs and JSON logs - After logs injection setup, a method is invoked another AppDomain to test LogicalCallContext storage - Libraries tested - log4net - .NET Framework: 1.2 - 2.x - .NET Core: 2.0.6+ - NLog - .NET Framework: 1.x - 4.x - .NET Core: 4.5+ - Serilog - .NET Framework: 1.x - 2.x - .NET Core: 2.x - Caveats - **Note**: Windows testing currently does not test multiple package versions. That will need to be enabled at a later point in time - All log4net versions are still vulnerable to serialization issues when crossing AppDomains, so the cross-AppDomain scenario is skipped in the log4net test application Deleted Tests - `Log4Net.SerializationException` regression test (and helper library ApplicationWithLog4Net) - The regression test was added in DataDog/dd-trace-dotnet#551 but is now solved by adding new sample applications that directly cause calls to cross AppDomain boundaries - `NLog10LogsInjection.NullReferenceException` regression test - The regression test was added in DataDog/dd-trace-dotnet#614 but is now solved by the new type FallbackNLogLogProvider Fixes - Disable Serilog logs injection support when the detected Serilog version is earlier than 2.0. This is enforced by adding a new `NoOpSerilogLogProvider` log provider to perform no-ops in this scenario - Add better exception handling in `CustomLog4NetLogProvider.ExtJsonAssemblySupported()` Add Microsoft.Extensions.Logging.ILogger instrumentation (DataDog/dd-trace-dotnet#1663) * Add LogsInjectionTestBase from open-telemetry#1637 * Add ILogger integration When a logger iterates the scopes for a log message we insert ourselves as the first entry. Note that we don't _really_ add a scope (by instrumenting `BeginScope()` or similar) as that would insert our scope in the "scope stack", and make us responsible for removing it at the appropriate time. The approach here is much simpler, and works for all types of instrumentation that are using the standard ILogger infrastructure This essentially works the same way as the "activity" ILogger integration does in the runtime (https://github.com/dotnet/runtime/blob/v5.0.0/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactoryScopeProvider.cs#L36-L52) * Update profiler skipped assembly prefixes We need to instrument Microsoft.Extensions.Logging.Abstractions now, so can't skip the Microsoft.Extensions prefix This adds most of them to the skip list, but there may be a better approach, e.g. an additional "don't skip" list? * Add LogsInjection aspnetcore sample * Add tests for ILogger logs injection * Update ILogger integration to inject dd_service etc even if we don't have a trace_id * Inject dd_span when a span is available Update tests to verify span_id is injected correctly * Update integrations.json after rebase * Convert DatadogLoggingScope indexer to switch expression * Add required [Browsable(false)] and [EditorBrowsable(Never)] attributes to ilogger integration Reduce snapshot file path length (DataDog/dd-trace-dotnet#1696) * Use numeric status code in snapshots + a shorter name numeric values are (mostly) shorter than the long names, and every test is called SubmitTraces, so don't need to include that in the filenames * Remove method name from aspnetcore snapshots to reduce file length * Rename snapshots Merging repos: 'dd-shared-components-dotnet' into 'dd-trace-dotnet'. (DataDog/dd-trace-dotnet#1694) This is the first of several self-contained PRs targeted at moving the Shared Code (as in _shared between the Tracer and the Profiler_) from [dd-shared-components-dotnet](https://github.com/DataDog/dd-shared-components-dotnet) into [dd-trace-dotnet](https://github.com/DataDog/dd-trace-dotnet). For this PR, I created a branch `permanent/MoveFrom_dd-shared-components-dotnet` and moved all files from the `dd-shared-components-dotnet` into there, including their complete change history. The branch will remain permanently as a history record, and we will squash-merge this PR into the `master` branch. The files are all inside the `/shared` folder, so that they can be naturally merged into the planned joint directory structure. In addition, this PR also makes the following small changes: * Make whatever tweaks necessary so that _all_ the Shared Code projects build inside `dd-trace-dotnet`. * Set up placeholder folders for the `/tracer` and the `/profiler` products to facilitate the directory refactoring in the subsequent PRs. * Update ReadMe files to describe the changes. Note that this PR keeps the directory structure within the `/shared` folder exactly as it was within the original repo. Subsequent changed will adjust this structure to joint conventions as required. This will require coordination with the profiler build configuration which makes some assumptions about the layout of the relevant folders. This change is self-contained. It will not affect any existing Tracer components, and it will allow the overall repo merge project to proceed in several steps that can be validated independently. This PR will be followed up with additional PR to address the following: * Move Tracer-specific files into the `/tracer` folder. * Ensure that changes contained to shared files that are not (yet) used by the Tracer do not require the Tracer CI validation to pass (this will be updated in the future if and when the Tracer uses more shared components) * Joint build of components used by several products. * Joint release package. * Joint conventions for folder structure. * Etc... Sending more relevant data to backend (DataDog/dd-trace-dotnet#1695) * Adding more data and filtering cookie from the no cookie header waf address Remove usage of non-builtin types in Attribute named arguments (DataDog/dd-trace-dotnet#1601) To workaround an issue discovered in Azure Functions, remove all usages of named arguments in Attributes that are not built-in types. This means removing arguments that use our own custom enums, but keeping arguments that use `string`, etc. While rare, the failing scenario occurs when the corelib attempts to create the `Attribute` object from the metadata but the assembly that gets resolved from the default load context (or one of its handlers) does not resolve to the same assembly as the one we expected, causing type equality to fail. Note: This workaround is only required for attributes that will be instantiated from metadata, like `DuckAttribute`, but it is easier to maintain a broader rule that we can always revisit later. Specific changes include: - Replacing all usages of `[Duck(Kind = DuckKind.Field)]` with a new attribute `[DuckField]` - Replacing the one usage of `[InterceptMethod(MethodReplacementAction = MethodReplacementActionType.InsertFirst)]` with `[InsertFirstInterceptMethod]` - Adding a unit test to continuously maintain this coding guideline Don't cache the process instance for runtime metrics (DataDog/dd-trace-dotnet#1698) Add tracer metrics to startup logs (DataDog/dd-trace-dotnet#1689) Use AppDomain.CurrentDomain.BaseDirectory to get current directory for configuration (DataDog/dd-trace-dotnet#1700) Environment.CurrentDirectory does not necessarily point to the directory containing the application. The most obvious example is Windows Services, where Environment.CurrentDirectory points to C:\Windows\System32 Using AppDomain.CurrentDomain.BaseDirectory seems to be a better (and safer) alternative. Tested the following scenarios, and they behave as expected * asp.net sites, (iis express + iis) * asp.net in a virtual directory ((iis express + iis) * Owin self hosted * Owin hosted in a windows service * ASP.NET Core app hosted in iis * `dotnet run` on an asp.net core app (returns the bin directory, whereas Environment.CurrentDirectory returns the app directory/content root) * `dotnet dll`/running .exe directly. Returns app directory instead of the console's current directory (e.g. if entering full path to exe) Prepare the `/shared` folder for consumption by the Profiler. (DataDog/dd-trace-dotnet#1697) This change refactors the file structure within the top-level `/shared` folder into its long-term state. Specifically: * Modify the directory names inside of the top-level shared directories according to team design discussions (lower case). * Simplify references to project-external (shared) `.cs` files. * Clean up relative directory references. * Update T4 auto-generated files. * Add ReadMe information. * Ensure that all shared projects build cleanly. This is the [key ReadMe file describing the shared sources](https://github.com/DataDog/dd-trace-dotnet/tree/macrogreg/master/shared/src#readme). It explains things in detail. It is located under `/shared/src/README.md`. The original PR has a screen shot the folder, drilled in up to the individual libraries: DataDog/dd-trace-dotnet#1697 New version of the WAF (DataDog/dd-trace-dotnet#1692) New version of the WAF The latest version of the C++ binary that analyses the incoming web requests. The configs have moved to a yaml, I have included a test rule, but there will be another later PR that will add yaml parsing and a real default rule set. Add fix for Samples.MultiDomainHost.App.NuGetHttpWithRedirects to send test HTTP traffic to a local HTTP listener instead of contoso.com (DataDog/dd-trace-dotnet#1691) Add PreserveContext attribute for async integrations (DataDog/dd-trace-dotnet#1702) Don't trigger Tracer-specific CI for changes to shared assets not used by the Tracer (DataDog/dd-trace-dotnet#1701) [Version Bump] 1.28.5-prerelease (DataDog/dd-trace-dotnet#1709) Co-authored-by: zacharycmontoya <zacharycmontoya@users.noreply.github.com> In the integration_tests_arm64 stage, replace COMPOSE_PROJECT_NAME with the projectName input on the DockerCompose Azure DevOps task (DataDog/dd-trace-dotnet#1715) This should correctly set the project name because the task's projectName input takes precedence over the environment variable with the -p option, and the task will always set a projectName input even if not specified. Don't fail the build if uploading core dumps fails (DataDog/dd-trace-dotnet#1722) By adding `continueOnError: true` to the log upload stage Also removed some "duplicate" uploads for consistency Create new docker-compose services only for arm64 that exposes the ports only via linked services (DataDog/dd-trace-dotnet#1719) This allows multiple stacks to be hosted on one machine, as is the case in the arm64 integration test runs Fix flakiness in Datadog.Trace.ClrProfiler.IntegrationTests.WebRequestTests.SubmitsTraces (DataDog/dd-trace-dotnet#1718) As part of the test, the trace ID and span ID of `spans.First()` is compared against the first distributed tracing headers seen by the in-memory HttpListener. In the most recent test failure, the trace ID comparison failed, and I suspect it's because the in-memory list of spans was not ordered by start time. Ordering the spans should solve this. Enable code coverage for security tests (DataDog/dd-trace-dotnet#1699) * Enable code coverage for security tests * Enable aspnetcore5 tests * Try fixing RabbitMQ coverage Add support for Aerospike (DataDog/dd-trace-dotnet#1717) Update name of NuGet package carrying automatic instrumentation assets (DataDog/dd-trace-dotnet#1720) * Rename Datadog.Instrumentation to Datadog.Monitoring.Distribution, and rename all "instrumentation" references to "distribution" * Update NuGet Deployment test to not expose the Datadog agent to the host, only to the running containers Add FmtLib debug files that were previously forgotten during import of shared files. (DataDog/dd-trace-dotnet#1725) The recent merge of the `dd-shared-components-dotnet` repo included the native [fmt library](https://github.com/DataDog/dd-shared-components-dotnet/tree/master/src/Shared-Libs/Native). During the merge, we included the release version of that library, but we forgot the [debug files](https://github.com/DataDog/dd-shared-components-dotnet/tree/master/src/Shared-Libs/Native/fmt_x64-windows-static/debug/lib). Now, we need to have the Profiler use the shared files from their new location in `dd-trace-dotnet`. However, that is currently not building correctly, because the fmt debug files are missing. This change adds the missing files. There is no other functional change, beyond unblocking the Profiler build. Define 'SharedSrcBaseDir' consistently across projects. (DataDog/dd-trace-dotnet#1724) Several projects in `/shared` and in `dd-continuous-profiler-dotnet` define the `SharedSrcBaseDir` MSBuild property to refer to the folder that has recently been moved to `dd-trace-dotnet\shared\src\managed-src\`. Previously, projects were using different ways to refer to that same folder, which was fragile. This change switches to a consistent reference based on the existing `EnlistmentRoot` MSBuild variable. Add a beta suffix to the Datadog.Monitoring.Distribution NuGet package (DataDog/dd-trace-dotnet#1728) Add a beta01 suffix to the Datadog.Monitoring.Distribution while we ensure it's production-ready Implement shutdown to help fix app sec test flakiness (DataDog/dd-trace-dotnet#1713) Package windows native symbols in a separate archive (DataDog/dd-trace-dotnet#1727) * Package windows native symbols in a separate archive * Only publish symbols in gitlabs Native memory improvements (DataDog/dd-trace-dotnet#1723) * changes * Skip Dynamic and Resource modules from CallTarget. Other improvements. * Fix linux build * Revert RequestRejitForNGenInliners call move. * Move string allocations to contants * Changes based in the review. * Simplify the ModuleLoadFinished callback on CallTarget * use constant * fix test project. * Changes based in the review. Add a previously forgotten comment to a shared editorconfig file. (DataDog/dd-trace-dotnet#1735) Downgrade severity of a shared editoconfig rule to match ongoing Profiler work. (DataDog/dd-trace-dotnet#1736) (The modified `.editorconfig` currently only applies to files used by the Profiler.) Disable AppSec crank till a new runner machine can be created (DataDog/dd-trace-dotnet#1739) Implement 1.0.7 of libddwaf (DataDog/dd-trace-dotnet#1732) * Implement 1.0.7 of libddwaf Our C++ WAF implementation, libddwaf - previously PowerWaf / libSqreen, has been though a big clean up of their APIs and the format of configuration file. This commit synchronizes the API signatures and naming of methods / objects. The details of the new API can be found here: https://github.com/DataDog/libddwaf/blob/master/include/ddwaf.h Disable ILogger integration by default (DataDog/dd-trace-dotnet#1738) * Disable ILogger integration by default In order to better verify the performance impact of the ILogger integration, disable the integration by default, by adding an additional feature flag. * Fix configuration tests broken by disabling ilogger by default Correct memory management for interactions with libddwaf (DataDog/dd-trace-dotnet#1742) I had assumed that libddwaf took ownership of objects assoicated with it and therefore free them when context objects where freed. However this isn't the case, all objects allocated by the C# code for the ddlibwaf need to be explicitly freed. This PR does that in a simple way. Revert "Disable ILogger integration by default (open-telemetry#1738)" (DataDog/dd-trace-dotnet#1744) This reverts commit fdbab1aef49b79cc8da9ae3a89d041f2c3ab51ca. Header keys should be lower case (DataDog/dd-trace-dotnet#1743) [Version Bump] 1.28.6 (DataDog/dd-trace-dotnet#1745) * [Version Bump] 1.28.6 * Update Changelog Co-authored-by: andrewlock <andrewlock@users.noreply.github.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Minor tweaks to shared files (DataDog/dd-trace-dotnet#1729) As we are cleaning up the Profiler build to reference `dd-trace-dotnet` instead of `dd-shared-components-dotnet`, we discover the need to minor tweaks. This PR is the collection of such tweaks which I collected into a single PR to minimize churn. The Profiler work is now done (https://github.com/DataDog/dd-continuous-profiler-dotnet/pull/135). Note that while this PR is being reviewed and merged, the Profiler builds cleanly while referencing `dd-trace-dotnet / macrogreg/SharedItemsCatchup`. Once this PR is merged, we must point the profiler to `dd-trace-dotnet / master`. Changes here: * Make packages path the same for all shared items. * Props files must use relative paths to construct shared file references. * Fix upward walking chains in `Directory.Build.props` files. Add minimal test applications that use service bus libraries (DataDog/dd-trace-dotnet#1690) Adds a test application and smoke test for the following service bus libraries. This tests that automatic instrumentation does not encounter issues with the core components of the service bus libraries. - MassTransit - NServiceBus - Rebus [DuckType] - Improve generic methods support. (DataDog/dd-trace-dotnet#1733) * Adds support for Types with generics types inside a generic parameters method. * Changes in the tests. * Update src/Datadog.Trace/DuckTyping/DuckType.Methods.cs Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Update src/Datadog.Trace/DuckTyping/DuckType.Methods.cs Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Remove unused ISpan/IScope (DataDog/dd-trace-dotnet#1746) Fix Benchmarks build break from removing Datadog.Trace.Abstractions namespace (DataDog/dd-trace-dotnet#1749) Fix compilation directive for NET5.0 (DataDog/dd-trace-dotnet#1731) Exclude liblog from code coverage by filepath (DataDog/dd-trace-dotnet#1753) Adding the "[ExcludeFromCodeCoverage]" attribute apparently causes all sorts of issues Refactor ILogger integration (DataDog/dd-trace-dotnet#1740) * Move ILogger instrumentation into Logging sub-folder We are going to likely have shared components related to ILogger so makes sense to move it now * Call ToString() on SpanId and TraceId Ensures that they are formatted as strings when using custom formatters * Add benchmark for logs injection * Add a sample app demonstrating how to use the ILogger integration The ILogger integration requires auto instrumentation, so uses the Datadog.Monitoring.Distribution to set that up in an easy way * Output a better format string for ILogger Logging Scope Some loggers don't call `GetEnumerator()` on scopes, and instead just call ToString(). Previously we were outputting empty span_id and trace_id values, which looked strange, and might break log parsers etc In addition, according to [this comment](https://github.com/DataDog/dd-trace-dotnet/blob/master/samples/AutomaticTraceIdInjection/SerilogExample/Program.cs#L26), log parsing requires the properties to be a JSON map, so to improve the chance of correct usage, this changes the ToString() value to be a json map. * Update DatadogLoggingScope.ToString() to not be a JSON object Refactor folder locations [latest] (DataDog/dd-trace-dotnet#1748) This PR moves almost all of the code related to development/test/build of the .NET Tracer into a `/tracer` directory to allow for better code partitioning when we add more products into the repo. File changes include: - Directories moved from `/` to `/tracer/` - build - samples - src - test - Individual files moved from `/` to `/tracer/` - Datadog.Trace.proj - Directory.Build.props - GlobalSuppressions.cs - build.cmd - build.ps1 - build.sh - build_in_docker.sh - integrations.json - stylecop.json - Intermediate artifact folders moved from `/src/bin` to `/tracer/src/bin` - artifacts - managed-publish - windows-tracer-home Fix incomplete update to Datadog.Trace.sln for latest sample projects during tracer folder move (DataDog/dd-trace-dotnet#1759) Remove CallTarget Integrations from json file. (DataDog/dd-trace-dotnet#1693) * Initial commit * Fixes x86 PInvoke calls. * fix after merge master * Integrations definition generator. * Changes based on review. * Changes. * Adds Dispose to the NativeCallTargetDefinition instance. * Change WStr("") to EmptyWStr constant * Rollback unnecessary changes. * Rollback unnecessary changes. Fix broken paths after repository move (DataDog/dd-trace-dotnet#1762) * Fix paths in GitHub workflows They were broken during the repository re-arrange * Fix paths in honeypot yml * Remove unused (incorrect) paths from third-party-test-suites They were wrong, but don't appear to have been used, so removed them * Fix path for GitLab build Move tracer snapshots to /tracer/test/snapshots directory (DataDog/dd-trace-dotnet#1766) This should help avoid long path issues with the snapshot files that have extremely descriptive names. Synchronously wait for tasks in HttpClient sample (DataDog/dd-trace-dotnet#1703)

* Share a function to send all headers between core / framework * socket ip address in remote ip property for net framework * Add environment value for DD_ENV * ip extractor algorithm * send correct ip address> local resolution, send extra headers * Unit tests * remove some changes in sln * Fixing sln * Changes after Roberts comments Fix unit tests according to new modifications * Don't send null values to backend #1698 * Actor should contain the resolved ip , Request should contain the peer IP * Add unit tests, remove port and send more headers * x-client-ip instead of client-ip * Dont assign port if no ip, fixes to comments

* Catch object disposed exception in Samples.HttpMessageHandler (#1774) Likely culprit for test failures in the execution benchmarks (only happens on .NET 5 interestingly) * Used the parsed version of the query string (#1777) Used the parsed version of the query string APPSEC-1404 Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * [Shared-Loader] - Add support for Module Entrypoint rewriting and .NET 4.5 fix (#1755) * Add support for Module Entrypoint rewriting and .NET 4.5 fix * Add environment variables to control loader options * Fix warnings and linux build * Debug logs on flat layout * testing a change in the algorithm for flat layout * Moar debugging data * Add missing LogDebugIsEnabled flags * Change feature flags to follow the tracer pattern * update code owners (#1750) * Update CODEOWNERS (#1785) * Update .NET SDK to 5.0.401 (#1782) * Update SDK version to 5.0.401 * Fix build_in_docker.sh script * Update version of alpine alpine 3.12 is no longer supported by .NET 5. Update to 3.14 (which will be lowest supported for .NET 6) * Disable Verify's clipboard and diff tool integration Causes failures in Alpine as the xsel tool isn't available * Try downgrading alpine for now * Rename ADO.NET providers integration name (#1781) * Rename provider integration names to ADO.NET to use the same behavior as CallSite. * Removes IntegrationName from IAdoNetClientData * Refactor InstrumentationDefinitions generator (#1780) * Refactor InstrumentationDefinitions generator. * Update tracer/build/_build/PrepareRelease/GenerateIntegrationDefinitions.cs Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Add support for Elasticsearch 7 (#1760) * Fix percentages in coverage report (#1793) * Fix percentages in coverage report * Include a link to the online Code Coverage report in the code coverage comparison comment * Increase timeout in MassTransit smoke tests (#1779) * Don't duplicate DB context configuration in MassTransit config * Remove unused variable * Try increasing command timeout to avoid flaky test * Enable shared logger for managed log file (#1788) `shared: true` should be set when multiple process may write to the managed log file. * Clean up old benchmark branches (#1792) This will automatically delete all but the specified "baseline" branch (1.27.1 currently) and the most recent 2 branches (the previous, plus the newly created benchmark branch) when auto-tagging the version bump commit. This should ensure we only have 3 active benchmark branches at any one time, reducing the burden on CI (The bash manipulation and renaming logic has all been tested and run locally) * Fix the reference to mscorlib (#1797) * Env var to override default AppSec rules (#1757) Allow users to specify their own rules file to change how AppSec will analyze request. Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> * Fix indentation in yaml (#1801) * Fix duplicating integrations due to multiple Initialize calls from different AppDomains. (#1794) * Fix duplicating integrations due to multiple AppDomains. Fix memory leak on NativeCallTargetDefinition disposal. * Add comments. * fix locks. * Restore Tracer.ActiveScope in ASP.NET when request switches to a different thread (#1783) ## Issue The existing ASP.NET instrumentation uses an HttpModule to handle the BeginRequest event at the beginning of a request. The callback creates a new scope, stores it in the execution context via `Tracer.ActiveScope`, and stores it inside the `HttpContext.Items` dictionary. Except for a few integrations like ASP.NET MVC, ASP.NET Web API 2, and WCF, all other integrations will only correlate their spans to the root span when the `Tracer.ActiveScope` property (which uses the execution context) is populated. The issue is that ASP.NET may switch threads when handling requests. When it does this, it will correctly set the `HttpContext.Current` property on the new thread but the execution context of the new thread IS NOT identical to the previous execution context, causing `Tracer.ActiveScope` to be `null` or possibly incorrect. ## Fix The fix is to add additional automatic instrumentation in the request pipeline to reset the `Tracer.ActiveScope` property. We've identified that the methods `System.Web.ThreadContext.AssociateWithCurrentThread` and `System.Web.ThreadContext.DisassociateFromCurrentThread` are responsible for managing HttpContext assignments, so these methods can be instrumented to extract the scope placed in the HttpContext and appropriately store/remove the `Tracer.ActiveScope`. ## Changes - Add CallSite and CallTarget instrumentation on the following `System.Web` methods: - `System.Web.ThreadContext.AssociateWithCurrentThread`: Gets the scope from the `HttpContext.Items` dictionary and sets the `Tracer.ActiveScope` property - `System.Web.ThreadContext.DisassociateFromCurrentThread`: Unsets the `Tracer.ActiveScope` property - Add a regression test that deterministically forces a thread switch and ensures that the two spans are included in the same trace * Additional update to Web Api 2 tests (#1772) This is a continuation of PR https://github.com/DataDog/dd-trace-dotnet/pull/1734 to ensure that we test the same routes in both the OWIN Web API 2 sample and the ASP.NET Web API 2 sample. Changes: - Add message handler requests to the OwinWebApi2 test cases with updated snapshots - Add expected span counts to the test cases to ensure the test waits for the proper amount of spans from the MockTraceAgent * Azure Functions Instrumentation (#1613) * Azure Functions Instrumentation * Share code between diagnostic observer and middleware instrumentation * Use generic constraint * Skip diagnostic observer in functions * Rebase and new instrumentation definitions structure * Remove unused class * New API * Missed sharing RequestTrackingFeature * Use the "root" service name in all ILogger Logs (#1798) Previously we were using the active span's service name in the logs. In other ILogger injectors we only use the "application" service name. This updates the ILogger integration to do the same. * Fix a path in debug build of managed loader which is required after the recent repo restructure. (#1806) * Stop copying profiler/tracer home to test directories (#1808) * Add steps to clean the packages directory and obj files This is a horrible hack to try and get some space back in our integration and regression tests. But sometimes you gotta do whatcha gotta do. * Remove unused methods * Fix incorrect path in GetProfilerProjectBin() * Assume that we always have the tracer home directory (which can be overriden set with the environment variable TracerHomeDirectory I decided _not_ to just use the default DD_DOTNET_TRACER_HOME here, as thought it might cause extra confusiong for local testing, but I'm open to suggestions on this one Note that this requires you run `build BuildTracerHomer` before running integration tests _even in Visual Studio_ etc. This is not ideal, but was the quickest solution for now * Stop copying the profiler around in integration tests etc Removed all these properties: ExcludeManagedProfiler ExcludeNativeProfiler LoadManagedProfilerFromProfilerDirectory ManagedProfilerOutputDirectory ProfilerOutputDirectory * Don't reference Datadog.Trace indirectly in the AutomapperTest project * Revert "Add steps to clean the packages directory and obj files" This reverts commit 7fc1c101b6ff1105bce328ea3aa5e73eeefee3cb. * Fix default env vars in docker-compose * Fix Arm64 reference * Remove unused variables * Fix alpine Waf copying wrong version into tracer home * Store benchmark results as an artifact (#1799) * Add json exporter to benchmarks * Upload benchmark results as an artifact for later processing * Ignore the build_data folder (used in CI) * Add default exporters * Remove manual exporters * Fix exporter * [Appsec] Fix posting json missing with HttpStreamRequest: missing header / reset streams (#1778) * Fixed postjson for HttpStreamRequest * Fix headers * Request could be null if nothing has been posted * No need for headers * using stringbuildercache * missing implementation * Changing logging request in case of error * remove request content implementation * removing impl from benchmark * - local function to static function to avoid capturing - request turned to local object - some omitted config await false * Missing configure await false Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Fix Execution Benchmarks job filepath (#1810) * Fix tracer path * Ensure all sdks are installed. * Update test spans from Crank runs (#1592) Changes crank spans in the following ways: - Sets tag `env=ci` - Add `Crank.` prefix to the "Test Suite". For example, `calltarget.linux_arm64.arm64` will become `Crank.calltarget.linux_arm64.arm64` * Add a native API to parse boolean environment variable values (#1815) * Don't copy the logs for benchmarks (#1814) Apparently it takes 15 mins to copy them! * [app sec] fix bug when debug logs are enabled (#1796) * Fix a bug where app sec failed in debug log mode because of a failure formatting a value Also, - Move some logs over to structured logging - Remove native log for dlerror - Use http for samples Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Parallelize throughput tests (#1773) * Small fix for some paths in _build.csproj (#1812) * Fix some paths in _build.csproj since folders structure has changed * Propagate sampling priority to all spans during partial flush (#1803) * JITInline callback refactor to fix race condition. (#1823) * Restore the original env-var value before asserting. (#1816) * Restore the original env-var value before the assert * fix test * Changes from review. * Fixes the test when dogfooding CIApp with a custom AgentUri * Update tracer/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Fix elasticsearch v7 and GraphQL file locations (#1821) I think these must have got messed up during a rebase after the directory move * Disable memory dumps in CI (#1822) * Disable NGEN images on .NET Framework 4.5 (#1826) * Adds support for .NET Framework 4.5 in the native loader. (#1825) * Adds support for .NET 4.5 in the native loader. * initialize mask variables. * [Version Bump] 1.28.8 (#1827) * Fix verification step (broken in tracer move) * classify area:test-apps issues as build/test related * [Version Bump] 1.28.8 The following files were found to be modified (as expected) - [x] docs/CHANGELOG.md - [x] tracer/build/_build/Build.cs - [x] tracer/integrations.json - [x] tracer/samples/AutomaticTraceIdInjection/MicrosoftExtensionsExample/MicrosoftExtensionsExample.csproj - [x] tracer/samples/AutomaticTraceIdInjection/Log4NetExample/Log4NetExample.csproj - [x] tracer/samples/AutomaticTraceIdInjection/NLog40Example/NLog40Example.csproj - [x] tracer/samples/AutomaticTraceIdInjection/NLog45Example/NLog45Example.csproj - [x] tracer/samples/AutomaticTraceIdInjection/NLog46Example/NLog46Example.csproj - [x] tracer/samples/AutomaticTraceIdInjection/SerilogExample/SerilogExample.csproj - [x] tracer/samples/ConsoleApp/Alpine3.10.dockerfile - [x] tracer/samples/ConsoleApp/Alpine3.9.dockerfile - [x] tracer/samples/ConsoleApp/Debian.dockerfile - [x] tracer/samples/WindowsContainer/Dockerfile - [x] tracer/src/Datadog.Monitoring.Distribution/Datadog.Monitoring.Distribution.csproj - [x] tracer/src/Datadog.Trace.AspNet/Datadog.Trace.AspNet.csproj - [x] tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Datadog.Trace.ClrProfiler.Managed.Loader.csproj - [x] tracer/src/Datadog.Trace.ClrProfiler.Managed.Loader/Startup.cs - [x] tracer/src/Datadog.Trace.ClrProfiler.Native/CMakeLists.txt - [x] tracer/src/Datadog.Trace.ClrProfiler.Native/dd_profiler_constants.h - [x] tracer/src/Datadog.Trace.ClrProfiler.Native/Resource.rc - [x] tracer/src/Datadog.Trace.ClrProfiler.Native/version.h - [x] tracer/src/Datadog.Trace.MSBuild/Datadog.Trace.MSBuild.csproj - [x] tracer/src/Datadog.Trace.OpenTracing/Datadog.Trace.OpenTracing.csproj - [x] tracer/src/Datadog.Trace.Tools.Runner/Datadog.Trace.Tools.Runner.Standalone.csproj - [x] tracer/src/Datadog.Trace.Tools.Runner/Datadog.Trace.Tools.Runner.Tool.csproj - [x] tracer/src/Datadog.Trace/Datadog.Trace.csproj - [x] tracer/src/Datadog.Trace/TracerConstants.cs - [x] tracer/src/WindowsInstaller/WindowsInstaller.wixproj - [x] tracer/test/test-applications/regression/AutomapperTest/Dockerfile @DataDog/apm-dotnet * Add a sleep to minimize risks of segfaults on 2.1 (#1830) * Change more timeouts in MassTransit (#1829) * Post benchmark result comparison as a comment to the PR (#1811) * Extract PostCommentToPullRequest as separate function * Post benchmark results to the PR as a comment Based on the "results comparer" tool in the [dotnet/performance repo](https://github.com/dotnet/performance/blob/main/src/tools/ResultsComparer/README.md), using a default threshold and noise levels. We may well need to tweak these for our cases - we'll have to keep an eye on that. * Fix CSV reading Was using positional fields previously, but the CSV has a variable number of fields * Refactor benchmark comparison + add allocation comparison * Fix divide by zero error * Reduce sensitivity * Rename StatisticalTestThreshold to SignificantResultThreshold And update it to 10% - I misunderstood, we always use a 5% test for significance, but this rejects error results that show less than 10% difference * Update markdown generated * Fix path in gitlab.bat after repository move (#1832) * Parallelise Windows integration tests by framework (#1819) * Only build integration tests for a single framework Allows us to parallelize by framework * Parallelize integration tests by framework * Rename platform -> target platform Unfortunately, using the 'platform' key was causing the Nuke process itself to run as an x86 process, which was causing BadImageFormatException when loading the target frameworks for a project. Renaming the parameter to TargetPlatform instead solves the problem. * Dont test all integration package versions on every PR (#1818) * Fix TestAllPackageVersions usage (was always set to true) Ensure we compile multi-api samples using default package version if TestAllPackageVersions is false * Don't test all package versions on master branch * Fix tests to handle running the "default" package version tests * Add Test Framework version to test spans (#1828) * Generate Dependabot File for Integrations (#1754) * CallSite instrumentation - Add exceptions to active span during ASP.NET Web API 2 message handler exception (#1817) Follow-up to https://github.com/DataDog/dd-trace-dotnet/pull/1734 See the original PR for the full motivation. The original PR only added a fix for CallTarget instrumentation, so this PR adds the fix when the instrumented application is using CallSite instrumentation. * CI Visibility Mode v1 (#1795) * Initial CIVisibility POC * Add a Custom Sampler. * Refactor * Changes * Major refactor and support for adding origin tag to all CITracer spans. * Fix tests * Change AgentWriter algorithm. * Add dd-trace parameter to enable CI Visibility Mode * Apply suggestions from code review Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Change CIVisibilitySettings to readonly. * Use Tracer.UnsafeSetTracerInstance in all cases. * Refactor GetServiceNameFromRepository * Adds edge cases to GetServiceNameFromRepository and fixes the Regex to handle those cases. * Changes. * Add CIVisibility Mode autodetection to dd-trace based on the command wrapped. * Apply PR suggestion. * Fix LockedTracerInstanceSwap test to expect the InvalidOperationException. Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Small build improvements (#1813) * Stop copying profiler around for azure functions * Fix incorrect path in Directory.Build.props * Update launchSettings.json to use tracer home directory for profiler * Add shared helper project for samples, to reduce the need to copy-paste code everywhere * Delete copy pasted code and use the shared project instead * Apply suggestions from code review Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Apply suggestions from code review Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Use temporary folder for NServiceBus storage (#1840) * Enable static analysis for ConfigureAwait (#1833) * Enable static analysis for ConfigureAwait * Fix warning IL3000 error IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. * Update Gitlab build image to use 5.0.401 SDK (#1834) * Shutdown .NET server after build Might help avoid some locked files * Mention about downloading native symbols when creating a release * Ignore some types from the code coverage report that frequently come up but aren't meaningful * Update build image in GitLab (to use 5.0.401 SDK) * Fix transient error in aspnetcore tests (#1836) SubmitRequest can throw instead of just returning a status code if the connection couldn't be established. * Skip segmentation faults in .NET Core 2.1 tests (#1835) * Skip segmentation faults in .NET Core 2.1 tests * Managed loader cannot resolve profiler managed libraries when installed by the MSI installer (#1820) ### Context When installing the profiler and the tracer using the MSI installer, the profiler managed library cannot be resolved. To resolve the profiler managed library, the managed loader uses the `COR_PROFILER_PATH_XX` or `CORECLR_PROFILER_PATH_XX` to locate the profiler home directory. But, the MSI installer does not populate those environment variables, instead it registers the CLR profiler CLSID and the path in the registry. ### Fixes Changes proposed in this pull request: To resolve the tracer managed library, the managed loader relies on the `DD_DOTNET_TRACER_HOME` environment variable. (even if today the tracer does not use the managed loader). We can introduction a `DD_DOTNET_PROFILER_HOME` environment variable to replicate the same pattern used for the tracer. * Report attack when response has ended (#1847) Otherwise wrong status code is reported * Add a separate performance pipeline for running Performance tests (#1842) * Add a separate performance pipeline for running Benchmarks and Throughput tests The advantage of this is that we can make the benchmark and throughput results optional. We still want to run them for every PR, but it means in cases where a rebase or a trivial change is required, we wouldn't need to wait for the performance tests to finish before merging. * Update benchmark comparison tool to use correct pipeline ID * Need to have a stage without any dependencies * depends_on isn't required, as shuoldn't be triggered until source completes * Make stages run in parallel * Skip WriteTrace call when no there's no spans and improve filtering (#1843) * Skip WriteTrace call when no there's no spans and improve filtering * Skip the ArraySegment if doesn't have any span. * Update tracer/src/Datadog.Trace/Ci/Agent/CIAgentWriter.cs Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Simplify the CIAgentWriter algorithm. * Update CIAgentWriter.cs Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Fix performance pipeline (#1851) * Remove additional performance pipeline triggers With the additional triggers, the performance pipeline was being triggered twice - Immediately when a push to a monitored branch - When the ci pipeline had finished building We only want the latter one * Remove ContinueOnError from benchmarks These failures don't block the ability to merge, so they should be counted as real failures * Disable triggers * Disable PR verification triggers in performance pipeline * Try fix throughput tests not able to access the correct commit * Propagate the original source commit and repo from the originating pipeline This is such a horrible mess, but appears to be necessary * Try fix variables * Add lookup for `LocalRootSpanId` property and tests for `SpanId` and `LocalRootSpanId`. (#1839) A [recent PR](https://github.com/DataDog/dd-trace-dotnet/pull/1831) suggested to add direct access properties to Span to access the id and the id of the root span of the local non-reentrant sub-trace. That PR added private fields to `Span` to store those values, to avoid dereferencing multiple pointers at every lookup. However, it was not clear, whether that memory increase was justified, however small it was. To address that, _this_ PR provides and tests those properties without using additional memory. We are adding using tests, and will keep https://github.com/DataDog/dd-trace-dotnet/pull/1831 archived in case we need to get back to using private fields. * Check the logs for errors automatically after each test run (#1776) Excludes "known" issues such as "Unable to resolve method MongoDB" and the "CallTargetNativeTest"s * Add LifetimeManager for handling shutdown events (#1841) * Add LifetimeManager for handling shutdown We currently have duplicated handling of shutdown events in the Tracer and Security classes. Future work (e.g. Log submission and Telemetry) will like ly add further to this pattern. Rather than keep duplicating this pattern, adding a LifetimeManager to register all the callbacks should simplify things somewhat * Switch to a queue to preserve registration order * Replace CIVisibility lifetime hooks with LifetimeManager * Fix incorrect logger type reference * Additional updates to launchSettings.json for test applications (#1848) * Use Xunit.SkippableFact for inconclusive tests (#1858) * Update CI namespace types (#1864) * Update test package versions (#1862) Update the following NuGet packages under test: - MongoDB.Driver - Elasticsearch v7 - Confluent.Kafka - MySql.Data - Microsoft.Data.SqlClient - System.Data.SqlClient - ServiceStack.Redis - Microsoft.Data.Sqlite - MSTest.TestAdapter / MSTest.TestFramework - Microsoft.Azure.Cosmos - Aerospike.Client - NLog - AWSSDK.SQS - GraphQL / GraphQL.SystemReactive v4 * Kill the old Samples.Shared project (#1867) * Re-implement log4net logs injection (#1710) ## Changes This change modifies log4net logs injection to stop adding the Span information to the log4net MDC (which has demonstrated that it cannot safely be serialized across AppDomain boundaries) and to start adding the Span information directly to the log events. This requires automatic instrumentation so this information can be added as soon as possible after the LoggingEvent object is created, before other Appenders (i.e. sinks) consume the events and write them elsewhere. This change has a big limitation: **log4net logs injection now requires automatic instrumentation.** Additionally, the CustomLog4NetLogProvider type has been renamed to NoOpLog4NetLogProvider and modified so that the LibLog interface does not affect the log4net state. ## Testing The sample `test/test-applications/integrations/LogsInjection.Log4Net` has been updated so that the application crosses an AppDomain boundary after a trace has started, demonstrating that the previous implementation throws a SerializationException while the new implementation does not. ## Benefits The biggest benefit of this change is that we should no longer receive issues about log4net logs injection causing SerializationExceptions. This is because we no longer store the Datadog span information in the ambient context. Instead, we directly attach the Datadog span information to the log event at the time it's generated. A side benefit is that the properties will always exist on the log event objects, so we can now support some async scenarios where the customer previously could not grab the ambient log context. ## Performance Considerations I expected the new logs injection mechanism to be less performant because we must add Span information on each log statement, instead of once per scope activation. However, the benchmark results show that the overhead is largely unchanged. I guess this isn't too surprising as the new logs injection mechanism is just as performant as manually adding an Appender to the logging configuration (which is what New Relic does for log correlation). ## Follow-up If we need to make log4net logs injection operate with only manual instrumentation, we can publish an Appender package that customers can add via NuGet and configure themselves. * Add PInvoke Map rewriting in all platforms to support native library renaming (#1809) * Add PInvoke Map rewriting in all platforms to support native library renaming. * Fallback to the fixed name when we can't extract the path (Installed in the GAC). * Fix native filename per platform. * Adds the filename checker project. (Test is still missing for next commit) * Changes. * Fix GetCLRProfilerPath function. Adds the NativeProfilerChecks test. * Fix non Windows build * Revert unnecessary changes. * Add try/catch on the Checker path. Change GetCLRPRofilerPath to detect the runtime type. * try to fix tests. * Adds conditional for the check * setup the test project for linux. * commenting to do some checks. * Build passing. Uncomment, initialization block. * Add ToString to check and simplify catch scenario. * comments * change * change std::filepath with a normal rfind. * changing algorithm to avoid std::filesystem * Remove validation based on the process name, all samples are running through the dotnet.exe process * Delete performance pipeline, and manually update GitHub statuses (#1868) * Revert "Fix performance pipeline (#1851)" This reverts commit 438c66895a4a0ab143e6948980ec0e775237492e. * Revert "Add a separate performance pipeline for running Performance tests (#1842)" This reverts commit 91b80bb98053ec8b0c161873c520ab50fcb11c54. * Manually report statuses to GitHub This gives us pretty much the same "out of the box" experience as we would have if the app connection would work (but it doesn't, so this will have to do!) I'm not reporting statuses for _everything_ (e.g. code coverage, upload to s3) to reduce some clutter * Initialize LocalDB ahead of time in the CI (#1873) * Initialize LocalDB ahead of time in the CI * Increase timeout to 60 minutes * Add tags for App Sec (#1869) * Fix bug logging response even if successful (#1874) * Fix buffer overflow reported by Clang Address Sanitizer (#1872) * Fix buffer overflow reported by Clang Address Sanitizer using memcmp, we limited the count by taking the min between NameBuffer (1024) and the size of the functionName (functionNameLength). But, if the SpecificMethodToInjectName (and SpecificTypeToInjectName) is shorter than the max count, we will read memory beyond. ==21348==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fff40460ab2 at pc 0x7fff402c3682 bp 0x00c823dfc460 sp 0x00c823dfbbe8 READ of size 74 at 0x7fff40460ab2 thread T0 #0 0x7fff402c36b0 in MemcmpInterceptorCommon(void *, int (__cdecl *)(void const *, void const *, unsigned __int64), void const *, void const *, unsigned __int64) D:\a01\_work\2\s\src\vctools\crt\asan\llvm\compiler-rt\lib\sanitizer_common\sanitizer_common_interceptors.inc:851 #1 0x7fff402c5b71 in __asan_wrap_memcmp D:\a01\_work\2\s\src\vctools\crt\asan\llvm\compiler-rt\lib\sanitizer_common\sanitizer_common_interceptors.inc:882 #2 0x7fff4027e8d1 in shared::Loader::HandleJitCachedFunctionSearchStarted(unsigned __int64, int *) C:\Users\gregory.leocadie\repos\dd-trace-dotnet\shared\src\native-src\loader.cpp:670 #3 0x7fff401ea649 in CorProfilerCallback::JITCachedFunctionSearchStarted(unsigned __int64, int *) C:\Users\gregory.leocadie\repos\dd-continuous-profiler-dotnet\src\ProfilerEngine\Datadog.AutoInstrumentation.Profiler.Native.Shared\CorProfilerCallback.cpp:647 #4 0x7fffd09799ae (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x1805299ae) #5 0x7fffd06620b0 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x1802120b0) #6 0x7fffd045f05b (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x18000f05b) #7 0x7fffd0454854 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180004854) #8 0x7fffcb70ac4c (C:\Windows\assembly\NativeImages_v4.0.30319_64\mscorlib\16234675ede351917e6b94c968a734df\mscorlib.ni.dll+0x6447857ac4c) #9 0x7fffcb70a88d (C:\Windows\assembly\NativeImages_v4.0.30319_64\mscorlib\16234675ede351917e6b94c968a734df\mscorlib.ni.dll+0x6447857a88d) #10 0x7fffcb713b70 (C:\Windows\assembly\NativeImages_v4.0.30319_64\mscorlib\16234675ede351917e6b94c968a734df\mscorlib.ni.dll+0x64478583b70) #11 0x7fffd0456952 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180006952) #12 0x7fffd0456857 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180006857) #13 0x7fffd0457117 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180007117) #14 0x7fffd0581bf9 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180131bf9) #15 0x7fffd0590970 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180140970) #16 0x7fffd0592176 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180142176) #17 0x7fffd0591f63 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180141f63) #18 0x7fffd0591cbc (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180141cbc) #19 0x7fffd0592ea3 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\clr.dll+0x180142ea3) #20 0x7fffd13f8c00 (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\mscoreei.dll+0x180008c00) #21 0x7fffd17dac41 (C:\Windows\SYSTEM32\MSCOREE.DLL+0x18000ac41) #22 0x7fffe7947033 (C:\Windows\System32\KERNEL32.dll+0x180017033) #23 0x7fffe92a2650 (C:\Windows\SYSTEM32\ntdll.dll+0x180052650) * Handle the expected segfault in smoke tests (#1878) * Move HttpListener test logic to shared project * Create a shared `CurrentProcess` helper class in based on the Tracer's internal helper. (#1876) The Tracer has an (internal) `ProcessHelpers` utility class. We recently copied some code out of it for use in the Profiler. We already know that CodeHotspots will also need functionality of the Process API. This change sets up up a helper class in the shared code folder that covers the same functionality. To avoid cross-PR dependencies, it is not yet used, but we will start using it in the Profiler after this PR is merged. The Tracer team should consider whether or not they like using the shared code too, and decide based on their preferences. * Add more information to aerospike tags (#1865) * Add more information to aerospike tags * Fix errors in the CI when the startup log thread gets aborted (#1879) * Make uses of `Process` in shared-managed-files safe and add required capabilities to `CurrentProcess`. (#1884) Following up on [this recent work](https://github.com/DataDog/dd-trace-dotnet/pull/1876), I inspected Profiler and shared code for potentially unsafe usages of the `Process` class. In shared files, I only found usages in the Logger in 2 places. Having those sites call the new `CurrentProcess` utility would mean that we need to touch every single project that uses the logger to also include `CurrentProcess.cs` into those builds. We can consider doing it in the future. For now I have created private helpers in those places to ensure safe usage of `Process` without touching all the projects' project-files (this PR). In the Profiler, I switched to using `CurrentProcess` everywhere it was possible ([see here](https://github.com/DataDog/dd-continuous-profiler-dotnet/pull/196)). Only one location remained. That scenario was not supported by `CurrentProcess`. This PR adds that support (obtaining process modules). * Cleanup startup logs (#1890) * Add sanity check for _OR_GREATER compiler directives (#1895) * Add sanity check for _OR_GREATER compiler directives * Propagate the error code in gitlabs if the build failed * Refactor ModuleMetadata handling to reduce memory allocations. (#1891) * Refactor both CallTarget_RequestRejitForModule and RewritingPInvokeMaps methods to reduce allocations, specially for CallTarget scenarios. * Changes and fixes. * fixes * Complete refactor and fixes. * Revert Debug logs by default. * Change Log levels. * Improve comment. * minimal refactor to avoid any possible leak when creating ModuleMetadata struct * Changes based on the review * Changes based in the review. * Ensure closure variables pass by reference * Fix comments * appsec: fix appsec event tag value (#1898) * [AppSec] Send ip addresses headers to the backend (#1764) * Share a function to send all headers between core / framework * socket ip address in remote ip property for net framework * Add environment value for DD_ENV * ip extractor algorithm * send correct ip address> local resolution, send extra headers * Unit tests * remove some changes in sln * Fixing sln * Changes after Roberts comments Fix unit tests according to new modifications * Don't send null values to backend #1698 * Actor should contain the resolved ip , Request should contain the peer IP * Add unit tests, remove port and send more headers * x-client-ip instead of client-ip * Dont assign port if no ip, fixes to comments * Test the tracer works with F# web framework 'Giraffe' (#1888) * Revert "Refactor ModuleMetadata handling to reduce memory allocations. (#1891)" (#1904) This reverts commit b0e6511f9a70f787fe84ae3618c2a304d1606a20. * Skip Pipeline for Dependabot Lures (#1905) * Reduce the number of times we call the WAF (#1901) Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Mode additions to `CurrentProcess` wrapper (`GetMainModule()`). (#1896) A further API is needed on the `CurrentProcess` utility. This is required for [this PR](https://github.com/DataDog/dd-continuous-profiler-dotnet/pull/200). * [Version Bump] 1.29.0 (#1913) * [Version Bump] 1.29.0 * Small update to CHANGELOG * Remove changelog Changes that don't affect the production tracer * Revert changes to Datadog.Dependabot.Integrations.csproj Co-authored-by: zacharycmontoya <zacharycmontoya@users.noreply.github.com> Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Revert "Test the tracer works with F# web framework 'Giraffe' (#1888)" (#1914) This reverts commit 369e89d5b7edd1b481bea2b1f4ee24a1d6b32a10. The GitLab build process is having an issue with restoring F# projects. This revert is temporary so we can proceed with the Tracer release and can be undone later. * Build a beta MSI that contains multiple .NET products (#1870) ## Goal Build a beta MSI that can be used to do a one-click install of the .NET Tracer and the Continuous Profiler. When the Continuous Profiler product is GA, this MSI will replace the current GA MSI. ## Changes - Create a new `.wixproj` project in the shared directory to build the new MSI - Create subdirectories to store the `.wxs` files for each part of the product: `ContinuousProfiler`, `shared`, `tracer` - Files and environment variables to be installed are defined in their own `.wxs` files - Nuke changes - New build targets: - `BuildProfilerHome` - Builds the Continuous Profiler home directory - `BuildMonitoringHome` - Builds the combined Monitoring home directory - `PackageMonitoringHomeBeta` - Builds the new MSI - New build arguments: - `ProfilerHome` - Defines where the Continuous Profiler home directory will be - `MonitoringHome` - Defines where the combined Monitoring home directory will be - `BetaMsiSuffix` - Defines the MSI filename suffix, currently used to define the build_id and commit_sha (Temporary) - `ProfilerSrcDirectory` - Defines where the continuous profiler sources are (Temporary) - Azure DevOps `consolidated-pipeline` changes to build the native loader, the Continuous Profiler, and the new MSI ### MSI source directory structure See https://github.com/DataDog/dd-trace-dotnet/pull/1870 ## Development 1. Add the `dd-continuous-profiler-dotnet` repo as a sibling directory to `dd-trace-dotnet` 2. Run the following build command to produce the new MSI, which builds the tracer, profiler, and native loader binaries: ``` build BuildTracerHome BuildProfilerHome BuildMonitoringHome PackageMonitoringHomeBeta ``` ## Results ### Outputs Produces two MSI's, one which is only a 32-bit build and one which is a 64-bit build: - `tracer/bin/artifacts/x64/en-us/datadog-dotnet-apm-<VERSION>-x64-profiler-beta-<BUILD_ID>-<COMMIT_SHA>.msi` - `tracer/bin/artifacts/x86/en-us/datadog-dotnet-apm-<VERSION>-x86-profiler-beta-<BUILD_ID>-<COMMIT_SHA>.msi` ### MSI install directory structure See https://github.com/DataDog/dd-trace-dotnet/pull/1870 ## Remaining Work https://github.com/DataDog/dd-trace-dotnet/pull/1870 * Remove the "comprehensive" testing suite (#1907) * Remove the "comprehensive" testing suite We never run it, and in fact _can't_ run it, because it's so extensive. The major + minor version we have should be sufficient anyway * Update package versions * Allow requesting comprehensive testing in the pipeline Useful when you know you're introducing risky changes, and want to be safe before merging to master * Add TestAllPackageVersions to docker-compose * Set TestAllPackageVersions in run_in_docker.sh * Don't use ITestOutputHelper after test has finished (#1877) * Improve testing for Aerospike (#1889) - Add a test case for queries with statements - Remove the "Sync" command prefix that was never used * Clean the workspace before building on ARM64 (#1908) * Clean the workspace before building on ARM64 * Also clean during test jobs * Add test application for MySqlConnector nuget package (#1863) * Update Datadog.Trace to 1.19.1 in regression test (#1919) * Upgrade to Alpine 3.14 (#1899) * Upgrade to Alpine 3.14 * Define GENERATED_OBJ_FILES as EXTERNAL_OBJECT * Don't downgrade cmake on OSX (#1920) * Create version.txt (#1917) Adds a file containing the tracer version number to help make constructing URLs easier * Include the package version in all the MySqlConnector tests (#1921) We have to include the package version in _all_ the tests. When the MySqlConnector sample is built for the multi-api packages, it's _not_ built for the default package, which causes failures on master only * Correct _dd.appsec.enabled (#1918) It should be a metric * Modify the GitLab CI to build and sign the multi-product MSI (#1924) - Use an updated Windows build image for the GitLab CI build, so the new Profiler native bits can build (image was generated from PR https://github.com/DataDog/datadog-agent-buildimages/pull/174) - Clone the profiler repo into the working directory and set the appropriate symbolic links to trick the profiler repo to believe the tracer repo is in a sibling directory - Update the GitLab build command to build the profiler assets and the beta MSI - Set MSBuild property `Platform=AnyCPU` for the managed profiler build - Set MSBuild property `SpectreMitigation=false` for the native profiler build - This can be changed later, but this is how the profiler is currently being built and it ensures that the GitLab CI runs the exact same build as all other platforms. The current setting also prevents a build break in GitLab CI that can only be fixed with another Windows build image modification - Propagate the `MonitoringHomeDirectory` to the beta MSI build, so we don't rely on the relative-path fallback * Shared Managed Loader delays loading assemblies until AD has initialized. (#1916) `ExecuteDelayed` is about preventing side effects from running the loader very early in the AppDomain life cycle by delaying it towards a later point in the AppDomain Life cycle. Example for a crash caused by this kind of side effect: WCF applications using `BasicHttpsBinding` (note the "s" in https) were crashing with the continuous profiler attached. Error: _System.Configuration.ConfigurationErrorsException: Configuration binding extension 'system.serviceModel/bindings/basicHttpsBinding' could not be found.Verify that this binding extension is properly registered in system.serviceModel/extensions/bindingExtensions and that it is spelled correctly._ This was because the respective parts WCF configuration subsystem used `WebSocket.IsApplicationTargeting45()` to tweak their behavior on different framework versions. In turn, `WebSocket.IsApplicationTargeting45()` calls the static method `BinaryCompatibility.TargetsAtLeast_Desktop_V4_5()`. That method uses the static variable `s_map`. That, in turn, is initialized by the static cctor, i.e. first time `BinaryCompatibility` the class is used. This Assembly Loader calls `Array.Sort` while initializing its logger. That, it turn, also uses the `BinaryCompatibility` class internally, to choose a backward-compatible sorting algorithm. As a result those flags are initialized and cached when the loader is invoked. However, at that time, the AppDomain may not be completely initialized. To initialize, the `BinaryCompatibility` cctor invokes `AppDomain.GetTargetFrameworkName()`, which, in turn, calls `Assembly.GetEntryAssembly()`. That API returns `null` when invoked too early in the AppDomain lifecycle. As a result, a bogus target framework moniker is obtained (and cashed), and - in turn - the binary compatibility flags are initialized incorrectly (and also cached). As a result, everything that relies on the binary compatibility flags (or the target Framework moniker) may work in an unpredictable matter. This also leads to the WCF crash. To mitigate that, inside of <c>Execute()</c> we inspect whether `Assembly.GetEntryAssembly()` returns `null` before we start executing. If it does, we off-load the execution to a helper thread and returns immediately. The helper thread runs the <c>ExecuteDelayed(..)</c> method: It sleeps and periodically checks `Assembly.GetEntryAssembly()` until it no longer returns returns `null`. Then the Loader proceeds with its normal logic. As a result, the target framework moniker and the binary compatibility flags are initialized correctly. </summary> <remarks> The above logic is further specialized, depending on the kind of the current AppDomain and where the app is hosted: * On non-default AD: we do not wait. * On default AD, app NOT hosted in IIS:: `GetEntryAssembly` initially returns null, but once the AD is fully initialized, it returns the correct value. So, we apply the above strategy: wait on a separate thread until `GetEntryAssembly` is not null and then execute the loader. As mentioned, it is required because some APIs need `GetEntryAssembly` to populate bin compat flags in the Fx. * The user does not need to specify a parameter for this, since we wait _until `GetEntryAssembly` is not null_. * This behavior is on by default, but all delaying may be disabled using `DD_INTERNAL_LOADER_DELAY_ENABLED=false`. * On default AD, app IS hosted in IIS: `GetEntryAssembly` always returns null. It will always stay null, and there is no point delaying anything in that case. Even if we did delay, we would not have an end-condition for the wait as `GetEntryAssembly` always remains null forever. * So by default we do not wait on IIS. * As a precaution we support an _optional_ wait that use user can opt into by setting DD_INTERNAL_LOADER_DELAY_IIS_MILLISEC to a potitive number of milliseconds. (Since on IIS there is no exit condition to that delay, the option cannot be Boolean.) * Use GitHub API to hide old code coverage/benchmark reports when adding a new one (#1927) * Use GitHub API to hide old code coverage/benchmark reports when adding a new one The PR quickly fills up with clutter now. This should alleviate the issue, while still retaining the comments for prosperity in case people need them. Unfortunately, that functionality is only exposed via the GraphQL API, so had to use that * Update tracer/build/_build/Build.GitHub.cs Co-authored-by: Kevin Gosse <krix33@gmail.com> Co-authored-by: Kevin Gosse <krix33@gmail.com> * Add the standardizes logs for AppSec (#1894) Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Update dependabot PRs (#1930) * Don't trigger builds for PRs that only touch the tracer/dependabot files * Add a github action to automatically regenerate the package versions and start a test when dependabot detects changes * Run log injection tests on linux (#1929) * Enable running Logs Injection tests on Linux We really need to be running these so that we are testing multiple package versions * Pass package version in LogInjection integration tests We need to pass the package version so that we can get the path to the correct version of the application, when building the multi-package samples. Otherwise we are checking the wrong log files! * Try and workaround NLog on Linux issues No matter what I try, no matter which directory char we use etc, I just can't get NLog to behave when reading the xml config. As a workaround, just force it to put the logs in the correct place on linux by changing it in code * Use older "default" versions for logs injection on .NET Framework Windows only currently runs a single package version for integration tests. To increase the number of covered versions for logs-injection, use on of the .NET Framework-only packages when running on full framework. * Fix NLog for configuration for NLog 4.6+ In NLog4.6+, LibLog adds the attributes to the MDLC instead of the MDC, so we need to use a new config. * [Appsec] Send events in utf 8 without byte marker (#1932) * Utf 8 without byte marker * Changes to comments: test without BOM in appsec tests + read-only field * Refactor ModuleMetadata handling to reduce memory allocations (#1931) * Refactor ModuleMetadata creation from all loaded modules to only the required ones. * Adds std::future to the ModuleLoadFinished EnqueueProcessModule call. * Process module for ReJIT in the same thread as ModuleLoadFinished * Protect for shutdown deadlock. * Add support for retrieving memory dumps on integration tests hangs * Avoid calling RequestReJIT in the same thread of ModuleLoadFinished. * remove unused local variable * Change the memdump algorithm and disable it by default. * Ensure the promise is resolved on shutdown. * Give some time for the aspnetcore process to exit in AppSec tests (#1926) * Give some time for the aspnetcore process to exit * Let AspNetBase dispose the agent (The agent must be disabled after the target process shuts down) * Small build fixes (#1936) * Fix build_in_docker.sh This was broken when we moved things into the tracer folder and wasn't fixed correctly * Always build AspNetCoreRazorPages when building single sample If you try and build a single sample on linux using (for example) ``` ./tracer/build_in_docker.sh Clean BuildTracerHome BuildAndRunLinuxIntegrationTests -SampleName "LogsInjection.NLog" ``` then the build will fail. That's because the linux integration tests depend on `Samples.AspNetCoreRazorPages`, but when a single sample name is provided, it won't be built. This change ensures we _always_ build `Samples.AspNetCoreRazorPages`, even when a specific sample is specified using `-SampleName <name>` * Refactor reverse duck-typing and add negative tests (#1900) * Add tests that we _can't_ duck type invalidly defined types We have lots of tests that we _can_ duck type certain things, but there are many cases where we should throw when attempting to Duck Cast. If we don't, we risk accidentally thinking we've duck cast correctly when we haven't. Obviously an even better approach would be to have an analyzer that detects this too, but that's a _lot_ more work * Add checks for method resolution that Duck attribute works with generic parameter names * Add some error tests for reverse duck typing * Add failing test for reverse proxy where virtual methods already have a method body * Add test that reverse-proxy properties work correctly (they don't currently) * Fix uninitialized fields and properties in reverse proxy Call base constructor if the proxy type is inheriting from a class or abstract class. * Change type returned by DuckType.Create() to an object If the ProxyType is a struct, then even if the implementation is correct, this will throw for struct proxies when it attempts to cast to an IDuckType (as the struct doesn't implement IDuckType). The cast is largely unnecessary anyway, and can be cast explicitly by the caller if necessary (and safe) * Fix cases in field and property duck typing that were being too lenient They were being lenient to handle _reverse_ duck typing scenarios, but we'll split those out for clarity and increase the strictness here Also add some warnings where we don't (can't) know if a conversion is valid, so it would result in a runtime error accessing the member * Add additional guard clauses for method duck typing - Catch when an argument changes from generic to non-generic in a generic method - Catch when the original method returns void, but the new method returns non-void * Skip tests for which we can't (or won't) detect failures (Primarily around return types and conversions) * Split reverse-duck-typing from forward-duck-typing While these cases are quite similar, there are a bunch of requirements and edge cases that make interleaving the implementation problematic. Some of the additional tests added highlighted this problem. To try and make changes to the duck-typing code safer, this separates both the implementation and the API of reverse duck typing from "forward" duck typing. This also fixes some of the implementation issues that were previously causing failures * Make [DuckReverseMethod] a mirror of [Duck] attribute - Extracted the common features to a DuckAttributeBase - Allow [DuckReverseMethod] to be used on properties - Update method resolution to use the new attributes - Fix method resolution for forward duck-typing when using Duck attribute and have generic parameters - Fix method resolution for reverse duck-typing - [DuckIgnore] is no longer used by reverse duck typing, as you must decorate all your reverse methods/properties anyway * Fix [DuckReverseMethod] usages * Update SelectTargetMethod implementation - Fix case where the duck type attribute parameters don't match method parameters count - Fix duck chaining from reverse duck types - need to do _reverse_ duck chaining here, not normal forward duck chaining - Fix reverse duck typing not finding - Fix invalid test (caught with above changes) * Handle additional incorrect usage case and add additional exception tests * Ensure IntegrationOptions catches DuckType exceptions when using DuckCast(Type), as well as DuckCast<T> * Refactor DuckType.Methods.cs to reduce duplication * Update Public API tests This added some public DuckTyping methods. They could be made internal, but their equivalents are already public, so I think it makes more sense to keep them all public for now * Update ExceptionsTests Introduce ReverseProxyMustImplementGenericMethodAsGenericException to make it easier to debug the issue Remove unused exception Add missing tests for ReverseProxyMissingMethodImplementationException Add some extra tests for code coverage purposes * Update incorrect comments * Fix public API * Comments from code review * Update reverse proxy properties implementation Reverse proxies for methods was previously broken. As for methods, when we're doing reverse proxying, we need to do the opposite ducktype/duckcast compared to forward proxying This also removes the DuckKind.Field for reverse proxies (as that doesn't make sense) Also, updated the "duck cast/duck chain" delegates to return the actual type that you end up with, for use by the caller. * Handle case where [DuckReverseMethod] attribute has different number of parameters than method This always points to a configuration error for reverse duck typing, whereas for forward duck typing this isn't _necessarily_ an error (as the target may have optional parameters, for example) Add an explicit check for the reverse duck type case to catch this. * Fix incorrect arguments in test * Use public types so we can test net452 in reverse proxy error tests * Fix incorrectly ignored `WrongArgumentTypeInterfaceImplementations.DuckChainReturnMethod` test * Fix Public API tests * Ensure we catch _all_ exceptions when creating a duck type Co-authored-by: Tony Redondo <tony.redondo@datadoghq.com> * Fix typo in comments (#1928) Just merged https://github.com/DataDog/dd-trace-dotnet/commit/ea5e046488c612f8d4a4bee04a7c66532efa58b3, and subsequently discovered a typo in the comments. Sorry about that. This fixes the typo. * Add a "LatestMajors" option for testing all major package versions (#1939) It's an in-between step between TestAllMinorPackageVersions, and only testing the default samples, and is now the default for the multi-api package tests In many cases, the results are the same as using the default samples, but it should give more test coverage in PRs generally * Fix condition in Datadog.Trace.proj breaking the build on master (#1950) * Fix GitHub Actions workflows (#1946) * Try fix github workflow when dependabot creates PRs Also allow manual dispatch so we can update whenever we like * Auto tag the version bump commit on `release/*` branches too * Apply suggestions from code review Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Ensure we can run the workflow when doing manual bump Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Co-authored-by: Robert Pickering <robert.pickering@datadoghq.com> Co-authored-by: Tony Redondo <tony.redondo@datadoghq.com> Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> Co-authored-by: Kevin Gosse <krix33@gmail.com> Co-authored-by: Colin Higgins <colin-higgins@users.noreply.github.com> Co-authored-by: macrogreg <macrogreg@users.noreply.github.com> Co-authored-by: Anna <anna.yafi@datadoghq.com> Co-authored-by: Gregory LEOCADIE <gregory.leocadie@datadoghq.com> Co-authored-by: Rodrigo Fernandes <rtfrodrigo@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: zacharycmontoya <zacharycmontoya@users.noreply.github.com>

Don't cache the process instance for runtime metrics

38c5aaf

kevingosse requested a review from a team as a code owner August 24, 2021 11:55

andrewlock approved these changes Aug 24, 2021

View reviewed changes

zacharycmontoya approved these changes Aug 24, 2021

View reviewed changes

kevingosse merged commit 2cb1d18 into master Aug 25, 2021

kevingosse deleted the kevin/runtimeprocess branch August 25, 2021 09:58

github-actions Bot added this to the vNext milestone Aug 25, 2021

zacharycmontoya pushed a commit that referenced this pull request Aug 26, 2021

Don't cache the process instance for runtime metrics (#1698)

c0b7b86

zacharycmontoya mentioned this pull request Aug 26, 2021

Prepare hotfixes for 1.28.4, based on 1.28.2 #1705

Merged

zacharycmontoya pushed a commit that referenced this pull request Aug 26, 2021

Don't cache the process instance for runtime metrics (#1698)

9f54181

zacharycmontoya modified the milestones: 1.28.5-prerelease, 1.28.4 Aug 30, 2021

anna-git added a commit that referenced this pull request Oct 8, 2021

Don't send null values to backend #1698

fa82ff3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't cache the process instance for runtime metrics#1698

Don't cache the process instance for runtime metrics#1698
kevingosse merged 1 commit into
masterfrom
kevin/runtimeprocess

kevingosse commented Aug 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kevingosse commented Aug 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants