Response to comments

DataDog · Nov 25, 2024 · 8fa918f · 8fa918f
1 parent cfc38c2
commit 8fa918f
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 31 deletions.
diff --git a/tracer/src/Datadog.Trace/AppSec/EventTrackingSdk.cs b/tracer/src/Datadog.Trace/AppSec/EventTrackingSdk.cs
@@ -55,8 +55,8 @@ internal static void TrackUserLoginSuccessEvent(string userId, IDictionary<strin
 
         var setTag = TaggingUtils.GetSpanSetter(span, out var internalSpan);
 
-        setTag(Tags.AppSec.EventsUsers.LoginEvent.SuccessTrack, "true");
-        setTag(Tags.AppSec.EventsUsers.LoginEvent.SuccessSdkSource, "true");
+        setTag(Tags.AppSec.EventsUsers.LoginEvent.SuccessTrack, Tags.AppSec.EventsUsers.True);
+        setTag(Tags.AppSec.EventsUsers.LoginEvent.SuccessSdkSource, Tags.AppSec.EventsUsers.True);
         setTag(Tags.User.Id, userId);
 
         if (metadata is { Count: > 0 })
@@ -128,17 +128,16 @@ internal static void TrackUserLoginFailureEvent(string userId, bool exists, IDic
 
         var setTag = TaggingUtils.GetSpanSetter(span, out var spanInternal);
 
-        setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureTrack, "true");
-        setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureSdkSource, "true");
+        setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureTrack, Tags.AppSec.EventsUsers.True);
+        setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureSdkSource, Tags.AppSec.EventsUsers.True);
         setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserId, userId);
-        setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserExists, exists ? "true" : "false");
+        setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserExists, exists ? Tags.AppSec.EventsUsers.True : Tags.AppSec.EventsUsers.False);
 
         if (metadata is { Count: > 0 })
         {
-            var successTagPrefix = $"{Tags.AppSec.EventsUsers.LoginEvent.Failure}.";
             foreach (var kvp in metadata)
             {
-                setTag(successTagPrefix + kvp.Key, kvp.Value);
+                setTag($"{Tags.AppSec.EventsUsers.LoginEvent.Failure}.{kvp.Key}", kvp.Value);
             }
         }
 
@@ -187,17 +186,14 @@ internal static void TrackCustomEvent(string eventName, IDictionary<string, stri
 
         var setTag = TaggingUtils.GetSpanSetter(span, out var internalSpan);
 
-        setTag(Tags.AppSec.Track(eventName), "true");
-
-        var eventPrefix = $"{Tags.AppSec.Events}{eventName}";
-
-        setTag($"_dd.{eventPrefix}.sdk", "true");
+        setTag(Tags.AppSec.Track(eventName), Tags.AppSec.EventsUsers.True);
+        setTag($"_dd.{Tags.AppSec.Events}{eventName}.sdk", Tags.AppSec.EventsUsers.True);
 
         if (metadata is { Count: > 0 })
         {
             foreach (var kvp in metadata)
             {
-                setTag($"{eventPrefix}.{kvp.Key}", kvp.Value);
+                setTag($"{Tags.AppSec.Events}{eventName}.{kvp.Key}", kvp.Value);
             }
         }
 

diff --git a/...toInstrumentation/AspNetCore/UserEvents/AuthenticationHttpContextExtensionsIntegration.cs b/...toInstrumentation/AspNetCore/UserEvents/AuthenticationHttpContextExtensionsIntegration.cs
@@ -7,7 +7,6 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
-using System.Linq;
 using System.Security.Claims;
 using Datadog.Trace.AppSec;
 using Datadog.Trace.AppSec.Coordinator;
@@ -26,7 +25,7 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNetCore.UserEvents
     [InstrumentMethod(
         AssemblyName = AssemblyName,
         TypeName = HttpContextExtensionsTypeName,
-        ParameterTypeNames = new[] { "Microsoft.AspNetCore.Http.HttpContext", ClrNames.String, "System.Security.Claims.ClaimsPrincipal", "Microsoft.AspNetCore.Authentication.AuthenticationProperties" },
+        ParameterTypeNames = ["Microsoft.AspNetCore.Http.HttpContext", ClrNames.String, "System.Security.Claims.ClaimsPrincipal", "Microsoft.AspNetCore.Authentication.AuthenticationProperties"],
         MethodName = "SignInAsync",
         ReturnTypeName = ClrNames.Task,
         MinimumVersion = Major2,
@@ -42,10 +41,7 @@ public static class AuthenticationHttpContextExtensionsIntegration
         private const string HttpContextExtensionsTypeName = "Microsoft.AspNetCore.Authentication.AuthenticationHttpContextExtensions";
 
         // https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
-        private static readonly HashSet<string> ClaimsToTest = new HashSet<string>
-        {
-            ClaimTypes.NameIdentifier, ClaimTypes.Name, "sub", ClaimTypes.Email,  ClaimTypes.Name
-        };
+        private static readonly HashSet<string> ClaimsToTest = [ClaimTypes.NameIdentifier, ClaimTypes.Name, "sub", ClaimTypes.Email, ClaimTypes.Name];
 
         internal static CallTargetState OnMethodBegin<TTarget>(object httpContext, string scheme, ClaimsPrincipal claimPrincipal, object authProperties)
         {
@@ -96,7 +92,7 @@ internal static object OnAsyncMethodEnd<TTarget>(object returnValue, Exception e
                 if (foundUserId)
                 {
                     security.SetTraceSamplingPriority(span);
-                    setTag(Tags.AppSec.EventsUsers.LoginEvent.SuccessTrack, "true");
+                    setTag(Tags.AppSec.EventsUsers.LoginEvent.SuccessTrack, Tags.AppSec.EventsUsers.True);
                 }
                 else
                 {

diff --git a/...filer/AutoInstrumentation/AspNetCore/UserEvents/SignInManagerPasswordSignInIntegration.cs b/...filer/AutoInstrumentation/AspNetCore/UserEvents/SignInManagerPasswordSignInIntegration.cs
@@ -25,7 +25,7 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNetCore.UserEvents;
     AssemblyName = AssemblyName,
     TypeName = "Microsoft.AspNetCore.Identity.SignInManager`1",
     MethodName = "PasswordSignInAsync",
-    ParameterTypeNames = new[] { ClrNames.String, ClrNames.String, ClrNames.Bool, ClrNames.Bool },
+    ParameterTypeNames = [ClrNames.String, ClrNames.String, ClrNames.Bool, ClrNames.Bool],
     ReturnTypeName = "System.Threading.Tasks.Task`1[Microsoft.AspNetCore.Identity.SignInResult]",
     MinimumVersion = "2",
     MaximumVersion = SupportedVersions.LatestDotNet,
@@ -77,8 +77,8 @@ internal static TReturn OnAsyncMethodEnd<TTarget, TReturn>(TTarget instance, TRe
             var setTag = TaggingUtils.GetSpanSetter(span, out _);
             var tryAddTag = TaggingUtils.GetSpanSetter(span, out _, replaceIfExists: false);
 
-            setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureTrack, "true");
-            tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserExists, "false");
+            setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureTrack, Tags.AppSec.EventsUsers.True);
+            tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserExists, Tags.AppSec.EventsUsers.False);
 
             if (security.IsAnonUserTrackingMode)
             {

diff --git a/...r/AutoInstrumentation/AspNetCore/UserEvents/SignInManagerPasswordSignInUserIntegration.cs b/...r/AutoInstrumentation/AspNetCore/UserEvents/SignInManagerPasswordSignInUserIntegration.cs
@@ -81,16 +81,16 @@ internal static TReturn OnAsyncMethodEnd<TTarget, TReturn>(TTarget instance, TRe
             var tryAddTag = TaggingUtils.GetSpanSetter(span, out _, replaceIfExists: false);
             if (!returnValue.Succeeded)
             {
-                setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureTrack, "true");
+                setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureTrack, Tags.AppSec.EventsUsers.True);
                 setTag(Tags.AppSec.EventsUsers.LoginEvent.FailureAutoMode, security.Settings.UserEventsAutoInstrumentationMode);
-                tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserExists, userExists ? "true" : "false");
+                tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserExists, userExists ? Tags.AppSec.EventsUsers.True : Tags.AppSec.EventsUsers.False);
 
                 if (security.IsAnonUserTrackingMode)
                 {
                     var anonId = UserEventsCommon.GetAnonId(id);
-                    if (anonId != null)
+                    if (!string.IsNullOrEmpty(anonId))
                     {
-                        tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserId, anonId);
+                        tryAddTag(Tags.AppSec.EventsUsers.LoginEvent.FailureUserId, anonId!);
                     }
                 }
                 else
@@ -107,9 +107,9 @@ internal static TReturn OnAsyncMethodEnd<TTarget, TReturn>(TTarget instance, TRe
                 if (security.IsAnonUserTrackingMode)
                 {
                     var anonId = UserEventsCommon.GetAnonId(id);
-                    if (anonId != null)
+                    if (!string.IsNullOrEmpty(anonId))
                     {
-                        tryAddTag(Tags.User.Id, anonId);
+                        tryAddTag(Tags.User.Id, anonId!);
                     }
                 }
                 else

diff --git a/...ace/ClrProfiler/AutoInstrumentation/AspNetCore/UserEvents/UserManagerCreateIntegration.cs b/...ace/ClrProfiler/AutoInstrumentation/AspNetCore/UserEvents/UserManagerCreateIntegration.cs
@@ -85,9 +85,9 @@ internal static TReturn OnAsyncMethodEnd<TTarget, TReturn>(TTarget instance, TRe
                 if (security.IsAnonUserTrackingMode)
                 {
                     var anonId = UserEventsCommon.GetAnonId(id);
-                    if (anonId != null)
+                    if (!string.IsNullOrEmpty(anonId))
                     {
-                        tryAddTag(Tags.AppSec.EventsUsers.SignUpEvent.SuccessUserId, anonId);
+                        tryAddTag(Tags.AppSec.EventsUsers.SignUpEvent.SuccessUserId, anonId!);
                     }
 
                     setTag(Tags.AppSec.EventsUsers.SignUpEvent.SuccessAutoMode, SecuritySettings.UserTrackingAnonMode);

diff --git a/tracer/src/Datadog.Trace/Tags.AppSec.cs b/tracer/src/Datadog.Trace/Tags.AppSec.cs
@@ -18,6 +18,8 @@ internal static class AppSec
 
         internal static class EventsUsers
         {
+            internal const string True = "true";
+            internal const string False = "false";
             internal const string EventsUsersRoot = Events + "users";
 
             internal static class LoginEvent