Pull upstream changes #415
Merged
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
…430) * Changes DuckType defaults to bypass ToString method to the target instance. * Changes * Change the implementation to be part of the IDuckType interface adding an special case to avoid ToString throwing on null target instances.
… (#2434) * Ensure the Origin tag value on the public SpanContext .ctor when running in CI Visibility mode. * Changes from review.
Plug the profiler CI and add its jobs in the noop-pipeline when required * Github Actions jobs are recognized by their name (or key if the name is not present). When there is a matrix (example: x64,x86 / net45,net5.0), there will be as many jobs as number of combination of the matrix. The final name will be "name (XX, YY)" (ex: "build (x64, net45)"). * Make run.sh executable
Update .vsconfig Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> * exclude `.vsconfig` from builds * exclude `.vsconfig` from profiler pipeline
…acts (#2436) add auth header to httpclient
* Add try/catch to fix MogoDB regression when BsonDocument throws and exception. * Add snapshot tests for MongoDb (#2440) * Add a VerifyHelper for verifying full snapshots Primarily ensures a consistent ordering of traces, and of spans within each trace (can be an issue when you have any concurrent behaviour) * Make scrubbing easier for VerifyHelper * Update MongoDb tests to use snapshot testing instead * Update snapshots to remove `ismaster` and `hello` spans (Based on changes in #2216) * Update snapshots to match current results Based on changes in #2430 and #2435 * Remove buildinfo and getlasterrror spans from snapshots These spans are generated as part of server monitoring, but we will have a non-deterministic number of them, which will lead to flaky snapshots. As a workaround, remove them from the snapshots, and do some basic assertions against them instead. Also, adds some extra scrubbing for differences between localhost snapshot generation and cli (e.g. host, aggregate queries etc) * Add arm64 scrubber for hostname * Add additional snapshot and scrubbers to support 2.5.x + 2.6.x Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
… thread and use the CLR thread instead. (#2442)
We previously had a regression test called `Sandbox.ManualTracing` to make sure that a simple usage of custom instrumentation did not cause issues due to code permissions. This PR adds the correct test category to make sure it runs in CI. This PR also adds a `Sandbox.AutomaticInstrumentation` to run automatic instrumentation in a similar partial trust scenario. However, this application is expected to fail so we expect a failure in its test runs.
The dependabot tool in the dd-trace-dotnet repository reported an issue with version <3.15 of protobuf https://github.com/DataDog/dd-trace-dotnet/security/dependabot/2
* Disable telemetry by default in all cases
* Adding more info on instrumenting specific methods Penultimate parameter must be exception on end method Instrumenting child classes Instrumenting properties * Update AutomaticInstrumentation.md changing phrasing * response to comments * Response to comment on ref and in keywords Ref or in are the same, in is safer for not modifying arguments Remove spaces
* trigger gitlab pipeline from ours actually * fix * remove useless variables * Change variable name to DEPLOY_TO_REL_ENV to make it more explicit
…#2438)" (#2449) This reverts commit bee319f.
…TransferRequest calls (#2419) When HttpServerUtility.TransferRequest is called, this results in two separate IIS pipeline requests where the HttpContext.Response object for the initial (unused) request has a 200 status code and the HttpContext.Response object for the second (transferred) request has the correct status. The issue was that the root span would be marked with a http.status_code=200, which does not corrrespond to the actual status code. This overrides the http.status_code tag of the root span with the correct status code.
* first version of the migration doc * Trying to avoid triggering some OCD by switching to .NET everywhere * Apply suggestions from code review Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> * 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> * Apply suggestions from code review Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> * Update v1_to_v2.md Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com> Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
* Refactored the CallTargetTokens to maximize code sharing with the Debugger and squashed various refactorings including renamings to avoid merge conflicts with the Debugger slydog
* [Test Package Versions Bump] * CouchbaseNetClient 3.2.6 dropped support for netcoreapp2.1 That causes failures in .NET Core 2.1 and .NET Core 3.0 Co-authored-by: andrewlock <andrewlock@users.noreply.github.com> Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com>
…2451) * Update GraphQLTests to reduce flakiness Previously, we were trying to find an open port, and then use that for all requests. With this change, we rely on ASP.NET Core's own "find an open port" mechanism, and then read that port from the output. This _should_ be more reliable, and hopefully will reduce the number of cases of "unable to bind port" that we see * Update Security ASP.NET Core tests to reduce flakiness Previously, we were trying to find an open port, and then use that for all requests. With this change, we rely on ASP.NET Core's own "find an open port" mechanism, and then read that port from the output. This _should_ be more reliable, and hopefully will reduce the number of cases of "unable to bind port" that we see * Update tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/GraphQLTests.cs Co-authored-by: Kevin Gosse <krix33@gmail.com> Co-authored-by: Kevin Gosse <krix33@gmail.com>
* add TraceTags and tests * add Tracer.Tags * add trace-level tags to root spans during serialization * fix xml-doc comment * make one ctor private * fix off-by-one error * add comments * fix xml-doc comment, redux * keep only tags with "_dd.p.*" prefix * don't allocation the string if not needed * comments * no need for volatile read * improved validation, caching, and tests * implement IEnumerable, add GetEnumerator() and Count * don't use AsList(), rename overloads * remove AsList() * remove IEnumerable to prevent accidental allocations, keep GetEnumerator * add missing lock * expand comments * check for prefix earlier * more comments * fix build errors in tests
…] (#2453) We have known issues where .NET Core 2.1 specifically can throw a seg fault. We handle this, and rethrow as a `SkipException`, but these are only handled if we use `[SkippableFact]` and `[SkippableTheory]`. For simplicity, and to avoid missing any, updated all the integration test projects to _always_ use the skippable attributes.
* Fix incorrect timeout in BatchingSinkTests This should be 10s not 3hrs * Add some more logs to track down the flake
* Try reducing Redis flakiness Based on some recommendations here: StackExchange/StackExchange.Redis#1120 (comment) * Make sure we test against the latest StackExchange versions
* We get occasional SegFaults in NativeProfilerChecks, but we're not doing anything about them at the moment, so stop failing the build * Ignore AbandonedMutexException thrown by Coverlet * Ignore AbandonedMutexException thrown by Coverlet in smoke tests
…ast) (#2478) Get the Native Loader module file path from the HMODULE instead of COR[E]_PROFILER_XXX env variables
…g IncludeMinorPackageVersions (#2475)
* Add ExpandRouteParametersEnabled setting
Implement for ASP.NET Core and add tests
In order to keep the consistent behaviour for routes like `/`, `/Home`, `/Home/Index`, we can't simply short-circuit the route parameter replacement. Instead, we have to replace each individual segment, which is unfortunate, but I don't think it can be avoided
* Update implementation to use {routeTemplate} placeholders instead of '?' (a bit nicer)
* Annotate test data for clarity
* Doc updates
Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com>
* Rename ExpandRouteParameters -> ExpanRouteTemplates
Co-authored-by: Lucas Pimentel-Ordyna <lucas.pimentel@datadoghq.com>
* Add support for more rewriting points * Fixes
|
Notes after short review |
|
Answears from @lachmatt by me: @pellared amd @pjanotti: |
pellared
reviewed
Mar 8, 2022
pjanotti
reviewed
Mar 8, 2022
Kielek
force-pushed
the
pull-upstream-changes
branch
from
8480b3a to
50b33f5
Compare
Kielek
approved these changes
Mar 9, 2022
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.
Why
Keep in sync with upstream.
What
Upstream sync 6989e88..afb9638
Tests
CI