From c0e5e4d870b702e0003430a7b2afa5d133b9352c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Vandon?= Date: Tue, 22 Oct 2024 23:43:14 +0200 Subject: [PATCH] Reorganize code around DBM injection to make sure comments get injected 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 --- .../AdoNet/DbScopeFactory.cs | 16 +++++++----- .../DatabaseMonitoringPropagator.cs | 26 ++++++++++++++++--- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AdoNet/DbScopeFactory.cs b/tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AdoNet/DbScopeFactory.cs index 1319b55fb0a2..686ef8c4f549 100644 --- 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 index 2779eb662a10..b90f4532a06f 100644 --- 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(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; } }