Reorganize code around DBM injection to make sure comments get inject…

…ed even if setting context fails (#6167) Currently, if the `set context_info` fails for some reason, there is no injection applied. We can make sure activating full mode doesn't cause any regression. Also moved the debug log before sending the query to the DB so that we can see what's happening in the debug logs if it fails. --------- Co-authored-by: Steven Bouwkamp <steven.bouwkamp@datadoghq.com>
DataDog · Oct 22, 2024 · c0e5e4d · c0e5e4d
1 parent 0c611c8
commit c0e5e4d
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AdoNet/DbScopeFactory.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AdoNet/DbScopeFactory.cs
@@ -100,15 +100,17 @@ internal static class DbScopeFactory
                     }
                     else
                     {
+                        var propagationComment = DatabaseMonitoringPropagator.PropagateDataViaComment(tracer.Settings.DbmPropagationMode, tracer.DefaultServiceName, tagsFromConnectionString.DbName, tagsFromConnectionString.OutHost, scope.Span, integrationId, out var traceParentInjectedInComment);
+                        if (!string.IsNullOrEmpty(propagationComment))
+                        {
+                            command.CommandText = $"{propagationComment} {commandText}";
+                        }
+
+                        // try context injection only after comment injection, so that if it fails, we still have service level propagation
                         var traceParentInjectedInContext = DatabaseMonitoringPropagator.PropagateDataViaContext(tracer.Settings.DbmPropagationMode, integrationId, command.Connection, scope.Span);
-                        var propagatedCommand = DatabaseMonitoringPropagator.PropagateDataViaComment(tracer.Settings.DbmPropagationMode, tracer.DefaultServiceName, tagsFromConnectionString.DbName, tagsFromConnectionString.OutHost, scope.Span, integrationId, out var traceParentInjectedInComment);
-                        if (!string.IsNullOrEmpty(propagatedCommand))
+                        if (traceParentInjectedInComment || traceParentInjectedInContext)
                         {
-                            command.CommandText = $"{propagatedCommand} {commandText}";
-                            if (traceParentInjectedInComment || traceParentInjectedInContext)
-                            {
-                                tags.DbmTraceInjected = "true";
-                            }
+                            tags.DbmTraceInjected = "true";
                         }
                     }
                 }

diff --git a/tracer/src/Datadog.Trace/DatabaseMonitoring/DatabaseMonitoringPropagator.cs b/tracer/src/Datadog.Trace/DatabaseMonitoring/DatabaseMonitoringPropagator.cs
@@ -5,6 +5,7 @@
 
 using System;
 using System.Data;
+using System.Threading;
 using Datadog.Trace.Configuration;
 using Datadog.Trace.Logging;
 using Datadog.Trace.Propagators;
@@ -32,6 +33,8 @@ internal static class DatabaseMonitoringPropagator
 
         private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(DatabaseMonitoringPropagator));
 
+        private static int _remainingErrorLogs = 100; // to prevent too many similar errors in the logs. We assume that after 100 logs, the incremental value of more logs is negligible.
+
         internal static string PropagateDataViaComment(DbmPropagationLevel propagationStyle, string configuredServiceName, string? dbName, string? outhost, Span span, IntegrationId integrationId, out bool traceParentInjected)
         {
             traceParentInjected = false;
@@ -133,12 +136,29 @@ internal static bool PropagateDataViaContext(DbmPropagationLevel propagationLeve
                 parameter.DbType = DbType.Binary;
                 injectionCommand.Parameters.Add(parameter);
 
-                injectionCommand.ExecuteNonQuery();
-
                 if (Log.IsEnabled(LogEventLevel.Debug))
                 {
                     // avoid building the string representation in the general case where debug is disabled
-                    Log.Debug("Span data for DBM propagated for {Integration} via context_info with value {ContextValue} (propagation level: {PropagationLevel}", integrationId, HexConverter.ToString(contextValue), propagationLevel);
+                    Log.Debug("Propagating span data for DBM for {Integration} via context_info with value {ContextValue} (propagation level: {PropagationLevel}", integrationId, HexConverter.ToString(contextValue), propagationLevel);
+                }
+
+                try
+                {
+                    injectionCommand.ExecuteNonQuery();
+                }
+                catch (Exception e)
+                {
+                    // stop logging the error after a while
+                    if (_remainingErrorLogs > 0)
+                    {
+                        var actualRemaining = Interlocked.Decrement(ref _remainingErrorLogs);
+                        if (actualRemaining >= 0)
+                        {
+                            Log.Error<string, int>(e, "Error setting context_info [{ContextValue}] for DB query, falling back to service only propagation mode. There won't be any link with APM traces. (will log this error {N} more time and then stop)", HexConverter.ToString(contextValue), actualRemaining);
+                        }
+                    }
+
+                    return false;
                 }
             }