response to comments

DataDog · Dec 13, 2024 · e9072d2 · e9072d2
1 parent ee5f058
commit e9072d2
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 24 deletions.
diff --git a/tracer/build/supported_calltargets.g.json b/tracer/build/supported_calltargets.g.json
@@ -10591,10 +10591,10 @@
       "Item2": 65535,
       "Item3": 65535
     },
-    "InstrumentationTypeName": "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderCloseNpgsqlIntegration",
+    "InstrumentationTypeName": "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql.ReaderCloseNpgsqlIntegration",
     "IntegrationKind": 0,
     "IsAdoNetIntegration": false,
-    "InstrumentationCategory": 1
+    "InstrumentationCategory": 4
   },
   {
     "IntegrationName": "Npgsql",

diff --git a/...on/AdoNet/ReaderCloseNpgsqlIntegration.cs → ...et/Npgsql/ReaderCloseNpgsqlIntegration.cs b/...on/AdoNet/ReaderCloseNpgsqlIntegration.cs → ...et/Npgsql/ReaderCloseNpgsqlIntegration.cs
@@ -12,7 +12,7 @@
 using Datadog.Trace.Iast;
 using Datadog.Trace.Vendors.Serilog;
 
-namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet
+namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql
 {
     /// <summary>
     /// CallTarget instrumentation for:
@@ -27,24 +27,14 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet
         ParameterTypeNames = new[] { ClrNames.Bool, ClrNames.Bool, ClrNames.Bool, },
         MinimumVersion = "4.0.0",
         MaximumVersion = "8.*.*",
-        IntegrationName = nameof(IntegrationId.Npgsql))]
+        IntegrationName = nameof(IntegrationId.Npgsql),
+        InstrumentationCategory = InstrumentationCategory.Iast)]
     [Browsable(false)]
     [EditorBrowsable(EditorBrowsableState.Never)]
     public class ReaderCloseNpgsqlIntegration
     {
         private static bool errorLogged = false;
 
-        /// <summary>
-        /// OnMethodEnd callback
-        /// </summary>
-        /// <typeparam name="TTarget">Type of the target</typeparam>
-        /// <typeparam name="TReturn">Type of the return</typeparam>
-        /// <param name="instance">Instance value, aka `this` of the instrumented method.</param>
-        /// <param name="returnValue">Instance of the return value</param>
-        /// <param name="exception">Exception instance in case the original code threw an exception.</param>
-        /// <param name="state">Calltarget state value</param>
-        /// <returns>A response value, in an async scenario will be T of Task of T</returns>
-        // internal static CallTargetReturn OnMethodEnd<TTarget>(TTarget instance, Exception? exception, in CallTargetState state)
         internal static TReturn OnAsyncMethodEnd<TTarget, TReturn>(TTarget instance, TReturn returnValue, Exception exception, in CallTargetState state)
         {
             try

diff --git a/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs b/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs
@@ -408,7 +408,7 @@ internal static bool IsInstrumentedAssembly(string assemblyName)
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV4"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV5"
                     => Datadog.Trace.Configuration.IntegrationId.NLog,
-                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderCloseNpgsqlIntegration"
+                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql.ReaderCloseNpgsqlIntegration"
                     => Datadog.Trace.Configuration.IntegrationId.Npgsql,
                 "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"

diff --git a/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs b/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs
@@ -437,7 +437,7 @@ internal static bool IsInstrumentedAssembly(string assemblyName)
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV4"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV5"
                     => Datadog.Trace.Configuration.IntegrationId.NLog,
-                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderCloseNpgsqlIntegration"
+                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql.ReaderCloseNpgsqlIntegration"
                     => Datadog.Trace.Configuration.IntegrationId.Npgsql,
                 "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"

diff --git a/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs b/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs
@@ -432,7 +432,7 @@ internal static bool IsInstrumentedAssembly(string assemblyName)
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV4"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV5"
                     => Datadog.Trace.Configuration.IntegrationId.NLog,
-                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderCloseNpgsqlIntegration"
+                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql.ReaderCloseNpgsqlIntegration"
                     => Datadog.Trace.Configuration.IntegrationId.Npgsql,
                 "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"

diff --git a/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs b/...race.SourceGenerators/InstrumentationDefinitionsGenerator/InstrumentationDefinitions.g.cs
@@ -432,7 +432,7 @@ internal static bool IsInstrumentedAssembly(string assemblyName)
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV4"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Logging.NLog.LogsInjection.LoggerImplWriteIntegrationV5"
                     => Datadog.Trace.Configuration.IntegrationId.NLog,
-                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderCloseNpgsqlIntegration"
+                "Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql.ReaderCloseNpgsqlIntegration"
                     => Datadog.Trace.Configuration.IntegrationId.Npgsql,
                 "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"
                     or "Datadog.Trace.ClrProfiler.AutoInstrumentation.Testing.NUnit.NUnitCommandBuilderMakeTestCommandIntegration"

diff --git a/tracer/src/Datadog.Trace/Iast/IastModule.cs b/tracer/src/Datadog.Trace/Iast/IastModule.cs
@@ -904,10 +904,14 @@ public void UnregisterDbRecord(object instance)
             dataBaseRows.Remove(instance);
         }
 
-        // note: the return value this method indicates if the value could have been tainted
-        // if a iast context is available. It will return true if iast is enabled and the row
-        // count conditions are met. The result is used in the unit tests, don't use it
-        // anywhere else
+        /// <summary>
+        /// Add a DB value to be tainted
+        /// </summary>
+        /// <returns>The return value this method indicates if the value could have been tainted
+        /// if a iast context is available. It will return true if iast is enabled and the row
+        /// count conditions are met. The result is used in the unit tests, don't use it
+        /// anywhere else
+        /// </returns>
         public bool AddDbValue(object instance, string? column, string value)
         {
             if (!iastSettings.Enabled || iastSettings.DataBaseRowsToTaint <= 0)

diff --git a/tracer/src/Datadog.Tracer.Native/Generated/generated_calltargets.g.h b/tracer/src/Datadog.Tracer.Native/Generated/generated_calltargets.g.h
@@ -755,7 +755,7 @@ std::vector<CallTargetDefinition3> g_callTargets=
 {(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlCommand"),(WCHAR*)WStr("ExecuteReaderAsync"),g_callTargets_Sig_221,3,4,0,0,9,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.CommandExecuteReaderWithBehaviorAndCancellationAsyncIntegration"),CallTargetKind::Default,1,15},
 {(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlCommand"),(WCHAR*)WStr("ExecuteScalar"),g_callTargets_Sig_131,1,4,0,0,9,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.CommandExecuteScalarIntegration"),CallTargetKind::Default,1,15},
 {(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlCommand"),(WCHAR*)WStr("ExecuteScalarAsync"),g_callTargets_Sig_230,2,4,0,0,9,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.CommandExecuteScalarAsyncIntegration"),CallTargetKind::Default,1,15},
-{(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlDataReader"),(WCHAR*)WStr("Close"),g_callTargets_Sig_164,4,4,0,0,8,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderCloseNpgsqlIntegration"),CallTargetKind::Default,1,15},
+{(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlDataReader"),(WCHAR*)WStr("Close"),g_callTargets_Sig_164,4,4,0,0,8,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.Npgsql.ReaderCloseNpgsqlIntegration"),CallTargetKind::Default,4,15},
 {(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlDataReader"),(WCHAR*)WStr("GetString"),g_callTargets_Sig_139,2,4,0,0,8,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderGetStringIntegration"),CallTargetKind::Default,1,15},
 {(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlDataReader"),(WCHAR*)WStr("GetValue"),g_callTargets_Sig_133,2,4,0,0,8,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderGetStringIntegration"),CallTargetKind::Default,1,15},
 {(WCHAR*)WStr("Npgsql"),(WCHAR*)WStr("Npgsql.NpgsqlDataReader"),(WCHAR*)WStr("Read"),g_callTargets_Sig_094,1,4,0,0,8,65535,65535,assemblyName,(WCHAR*)WStr("Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.ReaderReadIntegration"),CallTargetKind::Default,1,15},