Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix | Revert Event source changes on TryBeginExecuteEvent and WriteEndExecuteEvent to address the failure on other MS products. #1258

Merged
merged 8 commits into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6594,7 +6594,7 @@ public SqlCommand Clone()

private void WriteBeginExecuteEvent()
{
SqlClientEventSource.Log.TryBeginExecuteEvent(ObjectID, Connection?.ClientConnectionId, CommandText);
SqlClientEventSource.Log.TryBeginExecuteEvent(ObjectID, Connection?.DataSource, Connection?.Database, CommandText, Connection?.ClientConnectionId);
}

private void WriteEndExecuteEvent(bool success, int? sqlExceptionNumber, bool synchronous)
Expand All @@ -6615,7 +6615,7 @@ private void WriteEndExecuteEvent(bool success, int? sqlExceptionNumber, bool sy

int compositeState = successFlag | isSqlExceptionFlag | synchronousFlag;

SqlClientEventSource.Log.TryEndExecuteEvent(ObjectID, Connection?.ClientConnectionId, compositeState, sqlExceptionNumber.GetValueOrDefault());
SqlClientEventSource.Log.TryEndExecuteEvent(ObjectID, compositeState, sqlExceptionNumber.GetValueOrDefault(), Connection?.ClientConnectionId);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ protected override DbParameterCollection DbParameterCollection
return Parameters;
}
}

internal static void CancelIgnoreFailureCallback(object state)
{
SqlCommand command = (SqlCommand)state;
Expand Down Expand Up @@ -3027,9 +3027,9 @@ protected override Task<DbDataReader> ExecuteDbDataReaderAsync(CommandBehavior b
throw result.Exception.InnerException;
}
return result.Result;
},
CancellationToken.None,
TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.NotOnCanceled,
},
CancellationToken.None,
TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.NotOnCanceled,
TaskScheduler.Default
);
}
Expand Down Expand Up @@ -7626,7 +7626,7 @@ private SmiRequestExecutor SetUpSmiRequest(SqlInternalConnectionSmi innerConnect

private void WriteBeginExecuteEvent()
{
SqlClientEventSource.Log.TryBeginExecuteEvent(ObjectID, Connection?.ClientConnectionId, CommandText);
SqlClientEventSource.Log.TryBeginExecuteEvent(ObjectID, Connection?.DataSource, Connection?.Database, CommandText, Connection?.ClientConnectionId);
}

/// <summary>
Expand All @@ -7653,7 +7653,7 @@ private void WriteEndExecuteEvent(bool success, int? sqlExceptionNumber, bool sy

int compositeState = successFlag | isSqlExceptionFlag | synchronousFlag;

SqlClientEventSource.Log.TryEndExecuteEvent(ObjectID, Connection?.ClientConnectionId, compositeState, sqlExceptionNumber.GetValueOrDefault());
SqlClientEventSource.Log.TryEndExecuteEvent(ObjectID, compositeState, sqlExceptionNumber.GetValueOrDefault(), Connection?.ClientConnectionId);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ internal virtual void ReclaimedConnectionRequest() { /*no-op*/ }
#endregion
}

// Any changes to event writers might be considered as a breaking change.
// Other libraries such as OpenTelemetry and ApplicationInsight have based part of their code on BeginExecute and EndExecute arguments number.
[EventSource(Name = "Microsoft.Data.SqlClient.EventSource")]
internal partial class SqlClientEventSource : SqlClientEventSourceBase
{
Expand Down Expand Up @@ -510,22 +512,20 @@ internal void TryScopeLeaveEvent(long scopeId)

#region Execution Trace
[NonEvent]
internal void TryBeginExecuteEvent(int objectId, Guid? connectionId, string commandText, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
internal void TryBeginExecuteEvent(int objectId, string dataSource, string database, string commandText, Guid? connectionId, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
{
if (Log.IsExecutionTraceEnabled())
{
BeginExecute(GetFormattedMessage(SqlCommand_ClassName, memberName, EventType.INFO,
string.Format("Object Id {0}, Client Connection Id {1}, Command Text {2}", objectId, connectionId, commandText)));
BeginExecute(objectId, dataSource, database, commandText, GetFormattedMessage(SqlCommand_ClassName, memberName, EventType.INFO, $"Object Id {objectId}, Client connection Id {connectionId}, Command Text {commandText}"));
}
}

[NonEvent]
internal void TryEndExecuteEvent(int objectId, Guid? connectionId, int compositeState, int sqlExceptionNumber, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
internal void TryEndExecuteEvent(int objectId, int compositeState, int sqlExceptionNumber, Guid? connectionId, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
{
if (Log.IsExecutionTraceEnabled())
{
EndExecute(GetFormattedMessage(SqlCommand_ClassName, memberName, EventType.INFO,
string.Format("Object Id {0}, Client Connection Id {1}, Composite State {2}, Sql Exception Number {3}", objectId, connectionId, compositeState, sqlExceptionNumber)));
EndExecute(objectId, compositeState, sqlExceptionNumber, GetFormattedMessage(SqlCommand_ClassName, memberName, EventType.INFO, $"Object Id {objectId}, Client Connection Id {connectionId}, Composite State {compositeState}, Sql Exception Number {sqlExceptionNumber}"));
}
}
#endregion
Expand Down Expand Up @@ -995,13 +995,22 @@ internal void TrySNIScopeLeaveEvent(long scopeId)
#endregion

#region Write Events
// Do not change the first 4 arguments in this Event writer as OpenTelemetry and ApplicationInsight are relating to the same format,
// unless you have checked with them and they are able to change their design. Additional items could be added at the end.
[Event(BeginExecuteEventId, Keywords = Keywords.ExecutionTrace, Task = Tasks.ExecuteCommand, Opcode = EventOpcode.Start)]
internal void BeginExecute(string message) =>
WriteEvent(BeginExecuteEventId, message);
internal void BeginExecute(int objectId, string dataSource, string database, string commandText, string message)
{
WriteEvent(BeginExecuteEventId, objectId, dataSource, database, commandText, message);
}

// Do not change the first 3 arguments in this Event writer as OpenTelemetry and ApplicationInsight are relating to the same format,
// unless you have checked with them and they are able to change their design. Additional items could be added at the end.
[Event(EndExecuteEventId, Keywords = Keywords.ExecutionTrace, Task = Tasks.ExecuteCommand, Opcode = EventOpcode.Stop)]
internal void EndExecute(string message) =>
WriteEvent(EndExecuteEventId, message);
internal void EndExecute(int objectId, int compositestate, int sqlExceptionNumber, string message)
{

JRahnama marked this conversation as resolved.
Show resolved Hide resolved
WriteEvent(EndExecuteEventId, objectId, compositestate, sqlExceptionNumber, message);
}

[Event(TraceEventId, Level = EventLevel.Informational, Keywords = Keywords.Trace)]
internal void Trace(string message) =>
Expand Down Expand Up @@ -1106,7 +1115,7 @@ internal static class EventType
public const string INFO = " | INFO | ";
public const string ERR = " | ERR | ";
}

internal readonly struct TrySNIEventScope : IDisposable
{
private readonly long _scopeId;
Expand Down