Conversation
Update SDK to use latest (still pinned) Only need the runtimes for earlier versions
| steps: | ||
| - task: UseDotNet@2 | ||
| displayName: install dotnet core sdk 3.1 | ||
| displayName: install dotnet core runtime 3.1 |
There was a problem hiding this comment.
I always thought we only needed the latest SDK and only the runtimes for the rest, and it used to be like that, but at some point there was an issue (I can't remember) and we switched to installing the full SDK instead. I hope this works as it will make the containers a bit smaller and faster to build, right?
There was a problem hiding this comment.
Oh, it was because the base runtime does not include aspnetcore runtime. See #868.
Maybe this task supports packageType: aspnetcore?
There was a problem hiding this comment.
There was a problem hiding this comment.
Soo... yeah... you're right. However, it's only the Windows x64 job that fails currently... could be indicative that we're not running all the tests we think we are on other platforms?
As an aside, the Windows agents already have a bazillion versions installed, so we could probably get away without any of these anyway 🤷♂️
There was a problem hiding this comment.
I don't really understand why the OTel fork was able to make these changes in open-telemetry/opentelemetry-dotnet-instrumentation#53 but we can't 🤔
There was a problem hiding this comment.
I think it works on Linux because we install the aspnetcore runtime separately in the dockerfile: https://github.com/DataDog/dd-trace-dotnet/blob/master/build/docker/dotnet.dockerfile
There was a problem hiding this comment.
argh, of course we do, I forget that this is only part of the build definition 😛
As the runtime install task doesn't include the aspnetcore runtime
| variables: | ||
| buildConfiguration: release | ||
| dotnetCoreSdkVersion: 5.0.100 | ||
| dotnetCoreSdkVersion: 5.0.103 |
There was a problem hiding this comment.
Maybe we should use 5.0.x to get all the minor updates
There was a problem hiding this comment.
I did that initially, and that's what the OTel version does, but in Gitlab CI we're explicitly using 5.0.103. Though it's probably better to keep those in sync?
There was a problem hiding this comment.
We have had issues before with using latest. A new SDK is released that requires an update to VS, but the AzDO images are not updated, so builds fail.
edit: see #854
Add success/failure and timing information to tests (DataDog/dd-trace-dotnet#1210) Switch integration tests to release (DataDog/dd-trace-dotnet#1212) * Switch integration tests to release * Fix build configuration * Fix runner pipeline bump Service Fabric version to 1.23.0 to match Datadog.Trace (DataDog/dd-trace-dotnet#1189) CallTarget Integration: Add base DBCommand async overloads (DataDog/dd-trace-dotnet#1206) * Add base DBCommand async overloads * Add Samples.FakeCommand to test the DBCommand instrumentation. * Add missing test. * Fix FakeCommandTests.SpansDisabledByAdoNetExcludedTypes test CallTarget Integration: ADO.NET MySqlData client version 6 (DataDog/dd-trace-dotnet#1184) * Add MySqlData support and execute the tests in multiple client versions. * Fix Samples.MySql.csproj project. * Add old MySql server for older clients test. * Fix StartsWith on .NET frameworks. * Fix Sample.MySql project * Fix the test for callsite instrumentation in 6.8.8 client version. * change sample app connection string * fix connection string param name. * Improves the csproj file conditions. * Remove code for debugging * update integrations.json CallTarget Integration: ADO.NET Microsoft.Data.Sqlite and System.Data.SQLite (DataDog/dd-trace-dotnet#1191) * Add Sqlite ADO.NET client structure. * Add SqliteConstants and Definitions basic values * Add missing methods instrumentation. * Sqlite Samples Apps * Add integration tests. * Add missing build project. * Add missing build project for arm64 and tests * Remove the SQLite test from arm64 due to incompatibility. Sqlite requires the native sqlite lib. * update integrations CallTarget Integration: ADO.NET Oracle.ManagedDataAccess (DataDog/dd-trace-dotnet#1193) * Add OracleConstants and OracleDefinitions * Adding samples project structure. * Adding sample projects. * Remove TaskCancellationSource and Add Oracle.DataAccess assembly support based on the source code. * update integrations.json * update integrations.json Add calltarget support for WebRequest (DataDog/dd-trace-dotnet#1204) * Add calltarget support for WebRequest Update stylecop (DataDog/dd-trace-dotnet#1216) * Update stylecop * Disable SA0001 Use release artifacts in package.sh (DataDog/dd-trace-dotnet#1222) NUnit: Add support for TestCase custom name (DataDog/dd-trace-dotnet#1213) * Adding support for testcase custom name and the IgnoreException. * Include TestName to the properties metadata following the new CiAPP specs. * Changes according to CiApp specs Enable optimizations when building the profiler (DataDog/dd-trace-dotnet#1223) Change `test.traits` format following the CiApp specs. (DataDog/dd-trace-dotnet#1221) Bump the .NET Tracer version to 1.24.0 (DataDog/dd-trace-dotnet#1226) switch build image to shared image (DataDog/dd-trace-dotnet#1100) update builder tag to reference new builder with dotnet5 (DataDog/dd-trace-dotnet#1229) Eager serialization of traces (DataDog/dd-trace-dotnet#1151) * Implement eager serialization of traces * Removing DD_TRACE_QUEUE_SIZE configuration key Update test dependencies + fix warnings (DataDog/dd-trace-dotnet#1227) * Update test dependencies + fix warnings * Removing xunit tool Remove obsolete Moq.ResetCalls (DataDog/dd-trace-dotnet#1231) Replace map count->[] with map find (DataDog/dd-trace-dotnet#1224) Fix environment variable name in docs (DataDog/dd-trace-dotnet#1233) Remove heap allocations from COR_SIGNATURE definitions in the native profiler (DataDog/dd-trace-dotnet#1217) * initial work to remove the heap allocation of signatures. * Remove all calltarget signatures heap allocations. Replacing DuckType `As<T>` with DuckCast, TryDuckCast, DuckAs and DuckIs (DataDog/dd-trace-dotnet#1220) * Replacing DuckType `As<T>` with `DuckCast<T>` and `DuckIs<T>` * Fixes test compilation. * change `DuckIs<T>` to `TryDuckCast<T>` * adding DuckAs and DuckIs extension methods. * DuckType method extension tests. Remove _W string operator from the native profiler (DataDog/dd-trace-dotnet#1215) * Remove the string _W operator. * Fix linux and test project compilation errors. * fix linux compilation. * change macro name from _LU to WStr as requested. * Fix native test compilation error. Fixes xUnit serializarion for CIEnvironmentVariableTests (DataDog/dd-trace-dotnet#1236) Call the right Log overload (DataDog/dd-trace-dotnet#1240) In some places, we call the Log overload expecting a line number, instead of the one with generic parameters. Also disabled the Resharper warning in the places where we call the right one, to make the warning more efficient. Fix NUnit passing status where there is not Assert. (DataDog/dd-trace-dotnet#1235) Update the automatic logs injection sample applications (DataDog/dd-trace-dotnet#1195) - Add another sample that consumes NLog 4.0, whose JsonLayout does not have the `includeMdc`/`includeMdlc` properties - Update the `Datadog.Trace` package in the samples to `1.23.0` and allow it to be updated by our versioning tool - Add `DD_ENV`, `DD_SERVICE`, and `DD_VERSION` to the logger configuration files - Some general documentation cleanup Update default log rate limit in debug mode (DataDog/dd-trace-dotnet#1239) * If there's a problem constructing the logger, disable rate limiting by default * Don't rate limit messages by default when in debug mode CallTarget Integration: ServiceStack.Redis and StackExchange.Redis (DataDog/dd-trace-dotnet#1230) * StackExchange redis implementation. * Fix tests * Refactor RedisExecuteAsync and RedisExecuteSync classes and add missing types. * Add CallTarget ServiceStack redis client instrumentation. Fix System.Byte and System.SByte signature equivalence. * Fixes tests add Service Fabric sample app (DataDog/dd-trace-dotnet#1190) Add GraphQL CallTarget instrumentation (DataDog/dd-trace-dotnet#1241) Strengthen type check in the method resolution for CallSite instrumentation and improve fallback mechanism (DataDog/dd-trace-dotnet#1225) - When resolving the original MethodInfo via metadata token, add a check to ensure the object instance can correctly be assigned to the MethodInfo type - Enforce a stricter type check on `Task<>` return types to remove ambiguity in fallback method resolution. Modify all CallSite integrations that return `Task<>` to be explicit about the generic type - Add failure mode to all ADO.NET test-application programs, which will run in integration-tests CI pipeline === It is possible to load the same assembly multiple times in an application by loading it into different assembly load contexts. One example is in .NET Core where an assembly may be loaded into the Default load context and then again into its own Assembly Load Context. When this happens, the Types from one assembly are not considered equal to the corresponding Type in the other assembly, even though the information that we can publicly observe (e.g. MVID and metadata tokens) appear to be the same. This causes a crash condition in our automatic instrumentation when the following happens: 1. Assembly1 (e.g. `System.Data.SqlClient.dll`) is loaded through the default load context 2. Our `ModuleLookup.PopulateModules` method caches a key-value pair of Assembly1.MVID and Assembly1.Module 3. The same Assembly1, let's call it Assembly1~New, is loaded into a new ALC 4. Automatic instrumentation is triggered for Assembly1\~New. We retrieve the MethodInfo by getting the cached Assembly1.Module (NOT equivalent to Assembly1\~New.Module), get a metadata token from it, and construct a dynamic method that calls into the original method 5. Invoking the original method results in a type cast exception on the instance object One potential solution that was considered but rejected was improving the module caching to consider multiple versions of the same assembly. The only way to differentiate between the assemblies would be to compare Assembly objects or the IntPtr to the Assembly, which seemed too complex to me. The solution pursued is to detect this scenario by adding an additional type check after we get the target MethodInfo and check that the instance object can be assigned to the Type containing the MethodInfo. Failing this check forces the method resolution to use the reflection fallback to find the right method. This fix resolves the issue, but required a cascading set of changes to perform stronger name checks on return types when method overloads exist. Once this was done, further testing demonstrated the fallback mechanism worked for all of our integrations (minus RabbitMQ). After testing all affected CallSite integrations except `AspNetWebApi2` and `XUnit`, the only integration whose fallback method resolution fails is `RabbitMQ`. That may require the integration point to be modified, so the work should be deferred to a separate work item. Add CallTarget support to MongoDB (DataDog/dd-trace-dotnet#1214) * Add CallTarget support to MongoDB Adding Git information to test spans (DataDog/dd-trace-dotnet#1242) * Initial basic GitInfo implementation. * Adds Author, Committer and Message git parser. * Changes based on the review. Update .NET SDK versions in build (DataDog/dd-trace-dotnet#1237) Still pinned .NET 5 SDK to latest (to avoid issues when new versions are released) Use runtime where possible Windows integration test pipeline installers continue to use sdk, as it includes the asnetcore runtime Update README with latest SDK requirements Add calltarget instrumentation for ASP.NET MVC and WebAPI (DataDog/dd-trace-dotnet#1208) * Add calltarget instrumentation for ASP.NET MVC and WebAPI * Convert ASP.NET WebAPI callsite integration to ducktyping Update the version of log4net we use in our tests from 2.0.8 to 2.0.12 (DataDog/dd-trace-dotnet#1243) Adds GitParserTests to the suite. (DataDog/dd-trace-dotnet#1247) * Adds GitParserTests to the suite. * Fix tests * Fix DateTime format assert Remove Sync-over-async in DatadogHttpClient (DataDog/dd-trace-dotnet#1218) * Convert DatadogHttpClient to use ReadAsync instead of ReadByte * Avoid hanging when chunked encoding (unsupported) used * Fix new line in HTTP headers - HTTP requires that CRLF is always used, regardless of environment, so the previous implementation was invalid on Linux * Add a SkipUntil method to optimise seek operations in network stream * Add additional optimisations for async paths in DatadogHttpClient * Add unit tests for DatadogHttpClient response parsing and network partial data handling * Handle large skipping with small buffer in SkipUntil Rename test configuration tags to follow CIApp specification (DataDog/dd-trace-dotnet#1251) Fix CiApp Tests ServiceName (DataDog/dd-trace-dotnet#1244) * Fix Tests ServiceName * Changes DD_SERVICE_NAME to DD_SERVICE * Changes based on the review. * Fix ConfigurationSourceTests, due DD_SERVICE being set when testing DD_SERVICE_NAME. * Add missing DD_ENV to benchmark tests. CallTarget: CurlHandler integration (DataDog/dd-trace-dotnet#1252) * Initial CurlHandler integration. * Adds WinHttpHandler and CurlHandler test using an HttpMessageInvoker. * Add integration and test for internal WinHttpHandler and CurlHandler test * Apply suggestions from code review Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Apply suggestions from code review Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Remove comment. Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
* Catchup to upstream Add success/failure and timing information to tests (DataDog/dd-trace-dotnet#1210) Switch integration tests to release (DataDog/dd-trace-dotnet#1212) * Switch integration tests to release * Fix build configuration * Fix runner pipeline bump Service Fabric version to 1.23.0 to match Datadog.Trace (DataDog/dd-trace-dotnet#1189) CallTarget Integration: Add base DBCommand async overloads (DataDog/dd-trace-dotnet#1206) * Add base DBCommand async overloads * Add Samples.FakeCommand to test the DBCommand instrumentation. * Add missing test. * Fix FakeCommandTests.SpansDisabledByAdoNetExcludedTypes test CallTarget Integration: ADO.NET MySqlData client version 6 (DataDog/dd-trace-dotnet#1184) * Add MySqlData support and execute the tests in multiple client versions. * Fix Samples.MySql.csproj project. * Add old MySql server for older clients test. * Fix StartsWith on .NET frameworks. * Fix Sample.MySql project * Fix the test for callsite instrumentation in 6.8.8 client version. * change sample app connection string * fix connection string param name. * Improves the csproj file conditions. * Remove code for debugging * update integrations.json CallTarget Integration: ADO.NET Microsoft.Data.Sqlite and System.Data.SQLite (DataDog/dd-trace-dotnet#1191) * Add Sqlite ADO.NET client structure. * Add SqliteConstants and Definitions basic values * Add missing methods instrumentation. * Sqlite Samples Apps * Add integration tests. * Add missing build project. * Add missing build project for arm64 and tests * Remove the SQLite test from arm64 due to incompatibility. Sqlite requires the native sqlite lib. * update integrations CallTarget Integration: ADO.NET Oracle.ManagedDataAccess (DataDog/dd-trace-dotnet#1193) * Add OracleConstants and OracleDefinitions * Adding samples project structure. * Adding sample projects. * Remove TaskCancellationSource and Add Oracle.DataAccess assembly support based on the source code. * update integrations.json * update integrations.json Add calltarget support for WebRequest (DataDog/dd-trace-dotnet#1204) * Add calltarget support for WebRequest Update stylecop (DataDog/dd-trace-dotnet#1216) * Update stylecop * Disable SA0001 Use release artifacts in package.sh (DataDog/dd-trace-dotnet#1222) NUnit: Add support for TestCase custom name (DataDog/dd-trace-dotnet#1213) * Adding support for testcase custom name and the IgnoreException. * Include TestName to the properties metadata following the new CiAPP specs. * Changes according to CiApp specs Enable optimizations when building the profiler (DataDog/dd-trace-dotnet#1223) Change `test.traits` format following the CiApp specs. (DataDog/dd-trace-dotnet#1221) Bump the .NET Tracer version to 1.24.0 (DataDog/dd-trace-dotnet#1226) switch build image to shared image (DataDog/dd-trace-dotnet#1100) update builder tag to reference new builder with dotnet5 (DataDog/dd-trace-dotnet#1229) Eager serialization of traces (DataDog/dd-trace-dotnet#1151) * Implement eager serialization of traces * Removing DD_TRACE_QUEUE_SIZE configuration key Update test dependencies + fix warnings (DataDog/dd-trace-dotnet#1227) * Update test dependencies + fix warnings * Removing xunit tool Remove obsolete Moq.ResetCalls (DataDog/dd-trace-dotnet#1231) Replace map count->[] with map find (DataDog/dd-trace-dotnet#1224) Fix environment variable name in docs (DataDog/dd-trace-dotnet#1233) Remove heap allocations from COR_SIGNATURE definitions in the native profiler (DataDog/dd-trace-dotnet#1217) * initial work to remove the heap allocation of signatures. * Remove all calltarget signatures heap allocations. Replacing DuckType `As<T>` with DuckCast, TryDuckCast, DuckAs and DuckIs (DataDog/dd-trace-dotnet#1220) * Replacing DuckType `As<T>` with `DuckCast<T>` and `DuckIs<T>` * Fixes test compilation. * change `DuckIs<T>` to `TryDuckCast<T>` * adding DuckAs and DuckIs extension methods. * DuckType method extension tests. Remove _W string operator from the native profiler (DataDog/dd-trace-dotnet#1215) * Remove the string _W operator. * Fix linux and test project compilation errors. * fix linux compilation. * change macro name from _LU to WStr as requested. * Fix native test compilation error. Fixes xUnit serializarion for CIEnvironmentVariableTests (DataDog/dd-trace-dotnet#1236) Call the right Log overload (DataDog/dd-trace-dotnet#1240) In some places, we call the Log overload expecting a line number, instead of the one with generic parameters. Also disabled the Resharper warning in the places where we call the right one, to make the warning more efficient. Fix NUnit passing status where there is not Assert. (DataDog/dd-trace-dotnet#1235) Update the automatic logs injection sample applications (DataDog/dd-trace-dotnet#1195) - Add another sample that consumes NLog 4.0, whose JsonLayout does not have the `includeMdc`/`includeMdlc` properties - Update the `Datadog.Trace` package in the samples to `1.23.0` and allow it to be updated by our versioning tool - Add `DD_ENV`, `DD_SERVICE`, and `DD_VERSION` to the logger configuration files - Some general documentation cleanup Update default log rate limit in debug mode (DataDog/dd-trace-dotnet#1239) * If there's a problem constructing the logger, disable rate limiting by default * Don't rate limit messages by default when in debug mode CallTarget Integration: ServiceStack.Redis and StackExchange.Redis (DataDog/dd-trace-dotnet#1230) * StackExchange redis implementation. * Fix tests * Refactor RedisExecuteAsync and RedisExecuteSync classes and add missing types. * Add CallTarget ServiceStack redis client instrumentation. Fix System.Byte and System.SByte signature equivalence. * Fixes tests add Service Fabric sample app (DataDog/dd-trace-dotnet#1190) Add GraphQL CallTarget instrumentation (DataDog/dd-trace-dotnet#1241) Strengthen type check in the method resolution for CallSite instrumentation and improve fallback mechanism (DataDog/dd-trace-dotnet#1225) - When resolving the original MethodInfo via metadata token, add a check to ensure the object instance can correctly be assigned to the MethodInfo type - Enforce a stricter type check on `Task<>` return types to remove ambiguity in fallback method resolution. Modify all CallSite integrations that return `Task<>` to be explicit about the generic type - Add failure mode to all ADO.NET test-application programs, which will run in integration-tests CI pipeline === It is possible to load the same assembly multiple times in an application by loading it into different assembly load contexts. One example is in .NET Core where an assembly may be loaded into the Default load context and then again into its own Assembly Load Context. When this happens, the Types from one assembly are not considered equal to the corresponding Type in the other assembly, even though the information that we can publicly observe (e.g. MVID and metadata tokens) appear to be the same. This causes a crash condition in our automatic instrumentation when the following happens: 1. Assembly1 (e.g. `System.Data.SqlClient.dll`) is loaded through the default load context 2. Our `ModuleLookup.PopulateModules` method caches a key-value pair of Assembly1.MVID and Assembly1.Module 3. The same Assembly1, let's call it Assembly1~New, is loaded into a new ALC 4. Automatic instrumentation is triggered for Assembly1\~New. We retrieve the MethodInfo by getting the cached Assembly1.Module (NOT equivalent to Assembly1\~New.Module), get a metadata token from it, and construct a dynamic method that calls into the original method 5. Invoking the original method results in a type cast exception on the instance object One potential solution that was considered but rejected was improving the module caching to consider multiple versions of the same assembly. The only way to differentiate between the assemblies would be to compare Assembly objects or the IntPtr to the Assembly, which seemed too complex to me. The solution pursued is to detect this scenario by adding an additional type check after we get the target MethodInfo and check that the instance object can be assigned to the Type containing the MethodInfo. Failing this check forces the method resolution to use the reflection fallback to find the right method. This fix resolves the issue, but required a cascading set of changes to perform stronger name checks on return types when method overloads exist. Once this was done, further testing demonstrated the fallback mechanism worked for all of our integrations (minus RabbitMQ). After testing all affected CallSite integrations except `AspNetWebApi2` and `XUnit`, the only integration whose fallback method resolution fails is `RabbitMQ`. That may require the integration point to be modified, so the work should be deferred to a separate work item. Add CallTarget support to MongoDB (DataDog/dd-trace-dotnet#1214) * Add CallTarget support to MongoDB Adding Git information to test spans (DataDog/dd-trace-dotnet#1242) * Initial basic GitInfo implementation. * Adds Author, Committer and Message git parser. * Changes based on the review. Update .NET SDK versions in build (DataDog/dd-trace-dotnet#1237) Still pinned .NET 5 SDK to latest (to avoid issues when new versions are released) Use runtime where possible Windows integration test pipeline installers continue to use sdk, as it includes the asnetcore runtime Update README with latest SDK requirements Add calltarget instrumentation for ASP.NET MVC and WebAPI (DataDog/dd-trace-dotnet#1208) * Add calltarget instrumentation for ASP.NET MVC and WebAPI * Convert ASP.NET WebAPI callsite integration to ducktyping Update the version of log4net we use in our tests from 2.0.8 to 2.0.12 (DataDog/dd-trace-dotnet#1243) Adds GitParserTests to the suite. (DataDog/dd-trace-dotnet#1247) * Adds GitParserTests to the suite. * Fix tests * Fix DateTime format assert Remove Sync-over-async in DatadogHttpClient (DataDog/dd-trace-dotnet#1218) * Convert DatadogHttpClient to use ReadAsync instead of ReadByte * Avoid hanging when chunked encoding (unsupported) used * Fix new line in HTTP headers - HTTP requires that CRLF is always used, regardless of environment, so the previous implementation was invalid on Linux * Add a SkipUntil method to optimise seek operations in network stream * Add additional optimisations for async paths in DatadogHttpClient * Add unit tests for DatadogHttpClient response parsing and network partial data handling * Handle large skipping with small buffer in SkipUntil Rename test configuration tags to follow CIApp specification (DataDog/dd-trace-dotnet#1251) Fix CiApp Tests ServiceName (DataDog/dd-trace-dotnet#1244) * Fix Tests ServiceName * Changes DD_SERVICE_NAME to DD_SERVICE * Changes based on the review. * Fix ConfigurationSourceTests, due DD_SERVICE being set when testing DD_SERVICE_NAME. * Add missing DD_ENV to benchmark tests. CallTarget: CurlHandler integration (DataDog/dd-trace-dotnet#1252) * Initial CurlHandler integration. * Adds WinHttpHandler and CurlHandler test using an HttpMessageInvoker. * Add integration and test for internal WinHttpHandler and CurlHandler test * Apply suggestions from code review Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Apply suggestions from code review Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Remove comment. Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> * Fix added env vars * Separate interfaces for OTel exporters * Skip flaky test ShouldCaptureFirstChanceExceptions * Run PrepareRelease integrations and versions * Fix garbled native constants * Remove and ignore ./blog/ folder * Fixes to Az Pipeline files: runner and unit-tests * Keep dotnet dev req in desc order for consistency Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
Based on changes in open-telemetry/opentelemetry-dotnet-instrumentation#53
@DataDog/apm-dotnet