Code review feedback

diff --git a/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs b/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs
index 480a492..b8c7d9b 100644 (file)
--- a/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs
+++ b/src/coreclr/tests/src/tracing/eventpipe/eventsourceerror/eventsourceerror.cs
@@ -81,6 +81,7 @@ namespace Tracing.Tests.EventSourceError
             source.Dynamic.All += (TraceEvent traceEvent) =>
             {
                 if (traceEvent.ProviderName == "SentinelEventSource"
+                    || traceEvent.ProviderName == "Microsoft-Windows-DotNETRuntime"
                     || traceEvent.ProviderName == "Microsoft-Windows-DotNETRuntimeRundown"
                     || traceEvent.ProviderName == "Microsoft-DotNETCore-EventPipe")
                 {
@@ -90,7 +91,7 @@ namespace Tracing.Tests.EventSourceError
                 ++eventCount;
 
                 if (traceEvent.ProviderName == "IllegalTypesEventSource"
-                    && traceEvent.EventName == "WriteEventString"
+                    && traceEvent.EventName == "EventSourceMessage"
                     && traceEvent.FormattedMessage.StartsWith("ERROR: Exception in Command Processing for EventSource IllegalTypesEventSource", StringComparison.OrdinalIgnoreCase))
                 {
                     sawEvent = true;

diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs
index d2e33d5..34a7b86 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipeEventProvider.cs
@@ -8,8 +8,6 @@ namespace System.Diagnostics.Tracing
     {
         // The EventPipeProvider handle.
         private IntPtr m_provHandle = IntPtr.Zero;
-        private object m_createEventLock = new object();
-        private IntPtr m_writeEventStringeEventHandle = IntPtr.Zero;
 
         // Register an event provider.
         unsafe uint IEventProvider.EventRegister(
@@ -52,7 +50,7 @@ namespace System.Diagnostics.Tracing
             int userDataCount,
             EventProvider.EventData* userData)
         {
-            if (eventDescriptor.EventId != 0 && eventHandle != IntPtr.Zero)
+            if (eventHandle != IntPtr.Zero)
             {
                 if (userDataCount == 0)
                 {
@@ -71,30 +69,7 @@ namespace System.Diagnostics.Tracing
                 }
                 EventPipeInternal.WriteEventData(eventHandle, userData, (uint)userDataCount, activityId, relatedActivityId);
             }
-            else
-            {
-                if (m_writeEventStringeEventHandle == IntPtr.Zero)
-                {
-                    lock (m_createEventLock)
-                    {
-                        if (m_writeEventStringeEventHandle == IntPtr.Zero)
-                        {
-                            string eventName = "WriteEventString";
-                            EventParameterInfo paramInfo = default(EventParameterInfo);
-                            paramInfo.SetInfo("message", typeof(string));
-                            byte[]? metadata = EventPipeMetadataGenerator.Instance.GenerateMetadata(0, eventName, 0, (uint)EventLevel.LogAlways, 0, new EventParameterInfo[] { paramInfo });
-                            uint metadataLength = (metadata != null) ? (uint)metadata.Length : 0;
 
-                            fixed (byte* pMetadata = metadata)
-                            {
-                                m_writeEventStringeEventHandle = ((IEventProvider)this).DefineEventHandle(0, eventName, 0, 0, (uint)EventLevel.LogAlways, pMetadata, metadataLength);
-                            }
-                        }
-                    }
-                }
-
-                EventPipeInternal.WriteEventData(m_writeEventStringeEventHandle, userData, (uint)userDataCount, activityId, relatedActivityId);
-            }
             return EventProvider.WriteEventErrorCode.NoError;
         }
 

diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
index 7ffb21c..fd42fb6 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
@@ -2149,7 +2149,11 @@ namespace System.Diagnostics.Tracing
             }
         }
 
-        private unsafe void WriteEventString(EventLevel level, long keywords, string msgString)
+        // WriteEventString is used for logging an error message (or similar) to
+        // ETW and EventPipe providers. It is not a general purpose API, it will
+        // log the message with Level=LogAlways and Keywords=All to make sure whoever
+        // is listening gets the message.
+        private unsafe void WriteEventString(string msgString)
         {
 #if FEATURE_MANAGED_ETW || FEATURE_PERFTRACING
             bool allAreNull = true;
@@ -2164,6 +2168,8 @@ namespace System.Diagnostics.Tracing
                 return;
             }
 
+            EventLevel level = EventLevel.LogAlways;
+            long keywords = -1;
             const string EventName = "EventSourceMessage";
             if (SelfDescribingEvents)
             {
@@ -2183,7 +2189,7 @@ namespace System.Diagnostics.Tracing
                 if (m_rawManifest == null && m_outOfBandMessageCount == 1)
                 {
                     ManifestBuilder manifestBuilder = new ManifestBuilder(Name, Guid, Name, null, EventManifestOptions.None);
-                    manifestBuilder.StartEvent(EventName, new EventAttribute(0) { Level = EventLevel.LogAlways, Task = (EventTask)0xFFFE });
+                    manifestBuilder.StartEvent(EventName, new EventAttribute(0) { Level = level, Task = (EventTask)0xFFFE });
                     manifestBuilder.AddEventParameter(typeof(string), "message");
                     manifestBuilder.EndEvent();
                     SendManifest(manifestBuilder.CreateManifest());
@@ -2206,7 +2212,27 @@ namespace System.Diagnostics.Tracing
 #if FEATURE_PERFTRACING
                     if (m_eventPipeProvider != null)
                     {
-                        m_eventPipeProvider.WriteEvent(ref descr, IntPtr.Zero, null, null, 1, (IntPtr)((void*)&data));
+                        if (m_writeEventStringEventHandle == IntPtr.Zero)
+                        {
+                            lock (m_createEventLock)
+                            {
+                                if (m_writeEventStringEventHandle == IntPtr.Zero)
+                                {
+                                    string eventName = "EventSourceMessage";
+                                    EventParameterInfo paramInfo = default(EventParameterInfo);
+                                    paramInfo.SetInfo("message", typeof(string));
+                                    byte[]? metadata = EventPipeMetadataGenerator.Instance.GenerateMetadata(0, eventName, keywords, (uint)level, 0, new EventParameterInfo[] { paramInfo });
+                                    uint metadataLength = (metadata != null) ? (uint)metadata.Length : 0;
+
+                                    fixed (byte* pMetadata = metadata)
+                                    {
+                                        m_writeEventStringEventHandle = m_eventPipeProvider.m_eventProvider.DefineEventHandle(0, eventName, keywords, 0, (uint)level, pMetadata, metadataLength);
+                                    }
+                                }
+                            }
+                        }
+
+                        m_eventPipeProvider.WriteEvent(ref descr, m_writeEventStringEventHandle, null, null, 1, (IntPtr)((void*)&data));
                     }
 #endif // FEATURE_PERFTRACING
                 }
@@ -3824,7 +3850,7 @@ namespace System.Diagnostics.Tracing
                     msg = "Reached message limit.   End of EventSource error messages.";
                 }
 
-                WriteEventString(EventLevel.LogAlways, -1, msg);
+                WriteEventString(msg);
                 WriteStringToAllListeners("EventSourceMessage", msg);
             }
             catch { }      // If we fail during last chance logging, well, we have to give up....
@@ -3881,6 +3907,8 @@ namespace System.Diagnostics.Tracing
         private volatile OverideEventProvider m_etwProvider = null!;   // This hooks up ETW commands to our 'OnEventCommand' callback
 #endif
 #if FEATURE_PERFTRACING
+        private object m_createEventLock = new object();
+        private IntPtr m_writeEventStringEventHandle = IntPtr.Zero;
         private volatile OverideEventProvider m_eventPipeProvider = null!;
 #endif
         private bool m_completelyInited;                // The EventSource constructor has returned without exception.