[ASM] only collect ip for appsec (#3379)

* only colelct ip for appsec * update verify files * Optional tags for ip now * Dont log warning if multiple ip headers found * fix snapshots * regenerate span metadata * fix snapshots * readd edge tags Co-authored-by: Anna <anna.yafi@datadoghq.com>
DataDog · Oct 24, 2022 · 0f7c26d · 0f7c26d
1 parent 0088407
commit 0f7c26d
Show file tree

Hide file tree

Showing 869 changed files with 33 additions and 2,351 deletions.
diff --git a/docs/span_metadata.md b/docs/span_metadata.md
@@ -38,14 +38,14 @@ Type | `web`
 ### Tags
 Name | Required |
 ---------|----------------|
-http.client_ip | Yes
+http.client_ip | No
 http.method | Yes
 http.request.headers.host | Yes
 http.route | No
 http.status_code | Yes
 http.url | Yes
 http.useragent | Yes
-network.client.ip | Yes
+network.client.ip | No
 span.kind | `server`
 
 ## AspNetMvc
@@ -102,14 +102,14 @@ Name | Required |
 aspnet_core.endpoint | No
 aspnet_core.route | No
 component | `aspnet_core`
-http.client_ip | Yes
+http.client_ip | No
 http.method | Yes
 http.request.headers.host | Yes
 http.route | No
 http.status_code | Yes
 http.url | Yes
 http.useragent | Yes
-network.client.ip | Yes
+network.client.ip | No
 span.kind | `server`
 
 ## AspNetCoreMvc

diff --git a/tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs b/tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs
@@ -149,7 +149,7 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
                 scope = tracer.StartActiveInternal(_requestOperationName, propagatedContext, tags: tags);
                 // Leave resourceName blank for now - we'll update it in OnEndRequest
                 scope.Span.DecorateWebServerSpan(resourceName: null, httpMethod, host, url, userAgent, tags, tagsFromHeaders);
-                if (!tracer.Settings.IpHeaderDisabled)
+                if (Security.Instance.Settings.Enabled)
                 {
                     Headers.Ip.RequestIpExtractor.AddIpToTags(httpRequest.UserHostAddress, httpRequest.IsSecureConnection, key => httpRequest.Headers[key], tracer.Settings.IpHeader, tags);
                 }

diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetWebApi2Integration.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetWebApi2Integration.cs
@@ -8,6 +8,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Web;
+using Datadog.Trace.AppSec;
 using Datadog.Trace.AspNet;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.DuckTyping;
@@ -67,9 +68,9 @@ internal static Scope CreateScope(IHttpControllerContext controllerContext, out
                         Log.Error(ex, "Error extracting propagated HTTP headers.");
                     }
 
-                    const string httpContextKey = "MS_HttpContext";
-                    if (!tracer.Settings.IpHeaderDisabled)
+                    if (Security.Instance.Settings.Enabled)
                     {
+                        const string httpContextKey = "MS_HttpContext";
                         if (request.Properties.TryGetValue("MS_OwinContext", out var owinContextObj))
                         {
                             if (owinContextObj != null)

diff --git a/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.cs b/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.cs
@@ -311,12 +311,6 @@ internal static partial class ConfigurationKeys
         /// <seealso cref="TracerSettings.IpHeader"/>
         public const string IpHeader = "DD_TRACE_CLIENT_IP_HEADER";
 
-        /// <summary>
-        /// Configuration key indicating if the header should not be collected. The default for DD_TRACE_CLIENT_IP_HEADER_DISABLED is false.
-        /// </summary>
-        /// <seealso cref="TracerSettings.IpHeaderDisabled"/>
-        public const string IpHeaderDisabled = "DD_TRACE_CLIENT_IP_HEADER_DISABLED";
-
         /// <summary>
         /// Configuration key to enable or disable the creation of a span context on exiting a successful Kafka
         /// Consumer.Consume() call, and closing the scope on entering Consumer.Consume().

diff --git a/tracer/src/Datadog.Trace/Configuration/ImmutableTracerSettings.cs b/tracer/src/Datadog.Trace/Configuration/ImmutableTracerSettings.cs
@@ -52,7 +52,6 @@ public ImmutableTracerSettings(TracerSettings settings)
             HeaderTags = new ReadOnlyDictionary<string, string>(settings.HeaderTags);
             GrpcTags = new ReadOnlyDictionary<string, string>(settings.GrpcTags);
             IpHeader = settings.IpHeader;
-            IpHeaderDisabled = settings.IpHeaderDisabled;
             TracerMetricsEnabled = settings.TracerMetricsEnabled;
             StatsComputationEnabled = settings.StatsComputationEnabled;
             StatsComputationInterval = settings.StatsComputationInterval;
@@ -193,11 +192,6 @@ public ImmutableTracerSettings(TracerSettings settings)
         /// </summary>
         internal string IpHeader { get; }
 
-        /// <summary>
-        /// Gets a value indicating whether the ip header should not be collected. The default is false.
-        /// </summary>
-        internal bool IpHeaderDisabled { get; }
-
         /// <summary>
         /// Gets a value indicating whether internal metrics
         /// are enabled and sent to DogStatsd.

diff --git a/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs b/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
@@ -214,8 +214,6 @@ public TracerSettings(IConfigurationSource source)
 
             IpHeader = source?.GetString(ConfigurationKeys.IpHeader) ?? source?.GetString(ConfigurationKeys.AppSec.CustomIpHeader);
 
-            IpHeaderDisabled = source?.GetBool(ConfigurationKeys.IpHeaderDisabled) ?? false;
-
             if (IsActivityListenerEnabled)
             {
                 // If the activities support is activated, we must enable W3C propagators
@@ -340,11 +338,6 @@ public bool LogsInjectionEnabled
         /// </summary>
         internal string IpHeader { get; set; }
 
-        /// <summary>
-        /// Gets or sets a value indicating whether the ip header should not be collected. The default is false.
-        /// </summary>
-        internal bool IpHeaderDisabled { get; set; }
-
         /// <summary>
         /// Gets or sets the map of metadata keys to tag names, which are applied to the root <see cref="Span"/>
         /// of incoming and outgoing GRPC requests.

diff --git a/tracer/src/Datadog.Trace/Headers/Ip/RequestIpExtractor.cs b/tracer/src/Datadog.Trace/Headers/Ip/RequestIpExtractor.cs
@@ -43,7 +43,8 @@ internal static IpInfo ExtractIpAndPort(Func<string, string> getHeader, string c
                 {
                     if (!string.IsNullOrEmpty(potentialIp) || !string.IsNullOrEmpty(customIpHeader))
                     {
-                        Log.Warning("Multiple ip headers have been found, none will be reported", potentialIp);
+                        // dont log more than debug level. some proxies have several ip headers and we end up with millions of logs.
+                        Log.Debug("Multiple ip headers have been found, none will be reported");
                         return null;
                     }
 

diff --git a/tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs b/tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs
@@ -7,6 +7,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Datadog.Trace.AppSec;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.ExtensionMethods;
 using Datadog.Trace.Headers;
@@ -121,7 +122,7 @@ public Scope StartAspNetCorePipelineScope(Tracer tracer, HttpContext httpContext
 
             var scope = tracer.StartActiveInternal(_requestInOperationName, propagatedContext, tags: tags);
             scope.Span.DecorateWebServerSpan(resourceName, httpMethod, host, url, userAgent, tags, tagsFromHeaders);
-            if (!tracer.Settings.IpHeaderDisabled)
+            if (Security.Instance.Settings.Enabled)
             {
                 var peerIp = new Headers.Ip.IpInfo(httpContext.Connection.RemoteIpAddress?.ToString(), httpContext.Connection.RemotePort);
                 Func<string, string> getRequestHeaderFromKey = key => request.Headers.TryGetValue(key, out var value) ? value : string.Empty;

diff --git a/tracer/test/Datadog.Trace.TestHelpers/SpanMetadataRules.cs b/tracer/test/Datadog.Trace.TestHelpers/SpanMetadataRules.cs
@@ -36,7 +36,8 @@ public static Result IsAspNet(this MockSpan span) => Result.FromSpan(span)
                 .Matches(Name, "aspnet.request")
                 .Matches(Type, "web"))
             .Tags(s => s
-                .IsPresent("http.client_ip")
+                .IsOptional("http.client_ip")
+                .IsOptional("network.client.ip")
                 .IsPresent("http.method")
                 .IsPresent("http.request.headers.host")
                 .IsOptional("http.route")
@@ -45,7 +46,6 @@ public static Result IsAspNet(this MockSpan span) => Result.FromSpan(span)
                 .IsPresent("http.url")
                 // BUG: component tag is not set
                 // .Matches("component", "aspnet")
-                .IsPresent("network.client.ip")
                 .Matches("span.kind", "server"));
 
         public static Result IsAspNetMvc(this MockSpan span) => Result.FromSpan(span)
@@ -75,6 +75,7 @@ public static Result IsAspNetWebApi2(this MockSpan span) => Result.FromSpan(span
                 .IsOptional("aspnet.controller")
                 .IsPresent("aspnet.route")
                 .IsOptional("http.client_ip")
+                .IsOptional("network.client.ip")
                 .IsPresent("http.method")
                 .IsPresent("http.request.headers.host")
                 .IsOptional("http.route")
@@ -90,7 +91,6 @@ public static Result IsAspNetWebApi2(this MockSpan span) => Result.FromSpan(span
                 .IsPresent("http.url")
                 // BUG: component tag is not set
                 // .Matches("component", "aspnet")
-                .IsOptional("network.client.ip")
                 .Matches("span.kind", "server"));
 
         public static Result IsAspNetCore(this MockSpan span, ISet<string> excludeTags = null) => Result.FromSpan(span, excludeTags)
@@ -100,14 +100,14 @@ public static Result IsAspNetCore(this MockSpan span, ISet<string> excludeTags =
             .Tags(s => s
                 .IsOptional("aspnet_core.endpoint")
                 .IsOptional("aspnet_core.route")
-                .IsPresent("http.client_ip")
+                .IsOptional("http.client_ip")
+                .IsOptional("network.client.ip")
                 .IsPresent("http.method")
                 .IsPresent("http.request.headers.host")
                 .IsOptional("http.route")
                 .IsPresent("http.status_code")
                 .IsPresent("http.useragent")
                 .IsPresent("http.url")
-                .IsPresent("network.client.ip")
                 .Matches("component", "aspnet_core")
                 .Matches("span.kind", "server"));
 

diff --git a/...apshots/AspNetCoreIisMinimalApisTests.InProcess.NoFF.__path=__statusCode=200.verified.txt b/...apshots/AspNetCoreIisMinimalApisTests.InProcess.NoFF.__path=__statusCode=200.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 200,
       http.url: http://localhost:00000/,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...NetCoreIisMinimalApisTests.InProcess.NoFF.__path=_api_delay_0_statusCode=200.verified.txt b/...NetCoreIisMinimalApisTests.InProcess.NoFF.__path=_api_delay_0_statusCode=200.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 200,
       http.url: http://localhost:00000/api/delay/0,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...NetCoreIisMinimalApisTests.InProcess.NoFF.__path=_bad-request_statusCode=500.verified.txt b/...NetCoreIisMinimalApisTests.InProcess.NoFF.__path=_bad-request_statusCode=500.verified.txt
@@ -15,14 +15,12 @@
 System.Exception: This was a bad request.
 at Samples.AspNetCoreMvc.Controllers.HomeController.ThrowException(),
       error.type: System.Exception,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 500,
       http.url: http://localhost:00000/bad-request,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...reIisMinimalApisTests.InProcess.NoFF.__path=_branch_not-found_statusCode=404.verified.txt b/...reIisMinimalApisTests.InProcess.NoFF.__path=_branch_not-found_statusCode=404.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 404,
       http.url: http://localhost:00000/branch/not-found,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...NetCoreIisMinimalApisTests.InProcess.NoFF.__path=_branch_ping_statusCode=200.verified.txt b/...NetCoreIisMinimalApisTests.InProcess.NoFF.__path=_branch_ping_statusCode=200.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 200,
       http.url: http://localhost:00000/branch/ping,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/.../AspNetCoreIisMinimalApisTests.InProcess.NoFF.__path=_delay_0_statusCode=200.verified.txt b/.../AspNetCoreIisMinimalApisTests.InProcess.NoFF.__path=_delay_0_statusCode=200.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 200,
       http.url: http://localhost:00000/delay/0,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...eIisMinimalApisTests.InProcess.NoFF.__path=_handled-exception_statusCode=500.verified.txt b/...eIisMinimalApisTests.InProcess.NoFF.__path=_handled-exception_statusCode=500.verified.txt
@@ -15,14 +15,12 @@
 System.Exception: Exception of type 'System.Exception' was thrown.
 ,
       error.type: System.Exception,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 500,
       http.url: http://localhost:00000/handled-exception,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...spNetCoreIisMinimalApisTests.InProcess.NoFF.__path=_not-found_statusCode=404.verified.txt b/...spNetCoreIisMinimalApisTests.InProcess.NoFF.__path=_not-found_statusCode=404.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 404,
       http.url: http://localhost:00000/not-found,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...ots/AspNetCoreIisMinimalApisTests.InProcess.NoFF.__path=_ping_statusCode=200.verified.txt b/...ots/AspNetCoreIisMinimalApisTests.InProcess.NoFF.__path=_ping_statusCode=200.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 200,
       http.url: http://localhost:00000/ping,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...imalApisTests.InProcess.NoFF.__path=_status-code-string_[200]_statusCode=500.verified.txt b/...imalApisTests.InProcess.NoFF.__path=_status-code-string_[200]_statusCode=500.verified.txt
@@ -15,14 +15,12 @@
 System.Exception: Input was not a status code
 at Samples.AspNetCoreMvc.Controllers.HomeController.StatusCodeTestString(String input),
       error.type: System.Exception,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 500,
       http.url: http://localhost:00000/status-code-string/%5B200%5D,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...oreIisMinimalApisTests.InProcess.NoFF.__path=_status-code_203_statusCode=203.verified.txt b/...oreIisMinimalApisTests.InProcess.NoFF.__path=_status-code_203_statusCode=203.verified.txt
@@ -9,14 +9,12 @@
     Tags: {
       component: aspnet_core,
       env: integration_tests,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 203,
       http.url: http://localhost:00000/status-code/203,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...oreIisMinimalApisTests.InProcess.NoFF.__path=_status-code_402_statusCode=402.verified.txt b/...oreIisMinimalApisTests.InProcess.NoFF.__path=_status-code_402_statusCode=402.verified.txt
@@ -11,14 +11,12 @@
       component: aspnet_core,
       env: integration_tests,
       error.msg: The HTTP response has status code 402.,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 402,
       http.url: http://localhost:00000/status-code/402,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,

diff --git a/...oreIisMinimalApisTests.InProcess.NoFF.__path=_status-code_500_statusCode=500.verified.txt b/...oreIisMinimalApisTests.InProcess.NoFF.__path=_status-code_500_statusCode=500.verified.txt
@@ -11,14 +11,12 @@
       component: aspnet_core,
       env: integration_tests,
       error.msg: The HTTP response has status code 500.,
-      http.client_ip: 127.0.0.1,
       http.method: GET,
       http.request.headers.host: localhost:00000,
       http.status_code: 500,
       http.url: http://localhost:00000/status-code/500,
       http.useragent: testhelper,
       language: dotnet,
-      network.client.ip: ::1,
       runtime-id: Guid_1,
       span.kind: server,
       version: 1.0.0,