Skip to content

AAS - Exclude URLs from Tracing#1447

Merged
colin-higgins merged 16 commits into
masterfrom
colin/exclude-infrastructure-http-calls
May 19, 2021
Merged

AAS - Exclude URLs from Tracing#1447
colin-higgins merged 16 commits into
masterfrom
colin/exclude-infrastructure-http-calls

Conversation

@colin-higgins

@colin-higgins colin-higgins commented May 4, 2021

Copy link
Copy Markdown
Member

Exclude certain URLs from tracing to avoid multiple top level spans.

This is important to prevent noise and excess CPU/Memory consumption in Azure Functions

Benchmarks run on multiple cultures and had consistent results across each, against all lowercase URL (which is best case scenario for ToLower and ToLowerInvariant as it avoids an allocation.)

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19042
Intel Core i7-8750H CPU 2.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=5.0.104
  [Host]     : .NET Core 3.1.14 (CoreCLR 4.700.21.16201, CoreFX 4.700.21.16208), X64 RyuJIT
  DefaultJob : .NET Core 3.1.14 (CoreCLR 4.700.21.16201, CoreFX 4.700.21.16208), X64 RyuJIT


|                                     Method |     Mean |   Error |  StdDev |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|------------------------------------------- |---------:|--------:|--------:|-------:|------:|------:|----------:|
|          CheckToLowerNoMatchStringContains | 239.2 ns | 1.99 ns | 1.66 ns |      - |     - |     - |         - |
|            CheckToLowerMatchStringContains | 157.5 ns | 1.22 ns | 1.08 ns |      - |     - |     - |         - |
| CheckToLowerInvariantNoMatchStringContains | 254.2 ns | 4.29 ns | 4.76 ns |      - |     - |     - |         - |
|   CheckToLowerInvariantMatchStringContains | 154.2 ns | 0.90 ns | 0.84 ns |      - |     - |     - |         - |
|          CheckToUpperNoMatchStringContains | 153.5 ns | 2.33 ns | 2.07 ns | 0.0474 |     - |     - |     224 B |
|            CheckToUpperMatchStringContains | 157.3 ns | 1.63 ns | 1.44 ns | 0.0527 |     - |     - |     248 B |
| CheckToUpperInvariantNoMatchStringContains | 148.6 ns | 2.58 ns | 2.42 ns | 0.0474 |     - |     - |     224 B |
|   CheckToUpperInvariantMatchStringContains | 154.4 ns | 1.15 ns | 1.08 ns | 0.0527 |     - |     - |     248 B |

edit:
setting name is DD_TRACE_HTTP_CLIENT_EXCLUDED_URL_SUBSTRINGS or TracerSettings.HttpClientExcludedUrlSubstrings - Lucas

@colin-higgins colin-higgins self-assigned this May 4, 2021
@colin-higgins
colin-higgins requested a review from a team as a code owner May 4, 2021 21:22
Comment thread src/Datadog.Trace/Configuration/ConfigurationKeys.cs Outdated
Comment thread src/Datadog.Trace.ClrProfiler.Managed/Helpers/HttpBypassHelper.cs Outdated
Comment thread src/Datadog.Trace.ClrProfiler.Managed/Helpers/HttpBypassHelper.cs Outdated
Comment thread src/Datadog.Trace/Configuration/TracerSettings.cs Outdated
Comment thread src/Datadog.Trace/Configuration/TracerSettings.cs Outdated
Comment thread src/Datadog.Trace.ClrProfiler.Managed/Helpers/HttpBypassHelper.cs Outdated
Comment thread test/Datadog.Trace.ClrProfiler.Managed.Tests/HttpBypassTests.cs Outdated
Comment thread src/Datadog.Trace.ClrProfiler.Managed/Helpers/HttpBypassHelper.cs Outdated
Comment thread src/Datadog.Trace/Configuration/TracerSettings.cs Outdated
@colin-higgins colin-higgins changed the title Colin/exclude infrastructure http calls AAS - Exclude URLs from Tracing May 6, 2021
@colin-higgins
colin-higgins force-pushed the colin/exclude-infrastructure-http-calls branch from 3e0cc79 to e40ea6c Compare May 7, 2021 15:12
Comment thread src/Datadog.Trace.ClrProfiler.Managed/Helpers/HttpBypassHelper.cs Outdated
Comment thread src/Datadog.Trace/Configuration/ConfigurationKeys.cs Outdated
Comment thread src/Datadog.Trace/Configuration/TracerSettings.cs Outdated
Comment thread test/benchmarks/Benchmarks.Trace/Program.cs
Comment thread src/Datadog.Trace/Configuration/TracerSettings.cs Outdated
Comment thread test/Datadog.Trace.Tests/TracerSettingsTests.cs Outdated

@lucaspimentel lucaspimentel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Just left a comment to add a single space in one of the test cases.

@colin-higgins
colin-higgins force-pushed the colin/exclude-infrastructure-http-calls branch from 2444e31 to 2ad6240 Compare May 18, 2021 17:56

using (var automaticScope = ScopeFactory.CreateOutboundHttpScope(tracer, "GET", null, new IntegrationInfo((int)IntegrationIds.HttpMessageHandler), out _))
{
Assert.Equal(expected: "GET ", actual: automaticScope.Span.ResourceName);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documents current behavior on null
We might want to look at changing this later.

@colin-higgins
colin-higgins merged commit 91196b6 into master May 19, 2021
@colin-higgins
colin-higgins deleted the colin/exclude-infrastructure-http-calls branch May 19, 2021 02:24
@lucaspimentel lucaspimentel added area:azure-site-extension Changes for AAS support area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:new-feature labels May 20, 2021
@andrewlock andrewlock added this to the 1.27.0 milestone Jun 1, 2021
zacharycmontoya pushed a commit to open-telemetry/opentelemetry-dotnet-instrumentation that referenced this pull request Jun 22, 2021
* Upstream sync 2021-05-31

Complete ARM64 support (#1449)

* Initial native checks for ARM64 and ARM.

* Comparing ICorProfilerCallback10 uuid for arm architecture.

* Change the GUID type to IID

* Fix compilation issue.

* Print GUID.

* Fix for linux

* changes.

* Revert class_factory.cpp modifications.

* Add test for .NET 5.0 runtime on ARM/ARM64 architectures.

* Fixes force calltarget.

* Change the behavior from force calltarget to enable calltarget by default.

* Include ARM64 build into the packages pipeline.

* Fix packages pipeline

* Adds ARM64 build to dotnet tool/cli

* chmod +x package.arm64.sh

* publish ddtrace arm64 standalone version.

* Add ARM64 build to proj file.

* change arm64 folder readme.

* Includes package.arm64.sh to SetAllVersions.cs file

* Simplify package pipeline for Linux X64 and Arm64 build.
Include calculated architecture in the unsupported error message of the dotnet tool.

* Removes package.arm64.sh from the SetAllVersion tool.

* Trying to fix `The ARCHITECTURE variable is not set. Defaulting to a blank string` message.

* Addressing PR comments.

* fix package.sh typo

Msmq  integration, taking into account all comments from PR #1457. (#1463)

* msmq integration

Co-authored-by: anna-git <anna,[email protected]>

Fix Call opcode in DuckTyping on struct targets. (#1469)

* Call/CallVirt Opcode fix when the target object is an struct (InProperties).

* Change `CallVirt` to `Call` when target object is a ValueType.

add Windows container sample (#1472)

* PR for Windows Container example

* Update samples/WindowsContainer/README.md

Co-authored-by: Lucas Pimentel-Ordyna <[email protected]>

* Update samples/WindowsContainer/Dockerfile

Co-authored-by: Lucas Pimentel-Ordyna <[email protected]>

* updated based on PR comments

* removing wwwroot/lib

* removed layout & moved to cdn

* setup for automatic tracer version updates

Co-authored-by: jay.dave <[email protected]>
Co-authored-by: Lucas Pimentel-Ordyna <[email protected]>

AAS - Exclude URLs from Tracing (#1447)

Enable classic mode in IIS tests (#1462)

* Enable classic mode in IIS tests

CIApp: Testing frameworks integrations test suite and fixes (#1317)

* Initial Samples projects structure.

* Initial xunit test suite with simple tests.

* Add XUnit Traits test

* Adding more tests.

* enable detailed logs

* Changes.

* debugging info.

* change

* Adds NUnit Test suite and fixes the cases when a test parameterized is skipped reporting different number of tests than console.

* Fix TestProperties on MsTest integration

* changes.

* Fix Parameterized tests on MsTestV2 integration.

* Fix MSTestV2 basic test suite.

* The issue with the XUnit test was that it relied on importing the file xunit.runner.visualstudio/<version>/build/<TFM>/xunit.runner.visualstudio.props (I'm guessing to place xunit.runner.visualstudio.dotnetcore.testadapter.dll in the output directory), but it was not imported because the Import evaluation for the Publish MSBuild Task occurred before the Restore MSBuild Task finished. The unoptimized solution for now is to invoke MSBuild twice. This requires two separate MSBuild processes to start up and might cause accidental rebuilds, but it works for now.

* Changes.

* change test projects.

* Fix integration for Skip parameterized tests.

* Ignore unrelated spans.

* Changes.

* Write spans on errors.

* fix.

* check the nunit version when running in net5.0

* changes.

* Write ProcessId and TargetProcessId

* update integrations.json

* Remove the inlining case.

* Fix Arm64 build and Add debug flag to the test integrations test cases.

* Add more debug flags.

* remove the logger.

* Adding environmental information of the test suite project.

* Remove ambiguity of Category attribute.

* Add NUnitTestAssemblyRunner.WaitForCompletion integration.

* Fix integration

* fix integrations.json

* Fixes the NUnit .NET 5 test case issue.

* Refactor test integrations and Add integration to XUnitTestAssemblyRunner.RunAsync.

* Logging flush.

* Adding Thread.Sleep to the span flush due the agentwriter behavior.

* Refactor internal flushspans logic.

* update integrations.json

* update integrations.json

* Avoiding any syncronization issues due Async over Sync

* Removing unnecessary SyncronizationContext switching

* Move the test tracer creation out of the .cctor

* Try "methodDisplay" = "method" value in the xunit.runner

* Update test/Datadog.Trace.TestHelpers/EnvironmentHelper.cs

Co-authored-by: Andrew Lock <[email protected]>

* Solves support for test name with only method name.

* Force foreground thread to block process exit to ensure flushing.

* Addressing PR comments.

* Final changes.

* Addressing PR comments for callsite.

Co-authored-by: Zach Montoya <[email protected]>
Co-authored-by: Andrew Lock <[email protected]>

Add kafka instrumentation (#1444)

* Add Kafka sample

Demonstrates multiple producers, multiple consumers, auto commit, manual commit tombstones etc

* Add docker-compose entries for running Kafka

Based on https://github.com/confluentinc/cp-all-in-one/blob/6.1.1-post/cp-all-in-one/docker-compose.yml
Run the full kafaka services using: docker-compose up kafka-control-center kafka-rest-proxy
Visit the Confluent Control Center at http://localhost:9021/

Note that I omitted the ksqldb and connect services

Added wait-for-it to integration test scripts and added to sample build scripts

* Add Kafka integration

Supports version 1.4.x-2.0.0 (May need to be careful there, internal API changed significantly in 1.4.x, so it could happen again)

Handles async producer, sync producer (with delivery handler), sync producer (no delivery handler), and consumer.

Span context is propagated from producer to consumer using headers, but due to the API design for consumers, we immediately close the span after creation, meaning work that _uses_ that Kafka record won't be using the span,

* Add tests for Kafka integration and helpers

* Finish KafkaHeadersCollectionAdapter implementation

Add Remove support and fix Set implementation

* Redesign consumer integration to assume a canonical setup, and thereby allow propagating scope

* Fix use injected tracer in KafkaHelper

* Update integrations.json

* Add setting to allow disabling the creation of "long-lived" consumer scopes

This may be necessary for customers that are using more complex patterns of consumers, other than the "single-topic in a while loop" approach

* Both consumers may consume a message, leading to "duplicate" spans

This is a Kafka design issue though, rather than an instrumentation issue - those consumes are correct, so we expect _at least_ x spans to be generated for consumers.

MessagePack Formatter: Cache Span tag keys in UTF8 (#1482)

Add and enforce copyright headers for C# files under /build, /src, and /tools (#1445)

- Modifies stylecop rules to enforce copyright headers on C# files
- Runs the automated rule to add the missing copyright headers
- Affects all subdirectories EXCEPT /test, which currently doesn't have stylecop applied

Optimize log injection with NLog (#1475)

Add missing headers (#1485)

_dd.origin tag improvements. (#1481)

* Initial works on including the _dd.origin tag on all spans and internal api.

* Fixes unit tests

* Adds Benchmark tests.

* performance improvements.

* Benchmark warmup.

* changes

* Add unit tests.

* Fixes test integrations test suite including the new _dd.origin tag.

* Fix MSTest origin tag.

* Change encoding object.

* Removes unnecessary condition

* Update test/benchmarks/Benchmarks.Trace/AgentWriterBenchmark.cs

Co-authored-by: Zach Montoya <[email protected]>

* Fix tests.

Co-authored-by: Zach Montoya <[email protected]>

Add DuckInclude attribute (#1487)

Temporal skip to avoid flakiness in xunit 2.2.0 test. (#1488)

Skip building Samples.OracleMDA.* projects (#1477)

* remove unused Samples.OracleMDA and Samples.OracleMDA.Core from build scripts
* run GeneratePackageVersions tool

Clean up project files (reduce conditionals and reference assemblies) (#1468)

* remove all references to nuget package Microsoft.NETFramework.ReferenceAssemblies

* remove most <TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">

* remove <PublicSign>

* remove redundant props file

* only build this project on netfx

* use default targets in test dependencies

Add GH Action to auto-create benchmark branch (#1483)

When we push a tag, we want to auto-create the benchmarks branch. This should extract the version number for the tag, and create a corresponding benchmarks branch. The benchmarks branches are used by CI to show historical performance.

We use a branch instead of keying directly off the tags, as we don't want to benchmark _every_ release, and so we can delete branches if applicable.

Add restart policy to Kafka containers (#1492)

The broker has a tendency to fall over, try and handle that

Adds ARM64 throughput test (#1490)

* Initial work to arm64 crank support.

* pipeline changes.

Convert ASP.NET tests to use Snapshot Testing with VerifyTests/Verify (#1454)

* Add required gitignore for Verify received files

These should never be checked in (in contrast to *.verified.* files

* Install Verify and update Newtonsoft Json

Json.NET update required as Verify depends on 13.0.1

* Add ModuleInitializerAttribute for < .NET 5

Lets us use C# 9 Module Initializers on < .NET 5

* Ensure sample apps have a consistent environment across machines

* Populate $(SolutionDir) property if not already set

* Add some helpers for verifying MockTracerAgent.Spans

We may need to add additional scrubbers etc.
In addition to the built-in scrubbers, I'm
- Excluding the variable duration/start fields
- Fixing localhost ports to all be :0000
- Fixing the _dd.tracer_kr: 1.0

Also adds helper for fetching the web spans (without asserting), and for sanitising paths for use in filenames

* Convert AspNetMvc4 tests to use Snapshot testing

* Convert AspNetMvc5 tests to use Snapshot testing

* Convert AspNetWebApi2 tests to use Snapshot testing

* Include architecture in the file name for aspnetcore tests

The stack traces are different depending on architecture.
There's a PR in to Verify to support this OOTB: VerifyTests/Verify#367

* Add x64 Snapshots

* Add x86 Snapshots

* Add copyright headers

* Fix CI app tests

Refactor DisposeWithException invocations to remove null conditional operator ?. (#1493)

Fixes .vcxproj file for latest msbuild version. (#1495)

Add GH Action to auto-create benchmark branch (#1483)

When we push a tag, we want to auto-create the benchmarks branch. This should extract the version number for the tag, and create a corresponding benchmarks branch. The benchmarks branches are used by CI to show historical performance.

We use a branch instead of keying directly off the tags, as we don't want to benchmark _every_ release, and so we can delete branches if applicable.

Add runtime-id tag to metrics (#1496)

Remove the static collection in the HTTP module (#1498)

* Remove the static collection in the HTTP module

* Fixes after upstream sync

* Remove test introduced in DataDog/dd-trace-dotnet#1447

* Support longpaths on Windows workflow

* Skip OriginTagSendTraces tests

* Skip failing integration tests

* Ignore correct tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:azure-site-extension Changes for AAS support area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:new-feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants