update code owners #1750
Merged
update code owners #1750
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewlock
approved these changes
Sep 8, 2021
zacharycmontoya
approved these changes
Sep 10, 2021
Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com>
lucaspimentel
force-pushed
the
lpimentel/codeowners
branch
from
d507cfb
to
434bde7
Compare
andrewlock
added
the
area:builds
lucaspimentel
added a commit
that referenced
this pull request
Oct 28, 2021
* 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update
CODEOWNERSfile to assign github teams as code owners to specific repository sub-folders.NOTE: Updated with changes from #1748
TODO: add GitHub team
profiling-dotnetto the default (*) owner when the profiler code moves into this repository.