From 967e6729660e825feac8da531c6899dc857cd776 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Thu, 9 Jan 2020 17:52:36 -0800 Subject: [PATCH 01/18] Move environment variable parsing logic to native code --- .../Eventing/EventPipeController.cs | 2 +- src/coreclr/src/inc/eventtracebase.h | 54 ++++++------- src/coreclr/src/vm/eventpipe.cpp | 75 ++++++++++++++++++- src/coreclr/src/vm/eventpipe.h | 3 + 4 files changed, 107 insertions(+), 27 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs index e38b6dbe32a679..1f9a23bcfd714c 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs @@ -48,7 +48,7 @@ internal static void Initialize() { // Enable tracing immediately. // It will be disabled automatically on shutdown. - EventPipe.Enable(BuildConfigFromEnvironment()); + // EventPipe.Enable(BuildConfigFromEnvironment()); } RuntimeEventSource.Initialize(); diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index d9fee257688685..72cf22ff78a86e 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -222,31 +222,7 @@ struct ProfilingScanContext; #include "etmdummy.h" #endif // FEATURE_EVENT_TRACE -#ifndef FEATURE_REDHAWK - -#include "corprof.h" - -// g_nClrInstanceId is defined in Utilcode\Util.cpp. The definition goes into Utilcode.lib. -// This enables both the VM and Utilcode to raise ETW events. -extern UINT32 g_nClrInstanceId; - -#define GetClrInstanceId() (static_cast(g_nClrInstanceId)) -#if defined(FEATURE_PAL) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) -#define KEYWORDZERO 0x0 - -/***************************************/ -/* Tracing levels supported by CLR ETW */ -/***************************************/ -#define MAX_TRACE_LEVEL 6 // Maximum Number of Trace Levels supported -#define TRACE_LEVEL_FATAL 1 // Abnormal exit or termination -#define TRACE_LEVEL_ERROR 2 // Severe errors that need logging -#define TRACE_LEVEL_WARNING 3 // Warnings such as allocation failure -#define TRACE_LEVEL_INFORMATION 4 // Includes non-error cases such as Entry-Exit -#define TRACE_LEVEL_VERBOSE 5 // Detailed traces from intermediate steps - -#define DEF_LTTNG_KEYWORD_ENABLED 1 -#include "clrproviders.h" -#include "clrconfig.h" +#ifdef FEATURE_EVENT_TRACE class XplatEventLoggerConfiguration { @@ -369,6 +345,34 @@ class XplatEventLoggerConfiguration UINT _level; bool _isValid; }; +#endif // FEATURE_EVENT_TRACE + + +#ifndef FEATURE_REDHAWK + +#include "corprof.h" + +// g_nClrInstanceId is defined in Utilcode\Util.cpp. The definition goes into Utilcode.lib. +// This enables both the VM and Utilcode to raise ETW events. +extern UINT32 g_nClrInstanceId; + +#define GetClrInstanceId() (static_cast(g_nClrInstanceId)) +#if defined(FEATURE_PAL) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) +#define KEYWORDZERO 0x0 + +/***************************************/ +/* Tracing levels supported by CLR ETW */ +/***************************************/ +#define MAX_TRACE_LEVEL 6 // Maximum Number of Trace Levels supported +#define TRACE_LEVEL_FATAL 1 // Abnormal exit or termination +#define TRACE_LEVEL_ERROR 2 // Severe errors that need logging +#define TRACE_LEVEL_WARNING 3 // Warnings such as allocation failure +#define TRACE_LEVEL_INFORMATION 4 // Includes non-error cases such as Entry-Exit +#define TRACE_LEVEL_VERBOSE 5 // Detailed traces from intermediate steps + +#define DEF_LTTNG_KEYWORD_ENABLED 1 +#include "clrproviders.h" +#include "clrconfig.h" class XplatEventLoggerController { diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 4712aff6a6c10f..5c9069dde93738 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -18,6 +18,7 @@ #include "eventpipesession.h" #include "eventpipejsonfile.h" #include "eventtracebase.h" +#include "eventtracebase.h" #include "sampleprofiler.h" #include "win32threadpool.h" #include "ceemain.h" @@ -94,12 +95,84 @@ void EventPipe::Initialize() #endif } - { CrstHolder _crst(GetLock()); if (tracingInitialized) s_state = EventPipeState::Initialized; } + + EnableViaEnvironmentVariables(); +} + +// +// If EventPipe environment variables are specified, parse them and start a session +// +void EventPipe::EnableViaEnvironmentVariables() +{ + STANDARD_VM_CONTRACT; + if (CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EnableEventPipe) != 0) + { + LPWSTR eventpipeConfig = NULL; + CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeConfig, &eventpipeConfig); + auto configuration = XplatEventLoggerConfiguration(); + auto configToParse = eventpipeConfig; + auto t_configToParse = eventpipeConfig; + + // TODO: The behavior should be the same as existing code - enable with default provider configuration + if (configToParse == nullptr || *configToParse == L'\0') + { + return; + } + + // Count how many providers there are to parse + int cnt = 0; + static WCHAR comma = W(','); + while(t_configToParse != nullptr) + { + cnt += 1; + auto end = wcschr(configToParse, comma); + if (end == nullptr) + { + break; + } + configToParse = end + 1; + } + + EventPipeProviderConfiguration* pProviders = new EventPipeProviderConfiguration[cnt]; + int i = 0; + + while (configToParse != nullptr) + { + auto end = wcschr(configToParse, comma); + configuration.Parse(configToParse); + + pProviders[i++] = EventPipeProviderConfiguration( + configuration.GetProviderName(), + configuration.GetEnabledKeywordsMask(), + configuration.GetLevel(), + nullptr + // TODO: Add arguments here + ); + + if (end == nullptr) + { + break; + } + configToParse = end + 1; + } + + UINT64 sessionID = EventPipe::Enable( + W("mytrace.nettrace"), + 128, + pProviders, + cnt, + EventPipeSessionType::File, + EventPipeSerializationFormat::NetTraceV4, + true, + nullptr + ); + EventPipe::StartStreaming(sessionID); + } } void EventPipe::Shutdown() diff --git a/src/coreclr/src/vm/eventpipe.h b/src/coreclr/src/vm/eventpipe.h index f6a60a97846473..7198724a42c73a 100644 --- a/src/coreclr/src/vm/eventpipe.h +++ b/src/coreclr/src/vm/eventpipe.h @@ -47,6 +47,9 @@ class EventPipe // Initialize the event pipe. static void Initialize(); + // Initialize environment variable based session + static void EnableViaEnvironmentVariables(); + // Shutdown the event pipe. static void Shutdown(); From 7b5e97f57e4e136d1f3e0e492b55cb8baf80cd3b Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 15 Jan 2020 08:47:14 -0800 Subject: [PATCH 02/18] Fix multiple provider issue --- src/coreclr/src/vm/eventpipe.cpp | 26 +++++++++++++++++--------- src/coreclr/src/vm/eventpipe.h | 1 + 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 5c9069dde93738..71abf95d2916fc 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -130,12 +130,12 @@ void EventPipe::EnableViaEnvironmentVariables() while(t_configToParse != nullptr) { cnt += 1; - auto end = wcschr(configToParse, comma); + auto end = wcschr(t_configToParse, comma); if (end == nullptr) { break; } - configToParse = end + 1; + t_configToParse = end + 1; } EventPipeProviderConfiguration* pProviders = new EventPipeProviderConfiguration[cnt]; @@ -146,13 +146,21 @@ void EventPipe::EnableViaEnvironmentVariables() auto end = wcschr(configToParse, comma); configuration.Parse(configToParse); - pProviders[i++] = EventPipeProviderConfiguration( - configuration.GetProviderName(), - configuration.GetEnabledKeywordsMask(), - configuration.GetLevel(), - nullptr - // TODO: Add arguments here - ); + // SampleProfiler can't be enabled on startup. + if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0) + { + cnt -= 1; + } + else + { + pProviders[i++] = EventPipeProviderConfiguration( + configuration.GetProviderName(), + configuration.GetEnabledKeywordsMask(), + configuration.GetLevel(), + nullptr + // TODO: Add arguments here + ); + } if (end == nullptr) { diff --git a/src/coreclr/src/vm/eventpipe.h b/src/coreclr/src/vm/eventpipe.h index 7198724a42c73a..85d728f47bb309 100644 --- a/src/coreclr/src/vm/eventpipe.h +++ b/src/coreclr/src/vm/eventpipe.h @@ -43,6 +43,7 @@ class EventPipe public: static const uint32_t MaxNumberOfSessions = 64; + static const bool EnableSampleProfilerOnStartup = false; // Initialize the event pipe. static void Initialize(); From d2a787b2a8d31c13992b622353b38ae4ceffcabb Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Fri, 14 Feb 2020 13:48:15 -0800 Subject: [PATCH 03/18] Use env vars to set up eventpipe config --- src/coreclr/src/inc/eventtracebase.h | 26 +------------------------- src/coreclr/src/vm/eventpipe.cpp | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index aa73d11bb43d02..fc9fb1853dc9a1 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -222,31 +222,7 @@ struct ProfilingScanContext; #include "etmdummy.h" #endif // FEATURE_EVENT_TRACE -#ifndef FEATURE_REDHAWK - -#include "corprof.h" - -// g_nClrInstanceId is defined in Utilcode\Util.cpp. The definition goes into Utilcode.lib. -// This enables both the VM and Utilcode to raise ETW events. -extern UINT32 g_nClrInstanceId; - -#define GetClrInstanceId() (static_cast(g_nClrInstanceId)) -#if defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) -#define KEYWORDZERO 0x0 - -/***************************************/ -/* Tracing levels supported by CLR ETW */ -/***************************************/ -#define MAX_TRACE_LEVEL 6 // Maximum Number of Trace Levels supported -#define TRACE_LEVEL_FATAL 1 // Abnormal exit or termination -#define TRACE_LEVEL_ERROR 2 // Severe errors that need logging -#define TRACE_LEVEL_WARNING 3 // Warnings such as allocation failure -#define TRACE_LEVEL_INFORMATION 4 // Includes non-error cases such as Entry-Exit -#define TRACE_LEVEL_VERBOSE 5 // Detailed traces from intermediate steps - -#define DEF_LTTNG_KEYWORD_ENABLED 1 -#include "clrproviders.h" -#include "clrconfig.h" +#ifdef FEATURE_EVENT_TRACE class XplatEventLoggerConfiguration { diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index b0fb62dfc74610..3be43d7caf0cc4 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -22,6 +22,7 @@ #include "sampleprofiler.h" #include "win32threadpool.h" #include "ceemain.h" +#include "configuration.h" #ifdef TARGET_UNIX #include "pal.h" @@ -114,9 +115,15 @@ void EventPipe::EnableViaEnvironmentVariables() { LPWSTR eventpipeConfig = NULL; CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeConfig, &eventpipeConfig); + LPCWSTR eventpipeOutputPath = Configuration::GetKnobStringValue(W("EventPipeOutputPath")); + uint32_t eventpipeCircularBufferMB = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeCircularMB); + + if (eventpipeOutputPath == NULL) + { + eventpipeOutputPath = W("trace.nettrace"); + } auto configuration = XplatEventLoggerConfiguration(); auto configToParse = eventpipeConfig; - auto t_configToParse = eventpipeConfig; // TODO: The behavior should be the same as existing code - enable with default provider configuration if (configToParse == nullptr || *configToParse == L'\0') @@ -127,17 +134,18 @@ void EventPipe::EnableViaEnvironmentVariables() // Count how many providers there are to parse int cnt = 0; static WCHAR comma = W(','); - while(t_configToParse != nullptr) + while(configToParse != nullptr) { cnt += 1; - auto end = wcschr(t_configToParse, comma); + auto end = wcschr(configToParse, comma); if (end == nullptr) { break; } - t_configToParse = end + 1; + configToParse = end + 1; } + configToParse = eventpipeConfig; EventPipeProviderConfiguration* pProviders = new EventPipeProviderConfiguration[cnt]; int i = 0; @@ -170,8 +178,8 @@ void EventPipe::EnableViaEnvironmentVariables() } UINT64 sessionID = EventPipe::Enable( - W("mytrace.nettrace"), - 128, + eventpipeOutputPath, + eventpipeCircularBufferMB, pProviders, cnt, EventPipeSessionType::File, From 9d2f0b8f7291447eed02f68c258aca49cb9eff62 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Fri, 14 Feb 2020 17:24:53 -0800 Subject: [PATCH 04/18] Add default provider configuration if eventpipeconfig is not set --- src/coreclr/src/vm/eventpipe.cpp | 68 ++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 3be43d7caf0cc4..8799e92125b7df 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -124,11 +124,12 @@ void EventPipe::EnableViaEnvironmentVariables() } auto configuration = XplatEventLoggerConfiguration(); auto configToParse = eventpipeConfig; + bool enableDefaultConfig = false; // TODO: The behavior should be the same as existing code - enable with default provider configuration if (configToParse == nullptr || *configToParse == L'\0') { - return; + enableDefaultConfig = true; } // Count how many providers there are to parse @@ -145,36 +146,51 @@ void EventPipe::EnableViaEnvironmentVariables() configToParse = end + 1; } - configToParse = eventpipeConfig; - EventPipeProviderConfiguration* pProviders = new EventPipeProviderConfiguration[cnt]; - int i = 0; + // Create EventPipeProviderConfiguration and start tracing. + EventPipeProviderConfiguration* pProviders = nullptr; - while (configToParse != nullptr) + // If COMPlus_EnableEventPipe is set to 1 but no configuration was specified, enable EventPipe session + // with the default provider configurations. + if (enableDefaultConfig) { - auto end = wcschr(configToParse, comma); - configuration.Parse(configToParse); - - // SampleProfiler can't be enabled on startup. - if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0) - { - cnt -= 1; - } - else + // TODO: Enable SampleProfiler once we can mutate a EventPipe session post-creation. + pProviders = new EventPipeProviderConfiguration[2]; + pProviders[0] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntime"), 0x4c14fccbd, 5, nullptr); + pProviders[1] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntimePrivate"), 0x4002000b, 5, nullptr); + cnt = 2; + } + else + { + configToParse = eventpipeConfig; + pProviders = new EventPipeProviderConfiguration[cnt]; + int i = 0; + while (configToParse != nullptr) { - pProviders[i++] = EventPipeProviderConfiguration( - configuration.GetProviderName(), - configuration.GetEnabledKeywordsMask(), - configuration.GetLevel(), - nullptr - // TODO: Add arguments here - ); - } + auto end = wcschr(configToParse, comma); + configuration.Parse(configToParse); - if (end == nullptr) - { - break; + // SampleProfiler can't be enabled on startup yet. + if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0) + { + cnt -= 1; + } + else + { + pProviders[i++] = EventPipeProviderConfiguration( + configuration.GetProviderName(), + configuration.GetEnabledKeywordsMask(), + configuration.GetLevel(), + nullptr + // TODO: Add arguments here + ); + } + + if (end == nullptr) + { + break; + } + configToParse = end + 1; } - configToParse = end + 1; } UINT64 sessionID = EventPipe::Enable( From f2a9138d096b7e036b28aff3bf88b23102713848 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 18 Feb 2020 14:17:46 -0800 Subject: [PATCH 05/18] Remove EventPipeController --- .../System.Private.CoreLib.csproj | 1 - .../Eventing/EventPipeController.cs | 221 ------------------ .../src/System/StartupHookProvider.cs | 4 +- 3 files changed, 2 insertions(+), 224 deletions(-) delete mode 100644 src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs diff --git a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj index 7bb83729b44d8c..028e62a3e321a3 100644 --- a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -153,7 +153,6 @@ - diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs deleted file mode 100644 index 1f9a23bcfd714c..00000000000000 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs +++ /dev/null @@ -1,221 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. -#if FEATURE_PERFTRACING -using System.IO; -using System.Reflection; -using System.Runtime.Versioning; - -namespace System.Diagnostics.Tracing -{ - /// - /// Simple out-of-process listener for controlling EventPipe. - /// The following environment variables are used to configure EventPipe: - /// - COMPlus_EnableEventPipe=1 : Enable EventPipe immediately for the life of the process. - /// - COMPlus_EventPipeConfig : Provides the configuration in xperf string form for which providers/keywords/levels to be enabled. - /// If not specified, the default configuration is used. - /// - COMPlus_EventPipeOutputFile : The full path to the netperf file to be written. - /// - COMPlus_EventPipeCircularMB : The size in megabytes of the circular buffer. - /// - internal static class EventPipeController - { - // Miscellaneous constants. - private const string DefaultAppName = "app"; - private const string NetPerfFileExtension = ".netperf"; - private const string NetTraceFileExtension = ".nettrace"; - private const uint DefaultCircularBufferMB = 256; // MB (PerfView and dotnet-trace default) - private const char ProviderConfigDelimiter = ','; - private const char ConfigComponentDelimiter = ':'; - - // The default set of providers/keywords/levels. Used if an alternative configuration is not specified. - private static EventPipeProviderConfiguration[] DefaultProviderConfiguration => new EventPipeProviderConfiguration[] - { - new EventPipeProviderConfiguration("Microsoft-Windows-DotNETRuntime", 0x4c14fccbd, 5, null), - new EventPipeProviderConfiguration("Microsoft-Windows-DotNETRuntimePrivate", 0x4002000b, 5, null), - new EventPipeProviderConfiguration("Microsoft-DotNETCore-SampleProfiler", 0x0, 5, null), - }; - - private static bool IsControllerInitialized { get; set; } = false; - - internal static void Initialize() - { - // Don't allow failures to propagate upstream. Ensure program correctness without tracing. - try - { - if (!IsControllerInitialized) - { - if (Config_EnableEventPipe > 0) - { - // Enable tracing immediately. - // It will be disabled automatically on shutdown. - // EventPipe.Enable(BuildConfigFromEnvironment()); - } - - RuntimeEventSource.Initialize(); - - IsControllerInitialized = true; - } - } - catch { } - } - - private static EventPipeConfiguration BuildConfigFromEnvironment() - { - // Build the full path to the trace file. - string traceFileName = BuildTraceFileName(); - string outputFilePath = Path.Combine(Config_EventPipeOutputPath, traceFileName); - - // Create a new configuration object. - EventPipeConfiguration config = new EventPipeConfiguration( - outputFilePath, - (Config_NetTraceFormat != 0) ? EventPipeSerializationFormat.NetTrace : EventPipeSerializationFormat.NetPerf, - Config_EventPipeCircularMB); - - // Get the configuration. - string? strConfig = Config_EventPipeConfig; - if (!string.IsNullOrEmpty(strConfig)) - { - // If the configuration is specified, parse it and save it to the config object. - SetProviderConfiguration(strConfig, config); - } - else - { - // Specify the default configuration. - config.EnableProviderRange(DefaultProviderConfiguration); - } - - return config; - } - - private static string BuildTraceFileName() - { - return GetAppName() + "." + Interop.GetCurrentProcessId().ToString() + - ((Config_NetTraceFormat != 0) ? NetTraceFileExtension : NetPerfFileExtension); - } - - private static string GetAppName() - { - string? appName = null; - Assembly? entryAssembly = Assembly.GetEntryAssembly(); - if (entryAssembly != null) - { - AssemblyName? assemblyName = entryAssembly.GetName(); - if (assemblyName != null) - { - appName = assemblyName.Name; - } - } - - if (string.IsNullOrEmpty(appName)) - { - appName = DefaultAppName; - } - - return appName; - } - - private static void SetProviderConfiguration(string strConfig, EventPipeConfiguration config) - { - if (string.IsNullOrEmpty(strConfig)) - { - throw new ArgumentNullException(nameof(strConfig)); - } - - // Provider format: "(GUID|KnownProviderName)[:Flags[:Level][:KeyValueArgs]]" - // where KeyValueArgs are of the form: "[key1=value1][;key2=value2]" - // `strConfig` must be of the form "Provider[,Provider]" - string[] providers = strConfig.Split( - ProviderConfigDelimiter, - StringSplitOptions.RemoveEmptyEntries); // Remove "empty" providers. - foreach (string provider in providers) - { - // Split expecting a maximum of four tokens. - string[] components = provider.Split( - ConfigComponentDelimiter, - 4, // if there is ':' in the parameters then anything after it will not be ignored. - StringSplitOptions.None); // Keep empty tokens - - string? providerName = components.Length > 0 ? components[0] : null; - if (string.IsNullOrEmpty(providerName)) - continue; // No provider name specified. - - ulong keywords = ulong.MaxValue; - if (components.Length > 1) - { - // We use a try/catch block here because ulong.TryParse won't accept 0x at the beginning - // of a hex string. Thus, we either need to conditionally strip it or handle the exception. - // Given that this is not a perf-critical path, catching the exception is the simpler code. - try - { - keywords = Convert.ToUInt64(components[1], 16); - } - catch - { - } - } - - uint level = 5; // Verbose - if (components.Length > 2) - { - uint.TryParse(components[2], out level); - } - - string? filterData = components.Length > 3 ? components[3] : null; - - config.EnableProviderWithFilter(providerName, keywords, level, filterData); - } - } - - /// - /// Returns -1 if the EnableEventPipe environment variable is not set at all (or is illegal) - /// - private static int Config_EnableEventPipe - { - get - { - string? stringValue = CompatibilitySwitch.GetValueInternal("EnableEventPipe"); - if ((stringValue == null) || (!int.TryParse(stringValue, out int value))) - { - value = -1; // Indicates no value (or is illegal) - } - - return value; - } - } - - private static int Config_NetTraceFormat - { - get - { - string? stringValue = CompatibilitySwitch.GetValueInternal("EventPipeNetTraceFormat"); - if ((stringValue == null) || (!int.TryParse(stringValue, out int value))) - { - value = -1; // Indicates no value (or is illegal) - } - - return value; - } - } - - private static string? Config_EventPipeConfig => CompatibilitySwitch.GetValueInternal("EventPipeConfig"); - - private static uint Config_EventPipeCircularMB - { - get - { - string? stringValue = CompatibilitySwitch.GetValueInternal("EventPipeCircularMB"); - if ((stringValue == null) || (!uint.TryParse(stringValue, out uint value))) - { - value = DefaultCircularBufferMB; - } - - return value; - } - } - - private static string Config_EventPipeOutputPath => - CompatibilitySwitch.GetValueInternal("EventPipeOutputPath") ?? "."; - } -} - -#endif // FEATURE_PERFTRACING diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs b/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs index 220c7bac4bb209..c2739308bfe84c 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs @@ -25,8 +25,8 @@ private struct StartupHookNameOrPath // containing a startup hook, and call each hook in turn. private static void ProcessStartupHooks() { - // Initialize tracing before any user code can be called. - System.Diagnostics.Tracing.EventPipeController.Initialize(); + // Initialize System.Runtime EventSource + System.Diagnostics.Tracing.RuntimeEventSource.Initialize(); string? startupHooksVariable = (string?)AppContext.GetData("STARTUP_HOOKS"); if (startupHooksVariable == null) From 73dbabdf73a950dbdf4638ff9296282fc8bdd1f4 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 18 Feb 2020 14:45:17 -0800 Subject: [PATCH 06/18] Add Argument parsing logic to XplatEventLoggerConfiguration --- src/coreclr/src/inc/eventtracebase.h | 23 +++++++++++++++++++++++ src/coreclr/src/vm/eventpipe.cpp | 3 +-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index fc9fb1853dc9a1..73087edf96ffec 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -258,6 +258,10 @@ class XplatEventLoggerConfiguration auto levelComponent = GetNextComponentString(keywordsComponent.End + 1); _level = ParseLevel(levelComponent); + + auto argumentComponent = GetNextComponentString(levelComponent.End + 1); + _argument = ParseArgument(argumentComponent); + _isValid = true; } @@ -281,6 +285,11 @@ class XplatEventLoggerConfiguration return _level; } + LPCWSTR GetArgument() const + { + return _argument; + } + private: struct ComponentSpan { @@ -340,9 +349,23 @@ class XplatEventLoggerConfiguration return level; } + LPCWSTR ParseArgument(ComponentSpan const & component) const + { + auto argument = (WCHAR*)nullptr; + if ((component.End - component.Start) != 0) + { + auto const length = component.End - component.Start; + argument = new WCHAR[length + 1]; + memset(argument, '\0', (length + 1) * sizeof(WCHAR)); + wcsncpy(argument, component.Start, length); + } + return argument; + } + LPCWSTR _provider; ULONGLONG _enabledKeywords; UINT _level; + LPCWSTR _argument; bool _isValid; }; #endif // FEATURE_EVENT_TRACE diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 8799e92125b7df..516af3246636f6 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -180,8 +180,7 @@ void EventPipe::EnableViaEnvironmentVariables() configuration.GetProviderName(), configuration.GetEnabledKeywordsMask(), configuration.GetLevel(), - nullptr - // TODO: Add arguments here + configuration.GetArgument() ); } From 6098b6cb121cba0d8871c96356ec00388ead7265 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 18 Feb 2020 15:42:53 -0800 Subject: [PATCH 07/18] cleanup --- src/coreclr/src/vm/eventpipe.cpp | 5 ++--- src/coreclr/src/vm/eventpipe.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 516af3246636f6..a18ea1a4449b49 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -126,7 +126,6 @@ void EventPipe::EnableViaEnvironmentVariables() auto configToParse = eventpipeConfig; bool enableDefaultConfig = false; - // TODO: The behavior should be the same as existing code - enable with default provider configuration if (configToParse == nullptr || *configToParse == L'\0') { enableDefaultConfig = true; @@ -154,10 +153,10 @@ void EventPipe::EnableViaEnvironmentVariables() if (enableDefaultConfig) { // TODO: Enable SampleProfiler once we can mutate a EventPipe session post-creation. - pProviders = new EventPipeProviderConfiguration[2]; + cnt = 2; + pProviders = new EventPipeProviderConfiguration[cnt]; pProviders[0] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntime"), 0x4c14fccbd, 5, nullptr); pProviders[1] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntimePrivate"), 0x4002000b, 5, nullptr); - cnt = 2; } else { diff --git a/src/coreclr/src/vm/eventpipe.h b/src/coreclr/src/vm/eventpipe.h index 3a16ccbfef8de4..a638ca118cd065 100644 --- a/src/coreclr/src/vm/eventpipe.h +++ b/src/coreclr/src/vm/eventpipe.h @@ -43,7 +43,6 @@ class EventPipe public: static const uint32_t MaxNumberOfSessions = 64; - static const bool EnableSampleProfilerOnStartup = false; // Initialize the event pipe. static void Initialize(); From d845bb00f119c5b0b891935ba7b7011be8be1b15 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 18 Feb 2020 17:20:57 -0800 Subject: [PATCH 08/18] Fix Linux build --- src/coreclr/src/inc/eventtracebase.h | 301 ++++++++++++++------------- 1 file changed, 151 insertions(+), 150 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index 73087edf96ffec..0ac07c036acbda 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -65,6 +65,154 @@ enum EtwThreadFlags kEtwThreadFlagThreadPoolWorker = 0x00000004, }; +#if defined(FEATURE_PERFTRACING) +class XplatEventLoggerConfiguration +{ +public: + XplatEventLoggerConfiguration() = default; + + XplatEventLoggerConfiguration(XplatEventLoggerConfiguration const & other) = delete; + XplatEventLoggerConfiguration(XplatEventLoggerConfiguration && other) + { + _provider = std::move(other._provider); + _isValid = other._isValid; + _enabledKeywords = other._enabledKeywords; + _level = other._level; + } + + ~XplatEventLoggerConfiguration() + { + _provider = nullptr; + } + + void Parse(LPWSTR configString) + { + auto providerComponent = GetNextComponentString(configString); + _provider = ParseProviderName(providerComponent); + if (_provider == nullptr) + { + _isValid = false; + return; + } + + auto keywordsComponent = GetNextComponentString(providerComponent.End + 1); + _enabledKeywords = ParseEnabledKeywordsMask(keywordsComponent); + + auto levelComponent = GetNextComponentString(keywordsComponent.End + 1); + _level = ParseLevel(levelComponent); + + auto argumentComponent = GetNextComponentString(levelComponent.End + 1); + _argument = ParseArgument(argumentComponent); + + _isValid = true; + } + + bool IsValid() const + { + return _isValid; + } + + LPCWSTR GetProviderName() const + { + return _provider; + } + + ULONGLONG GetEnabledKeywordsMask() const + { + return _enabledKeywords; + } + + UINT GetLevel() const + { + return _level; + } + + LPCWSTR GetArgument() const + { + return _argument; + } + +private: + struct ComponentSpan + { + public: + ComponentSpan(LPCWSTR start, LPCWSTR end) + : Start(start), End(end) + { + } + + LPCWSTR Start; + LPCWSTR End; + }; + + ComponentSpan GetNextComponentString(LPCWSTR start) const + { + static WCHAR ComponentDelimiter = W(':'); + + auto end = wcschr(start, ComponentDelimiter); + if (end == nullptr) + { + end = start + wcslen(start); + } + + return ComponentSpan(start, end); + } + + LPCWSTR ParseProviderName(ComponentSpan const & component) const + { + auto providerName = (WCHAR*)nullptr; + if ((component.End - component.Start) != 0) + { + auto const length = component.End - component.Start; + providerName = new WCHAR[length + 1]; + memset(providerName, '\0', (length + 1) * sizeof(WCHAR)); + wcsncpy(providerName, component.Start, length); + } + return providerName; + } + + ULONGLONG ParseEnabledKeywordsMask(ComponentSpan const & component) const + { + auto enabledKeywordsMask = (ULONGLONG)(-1); + if ((component.End - component.Start) != 0) + { + enabledKeywordsMask = _wcstoui64(component.Start, nullptr, 16); + } + return enabledKeywordsMask; + } + + UINT ParseLevel(ComponentSpan const & component) const + { + auto level = 5; + if ((component.End - component.Start) != 0) + { + level = _wtoi(component.Start); + } + return level; + } + + LPCWSTR ParseArgument(ComponentSpan const & component) const + { + auto argument = (WCHAR*)nullptr; + if ((component.End - component.Start) != 0) + { + auto const length = component.End - component.Start; + argument = new WCHAR[length + 1]; + wcsncpy(argument, component.Start, length); + argument[length] = '\0'; + } + return argument; + } + + LPCWSTR _provider; + ULONGLONG _enabledKeywords; + UINT _level; + LPCWSTR _argument; + bool _isValid; +}; +#endif // FEATURE_PERFTRACING + + #ifndef FEATURE_REDHAWK #if defined(FEATURE_EVENT_TRACE) @@ -222,154 +370,6 @@ struct ProfilingScanContext; #include "etmdummy.h" #endif // FEATURE_EVENT_TRACE -#ifdef FEATURE_EVENT_TRACE - -class XplatEventLoggerConfiguration -{ -public: - XplatEventLoggerConfiguration() = default; - - XplatEventLoggerConfiguration(XplatEventLoggerConfiguration const & other) = delete; - XplatEventLoggerConfiguration(XplatEventLoggerConfiguration && other) - { - _provider = std::move(other._provider); - _isValid = other._isValid; - _enabledKeywords = other._enabledKeywords; - _level = other._level; - } - - ~XplatEventLoggerConfiguration() - { - _provider = nullptr; - } - - void Parse(LPWSTR configString) - { - auto providerComponent = GetNextComponentString(configString); - _provider = ParseProviderName(providerComponent); - if (_provider == nullptr) - { - _isValid = false; - return; - } - - auto keywordsComponent = GetNextComponentString(providerComponent.End + 1); - _enabledKeywords = ParseEnabledKeywordsMask(keywordsComponent); - - auto levelComponent = GetNextComponentString(keywordsComponent.End + 1); - _level = ParseLevel(levelComponent); - - auto argumentComponent = GetNextComponentString(levelComponent.End + 1); - _argument = ParseArgument(argumentComponent); - - _isValid = true; - } - - bool IsValid() const - { - return _isValid; - } - - LPCWSTR GetProviderName() const - { - return _provider; - } - - ULONGLONG GetEnabledKeywordsMask() const - { - return _enabledKeywords; - } - - UINT GetLevel() const - { - return _level; - } - - LPCWSTR GetArgument() const - { - return _argument; - } - -private: - struct ComponentSpan - { - public: - ComponentSpan(LPCWSTR start, LPCWSTR end) - : Start(start), End(end) - { - } - - LPCWSTR Start; - LPCWSTR End; - }; - - ComponentSpan GetNextComponentString(LPCWSTR start) const - { - static WCHAR ComponentDelimiter = W(':'); - - auto end = wcschr(start, ComponentDelimiter); - if (end == nullptr) - { - end = start + wcslen(start); - } - - return ComponentSpan(start, end); - } - - LPCWSTR ParseProviderName(ComponentSpan const & component) const - { - auto providerName = (WCHAR*)nullptr; - if ((component.End - component.Start) != 0) - { - auto const length = component.End - component.Start; - providerName = new WCHAR[length + 1]; - memset(providerName, '\0', (length + 1) * sizeof(WCHAR)); - wcsncpy(providerName, component.Start, length); - } - return providerName; - } - - ULONGLONG ParseEnabledKeywordsMask(ComponentSpan const & component) const - { - auto enabledKeywordsMask = (ULONGLONG)(-1); - if ((component.End - component.Start) != 0) - { - enabledKeywordsMask = _wcstoui64(component.Start, nullptr, 16); - } - return enabledKeywordsMask; - } - - UINT ParseLevel(ComponentSpan const & component) const - { - auto level = TRACE_LEVEL_VERBOSE; - if ((component.End - component.Start) != 0) - { - level = _wtoi(component.Start); - } - return level; - } - - LPCWSTR ParseArgument(ComponentSpan const & component) const - { - auto argument = (WCHAR*)nullptr; - if ((component.End - component.Start) != 0) - { - auto const length = component.End - component.Start; - argument = new WCHAR[length + 1]; - memset(argument, '\0', (length + 1) * sizeof(WCHAR)); - wcsncpy(argument, component.Start, length); - } - return argument; - } - - LPCWSTR _provider; - ULONGLONG _enabledKeywords; - UINT _level; - LPCWSTR _argument; - bool _isValid; -}; -#endif // FEATURE_EVENT_TRACE - #ifndef FEATURE_REDHAWK @@ -380,7 +380,7 @@ class XplatEventLoggerConfiguration extern UINT32 g_nClrInstanceId; #define GetClrInstanceId() (static_cast(g_nClrInstanceId)) -#if defined(FEATURE_PAL) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) +#if defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) #define KEYWORDZERO 0x0 /***************************************/ @@ -397,6 +397,7 @@ extern UINT32 g_nClrInstanceId; #include "clrproviders.h" #include "clrconfig.h" + class XplatEventLoggerController { public: @@ -1513,4 +1514,4 @@ namespace ETW }; -#endif //_ETWTRACER_HXX_ +#endif //_ETWTRACER_HXX_ \ No newline at end of file From 6515b7ba3b12b914aa0696a178cb614b6e41dc2d Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 18 Feb 2020 17:32:16 -0800 Subject: [PATCH 09/18] Check provider configuration before enabling them --- src/coreclr/src/inc/eventtracebase.h | 2 +- src/coreclr/src/vm/eventpipe.cpp | 30 ++++++++++++++++++---------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index 0ac07c036acbda..dfae2ae40d2150 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -183,7 +183,7 @@ class XplatEventLoggerConfiguration UINT ParseLevel(ComponentSpan const & component) const { - auto level = 5; + auto level = 5; // Verbose if ((component.End - component.Start) != 0) { level = _wtoi(component.Start); diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index a18ea1a4449b49..e9e2a6b6de5a55 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -173,6 +173,10 @@ void EventPipe::EnableViaEnvironmentVariables() { cnt -= 1; } + else if (!configuration.IsValid()) // exclude invalid provider configurations + { + cnt -= 1; + } else { pProviders[i++] = EventPipeProviderConfiguration( @@ -191,17 +195,21 @@ void EventPipe::EnableViaEnvironmentVariables() } } - UINT64 sessionID = EventPipe::Enable( - eventpipeOutputPath, - eventpipeCircularBufferMB, - pProviders, - cnt, - EventPipeSessionType::File, - EventPipeSerializationFormat::NetTraceV4, - true, - nullptr - ); - EventPipe::StartStreaming(sessionID); + if (cnt != 0) + { + UINT64 sessionID = EventPipe::Enable( + eventpipeOutputPath, + eventpipeCircularBufferMB, + pProviders, + cnt, + EventPipeSessionType::File, + EventPipeSerializationFormat::NetTraceV4, + true, + nullptr + ); + EventPipe::StartStreaming(sessionID); + } + } } From 63a4a466d0cb535cd4fe2f558f6d9f374629f813 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 19 Feb 2020 13:51:22 -0800 Subject: [PATCH 10/18] wrap new code under ifdef, put EventPipeController back --- src/coreclr/clrfeatures.cmake | 4 + .../System.Private.CoreLib.csproj | 1 + .../Eventing/EventPipeController.cs | 221 ++++++++++++++++++ .../src/System/StartupHookProvider.cs | 4 +- src/coreclr/src/vm/eventpipe.cpp | 60 +++-- 5 files changed, 254 insertions(+), 36 deletions(-) create mode 100644 src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs diff --git a/src/coreclr/clrfeatures.cmake b/src/coreclr/clrfeatures.cmake index 078b4e73ac8044..35d8109823ae6b 100644 --- a/src/coreclr/clrfeatures.cmake +++ b/src/coreclr/clrfeatures.cmake @@ -27,3 +27,7 @@ endif(NOT DEFINED FEATURE_STANDALONE_GC) if(NOT DEFINED FEATURE_AUTO_TRACE) set(FEATURE_AUTO_TRACE 0) endif(NOT DEFINED FEATURE_AUTO_TRACE) + +if(NOT DEFINED FEATURE_AUTO_TRACE) + set(FEATURE_EVENTPIPE_STARTUP 0) +endif(NOT DEFINED FEATURE_AUTO_TRACE) diff --git a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj index 028e62a3e321a3..7bb83729b44d8c 100644 --- a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -153,6 +153,7 @@ + diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs new file mode 100644 index 00000000000000..e38b6dbe32a679 --- /dev/null +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipeController.cs @@ -0,0 +1,221 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +#if FEATURE_PERFTRACING +using System.IO; +using System.Reflection; +using System.Runtime.Versioning; + +namespace System.Diagnostics.Tracing +{ + /// + /// Simple out-of-process listener for controlling EventPipe. + /// The following environment variables are used to configure EventPipe: + /// - COMPlus_EnableEventPipe=1 : Enable EventPipe immediately for the life of the process. + /// - COMPlus_EventPipeConfig : Provides the configuration in xperf string form for which providers/keywords/levels to be enabled. + /// If not specified, the default configuration is used. + /// - COMPlus_EventPipeOutputFile : The full path to the netperf file to be written. + /// - COMPlus_EventPipeCircularMB : The size in megabytes of the circular buffer. + /// + internal static class EventPipeController + { + // Miscellaneous constants. + private const string DefaultAppName = "app"; + private const string NetPerfFileExtension = ".netperf"; + private const string NetTraceFileExtension = ".nettrace"; + private const uint DefaultCircularBufferMB = 256; // MB (PerfView and dotnet-trace default) + private const char ProviderConfigDelimiter = ','; + private const char ConfigComponentDelimiter = ':'; + + // The default set of providers/keywords/levels. Used if an alternative configuration is not specified. + private static EventPipeProviderConfiguration[] DefaultProviderConfiguration => new EventPipeProviderConfiguration[] + { + new EventPipeProviderConfiguration("Microsoft-Windows-DotNETRuntime", 0x4c14fccbd, 5, null), + new EventPipeProviderConfiguration("Microsoft-Windows-DotNETRuntimePrivate", 0x4002000b, 5, null), + new EventPipeProviderConfiguration("Microsoft-DotNETCore-SampleProfiler", 0x0, 5, null), + }; + + private static bool IsControllerInitialized { get; set; } = false; + + internal static void Initialize() + { + // Don't allow failures to propagate upstream. Ensure program correctness without tracing. + try + { + if (!IsControllerInitialized) + { + if (Config_EnableEventPipe > 0) + { + // Enable tracing immediately. + // It will be disabled automatically on shutdown. + EventPipe.Enable(BuildConfigFromEnvironment()); + } + + RuntimeEventSource.Initialize(); + + IsControllerInitialized = true; + } + } + catch { } + } + + private static EventPipeConfiguration BuildConfigFromEnvironment() + { + // Build the full path to the trace file. + string traceFileName = BuildTraceFileName(); + string outputFilePath = Path.Combine(Config_EventPipeOutputPath, traceFileName); + + // Create a new configuration object. + EventPipeConfiguration config = new EventPipeConfiguration( + outputFilePath, + (Config_NetTraceFormat != 0) ? EventPipeSerializationFormat.NetTrace : EventPipeSerializationFormat.NetPerf, + Config_EventPipeCircularMB); + + // Get the configuration. + string? strConfig = Config_EventPipeConfig; + if (!string.IsNullOrEmpty(strConfig)) + { + // If the configuration is specified, parse it and save it to the config object. + SetProviderConfiguration(strConfig, config); + } + else + { + // Specify the default configuration. + config.EnableProviderRange(DefaultProviderConfiguration); + } + + return config; + } + + private static string BuildTraceFileName() + { + return GetAppName() + "." + Interop.GetCurrentProcessId().ToString() + + ((Config_NetTraceFormat != 0) ? NetTraceFileExtension : NetPerfFileExtension); + } + + private static string GetAppName() + { + string? appName = null; + Assembly? entryAssembly = Assembly.GetEntryAssembly(); + if (entryAssembly != null) + { + AssemblyName? assemblyName = entryAssembly.GetName(); + if (assemblyName != null) + { + appName = assemblyName.Name; + } + } + + if (string.IsNullOrEmpty(appName)) + { + appName = DefaultAppName; + } + + return appName; + } + + private static void SetProviderConfiguration(string strConfig, EventPipeConfiguration config) + { + if (string.IsNullOrEmpty(strConfig)) + { + throw new ArgumentNullException(nameof(strConfig)); + } + + // Provider format: "(GUID|KnownProviderName)[:Flags[:Level][:KeyValueArgs]]" + // where KeyValueArgs are of the form: "[key1=value1][;key2=value2]" + // `strConfig` must be of the form "Provider[,Provider]" + string[] providers = strConfig.Split( + ProviderConfigDelimiter, + StringSplitOptions.RemoveEmptyEntries); // Remove "empty" providers. + foreach (string provider in providers) + { + // Split expecting a maximum of four tokens. + string[] components = provider.Split( + ConfigComponentDelimiter, + 4, // if there is ':' in the parameters then anything after it will not be ignored. + StringSplitOptions.None); // Keep empty tokens + + string? providerName = components.Length > 0 ? components[0] : null; + if (string.IsNullOrEmpty(providerName)) + continue; // No provider name specified. + + ulong keywords = ulong.MaxValue; + if (components.Length > 1) + { + // We use a try/catch block here because ulong.TryParse won't accept 0x at the beginning + // of a hex string. Thus, we either need to conditionally strip it or handle the exception. + // Given that this is not a perf-critical path, catching the exception is the simpler code. + try + { + keywords = Convert.ToUInt64(components[1], 16); + } + catch + { + } + } + + uint level = 5; // Verbose + if (components.Length > 2) + { + uint.TryParse(components[2], out level); + } + + string? filterData = components.Length > 3 ? components[3] : null; + + config.EnableProviderWithFilter(providerName, keywords, level, filterData); + } + } + + /// + /// Returns -1 if the EnableEventPipe environment variable is not set at all (or is illegal) + /// + private static int Config_EnableEventPipe + { + get + { + string? stringValue = CompatibilitySwitch.GetValueInternal("EnableEventPipe"); + if ((stringValue == null) || (!int.TryParse(stringValue, out int value))) + { + value = -1; // Indicates no value (or is illegal) + } + + return value; + } + } + + private static int Config_NetTraceFormat + { + get + { + string? stringValue = CompatibilitySwitch.GetValueInternal("EventPipeNetTraceFormat"); + if ((stringValue == null) || (!int.TryParse(stringValue, out int value))) + { + value = -1; // Indicates no value (or is illegal) + } + + return value; + } + } + + private static string? Config_EventPipeConfig => CompatibilitySwitch.GetValueInternal("EventPipeConfig"); + + private static uint Config_EventPipeCircularMB + { + get + { + string? stringValue = CompatibilitySwitch.GetValueInternal("EventPipeCircularMB"); + if ((stringValue == null) || (!uint.TryParse(stringValue, out uint value))) + { + value = DefaultCircularBufferMB; + } + + return value; + } + } + + private static string Config_EventPipeOutputPath => + CompatibilitySwitch.GetValueInternal("EventPipeOutputPath") ?? "."; + } +} + +#endif // FEATURE_PERFTRACING diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs b/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs index c2739308bfe84c..220c7bac4bb209 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/StartupHookProvider.cs @@ -25,8 +25,8 @@ private struct StartupHookNameOrPath // containing a startup hook, and call each hook in turn. private static void ProcessStartupHooks() { - // Initialize System.Runtime EventSource - System.Diagnostics.Tracing.RuntimeEventSource.Initialize(); + // Initialize tracing before any user code can be called. + System.Diagnostics.Tracing.EventPipeController.Initialize(); string? startupHooksVariable = (string?)AppContext.GetData("STARTUP_HOOKS"); if (startupHooksVariable == null) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index e9e2a6b6de5a55..036fd716bb13c9 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -101,8 +101,9 @@ void EventPipe::Initialize() if (tracingInitialized) s_state = EventPipeState::Initialized; } - +#ifdef FEATURE_EVENTPIPE_STARTUP EnableViaEnvironmentVariables(); +#endif // FEATURE_EVENTPIPE_STARTUP } // @@ -123,45 +124,37 @@ void EventPipe::EnableViaEnvironmentVariables() eventpipeOutputPath = W("trace.nettrace"); } auto configuration = XplatEventLoggerConfiguration(); - auto configToParse = eventpipeConfig; - bool enableDefaultConfig = false; - - if (configToParse == nullptr || *configToParse == L'\0') - { - enableDefaultConfig = true; - } - - // Count how many providers there are to parse - int cnt = 0; - static WCHAR comma = W(','); - while(configToParse != nullptr) - { - cnt += 1; - auto end = wcschr(configToParse, comma); - if (end == nullptr) - { - break; - } - configToParse = end + 1; - } + LPWSTR configToParse = eventpipeConfig; + int providerCnt = 0; // Create EventPipeProviderConfiguration and start tracing. EventPipeProviderConfiguration* pProviders = nullptr; // If COMPlus_EnableEventPipe is set to 1 but no configuration was specified, enable EventPipe session // with the default provider configurations. - if (enableDefaultConfig) + if (configToParse == nullptr || *configToParse == L'\0') { - // TODO: Enable SampleProfiler once we can mutate a EventPipe session post-creation. - cnt = 2; - pProviders = new EventPipeProviderConfiguration[cnt]; + providerCnt = 2; + pProviders = new EventPipeProviderConfiguration[providerCnt]; pProviders[0] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntime"), 0x4c14fccbd, 5, nullptr); pProviders[1] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntimePrivate"), 0x4002000b, 5, nullptr); } else { + // Count how many providers there are to parse + static WCHAR comma = W(','); + while(configToParse != nullptr) + { + providerCnt += 1; + auto end = wcschr(configToParse, comma); + if (end == nullptr) + { + break; + } + configToParse = end + 1; + } configToParse = eventpipeConfig; - pProviders = new EventPipeProviderConfiguration[cnt]; + pProviders = new EventPipeProviderConfiguration[providerCnt]; int i = 0; while (configToParse != nullptr) { @@ -171,11 +164,11 @@ void EventPipe::EnableViaEnvironmentVariables() // SampleProfiler can't be enabled on startup yet. if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0) { - cnt -= 1; + providerCnt -= 1; } - else if (!configuration.IsValid()) // exclude invalid provider configurations + else if (!configuration.IsValid()) // if we find any invalid configuration, do not trace. { - cnt -= 1; + return; } else { @@ -195,13 +188,13 @@ void EventPipe::EnableViaEnvironmentVariables() } } - if (cnt != 0) + if (providerCnt != 0) { - UINT64 sessionID = EventPipe::Enable( + uint64_t sessionID = EventPipe::Enable( eventpipeOutputPath, eventpipeCircularBufferMB, pProviders, - cnt, + providerCnt, EventPipeSessionType::File, EventPipeSerializationFormat::NetTraceV4, true, @@ -209,7 +202,6 @@ void EventPipe::EnableViaEnvironmentVariables() ); EventPipe::StartStreaming(sessionID); } - } } From 217fbd35cdae56e1377783ec6b6f47e236c25d15 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 19 Feb 2020 14:01:36 -0800 Subject: [PATCH 11/18] pr feedback --- src/coreclr/src/inc/eventtracebase.h | 301 ++++++++++++++------------- 1 file changed, 152 insertions(+), 149 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index dfae2ae40d2150..1a27c9887d43c7 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -65,153 +65,6 @@ enum EtwThreadFlags kEtwThreadFlagThreadPoolWorker = 0x00000004, }; -#if defined(FEATURE_PERFTRACING) -class XplatEventLoggerConfiguration -{ -public: - XplatEventLoggerConfiguration() = default; - - XplatEventLoggerConfiguration(XplatEventLoggerConfiguration const & other) = delete; - XplatEventLoggerConfiguration(XplatEventLoggerConfiguration && other) - { - _provider = std::move(other._provider); - _isValid = other._isValid; - _enabledKeywords = other._enabledKeywords; - _level = other._level; - } - - ~XplatEventLoggerConfiguration() - { - _provider = nullptr; - } - - void Parse(LPWSTR configString) - { - auto providerComponent = GetNextComponentString(configString); - _provider = ParseProviderName(providerComponent); - if (_provider == nullptr) - { - _isValid = false; - return; - } - - auto keywordsComponent = GetNextComponentString(providerComponent.End + 1); - _enabledKeywords = ParseEnabledKeywordsMask(keywordsComponent); - - auto levelComponent = GetNextComponentString(keywordsComponent.End + 1); - _level = ParseLevel(levelComponent); - - auto argumentComponent = GetNextComponentString(levelComponent.End + 1); - _argument = ParseArgument(argumentComponent); - - _isValid = true; - } - - bool IsValid() const - { - return _isValid; - } - - LPCWSTR GetProviderName() const - { - return _provider; - } - - ULONGLONG GetEnabledKeywordsMask() const - { - return _enabledKeywords; - } - - UINT GetLevel() const - { - return _level; - } - - LPCWSTR GetArgument() const - { - return _argument; - } - -private: - struct ComponentSpan - { - public: - ComponentSpan(LPCWSTR start, LPCWSTR end) - : Start(start), End(end) - { - } - - LPCWSTR Start; - LPCWSTR End; - }; - - ComponentSpan GetNextComponentString(LPCWSTR start) const - { - static WCHAR ComponentDelimiter = W(':'); - - auto end = wcschr(start, ComponentDelimiter); - if (end == nullptr) - { - end = start + wcslen(start); - } - - return ComponentSpan(start, end); - } - - LPCWSTR ParseProviderName(ComponentSpan const & component) const - { - auto providerName = (WCHAR*)nullptr; - if ((component.End - component.Start) != 0) - { - auto const length = component.End - component.Start; - providerName = new WCHAR[length + 1]; - memset(providerName, '\0', (length + 1) * sizeof(WCHAR)); - wcsncpy(providerName, component.Start, length); - } - return providerName; - } - - ULONGLONG ParseEnabledKeywordsMask(ComponentSpan const & component) const - { - auto enabledKeywordsMask = (ULONGLONG)(-1); - if ((component.End - component.Start) != 0) - { - enabledKeywordsMask = _wcstoui64(component.Start, nullptr, 16); - } - return enabledKeywordsMask; - } - - UINT ParseLevel(ComponentSpan const & component) const - { - auto level = 5; // Verbose - if ((component.End - component.Start) != 0) - { - level = _wtoi(component.Start); - } - return level; - } - - LPCWSTR ParseArgument(ComponentSpan const & component) const - { - auto argument = (WCHAR*)nullptr; - if ((component.End - component.Start) != 0) - { - auto const length = component.End - component.Start; - argument = new WCHAR[length + 1]; - wcsncpy(argument, component.Start, length); - argument[length] = '\0'; - } - return argument; - } - - LPCWSTR _provider; - ULONGLONG _enabledKeywords; - UINT _level; - LPCWSTR _argument; - bool _isValid; -}; -#endif // FEATURE_PERFTRACING - #ifndef FEATURE_REDHAWK @@ -397,6 +250,156 @@ extern UINT32 g_nClrInstanceId; #include "clrproviders.h" #include "clrconfig.h" +#endif // defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) + +#if defined(FEATURE_PERFTRACING) +class XplatEventLoggerConfiguration +{ +public: + XplatEventLoggerConfiguration() = default; + + XplatEventLoggerConfiguration(XplatEventLoggerConfiguration const & other) = delete; + XplatEventLoggerConfiguration(XplatEventLoggerConfiguration && other) + { + _provider = std::move(other._provider); + _isValid = other._isValid; + _enabledKeywords = other._enabledKeywords; + _level = other._level; + } + + ~XplatEventLoggerConfiguration() + { + _provider = nullptr; + } + + void Parse(LPWSTR configString) + { + auto providerComponent = GetNextComponentString(configString); + _provider = ParseProviderName(providerComponent); + if (_provider == nullptr) + { + _isValid = false; + return; + } + + auto keywordsComponent = GetNextComponentString(providerComponent.End + 1); + _enabledKeywords = ParseEnabledKeywordsMask(keywordsComponent); + + auto levelComponent = GetNextComponentString(keywordsComponent.End + 1); + _level = ParseLevel(levelComponent); + + auto argumentComponent = GetNextComponentString(levelComponent.End + 1); + _argument = ParseArgument(argumentComponent); + + _isValid = true; + } + + bool IsValid() const + { + return _isValid; + } + + LPCWSTR GetProviderName() const + { + return _provider; + } + + uint64_t GetEnabledKeywordsMask() const + { + return _enabledKeywords; + } + + uint32_t GetLevel() const + { + return _level; + } + + LPCWSTR GetArgument() const + { + return _argument; + } + +private: + struct ComponentSpan + { + public: + ComponentSpan(LPCWSTR start, LPCWSTR end) + : Start(start), End(end) + { + } + + LPCWSTR Start; + LPCWSTR End; + }; + + ComponentSpan GetNextComponentString(LPCWSTR start) const + { + const WCHAR ComponentDelimiter = W(':'); + const WCHAR * end = wcschr(start, ComponentDelimiter); + if (end == nullptr) + { + end = start + wcslen(start); + } + + return ComponentSpan(start, end); + } + + LPCWSTR ParseProviderName(ComponentSpan const & component) const + { + auto providerName = (WCHAR*)nullptr; + if ((component.End - component.Start) != 0) + { + auto const length = component.End - component.Start; + providerName = new WCHAR[length + 1]; + wcsncpy(providerName, component.Start, length); + providerName[length] = '\0'; + } + return providerName; + } + + uint64_t ParseEnabledKeywordsMask(ComponentSpan const & component) const + { + auto enabledKeywordsMask = (uint64_t)(-1); + if ((component.End - component.Start) != 0) + { + enabledKeywordsMask = _wcstoui64(component.Start, nullptr, 16); + } + return enabledKeywordsMask; + } + + uint32_t ParseLevel(ComponentSpan const & component) const + { + int level = 5; // Verbose + if ((component.End - component.Start) != 0) + { + level = _wtoi(component.Start); + } + return level; + } + + LPCWSTR ParseArgument(ComponentSpan const & component) const + { + auto argument = (WCHAR*)nullptr; + if ((component.End - component.Start) != 0) + { + int64_t length = component.End - component.Start; + argument = new WCHAR[length + 1]; + wcsncpy(argument, component.Start, length); + argument[length] = '\0'; + } + return argument; + } + + LPCWSTR _provider; + uint64_t _enabledKeywords; + uint32_t _level; + LPCWSTR _argument; + bool _isValid; +}; +#endif // FEATURE_PERFTRACING + +#if defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) + class XplatEventLoggerController { @@ -517,7 +520,7 @@ class XplatEventLogger } while (configToParse != nullptr) { - static WCHAR comma = W(','); + const WCHAR comma = W(','); auto end = wcschr(configToParse, comma); configuration.Parse(configToParse); XplatEventLoggerController::UpdateProviderContext(configuration); @@ -1514,4 +1517,4 @@ namespace ETW }; -#endif //_ETWTRACER_HXX_ \ No newline at end of file +#endif //_ETWTRACER_HXX_ From bba144629a59475c90e9ef40ecb04879fae27199 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 19 Feb 2020 14:59:54 -0800 Subject: [PATCH 12/18] fix x86 build --- src/coreclr/src/inc/eventtracebase.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index 1a27c9887d43c7..c3b6820f1b6b56 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -65,7 +65,6 @@ enum EtwThreadFlags kEtwThreadFlagThreadPoolWorker = 0x00000004, }; - #ifndef FEATURE_REDHAWK #if defined(FEATURE_EVENT_TRACE) @@ -223,7 +222,6 @@ struct ProfilingScanContext; #include "etmdummy.h" #endif // FEATURE_EVENT_TRACE - #ifndef FEATURE_REDHAWK #include "corprof.h" @@ -382,7 +380,7 @@ class XplatEventLoggerConfiguration auto argument = (WCHAR*)nullptr; if ((component.End - component.Start) != 0) { - int64_t length = component.End - component.Start; + auto const length = component.End - component.Start; argument = new WCHAR[length + 1]; wcsncpy(argument, component.Start, length); argument[length] = '\0'; @@ -400,7 +398,6 @@ class XplatEventLoggerConfiguration #if defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) - class XplatEventLoggerController { public: From d1632c5846e5e90b9aa2d44722ec283cf40d2f33 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 19 Feb 2020 15:47:54 -0800 Subject: [PATCH 13/18] fix typo --- src/coreclr/clrfeatures.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/clrfeatures.cmake b/src/coreclr/clrfeatures.cmake index 35d8109823ae6b..4a1f2b7225d54e 100644 --- a/src/coreclr/clrfeatures.cmake +++ b/src/coreclr/clrfeatures.cmake @@ -28,6 +28,6 @@ if(NOT DEFINED FEATURE_AUTO_TRACE) set(FEATURE_AUTO_TRACE 0) endif(NOT DEFINED FEATURE_AUTO_TRACE) -if(NOT DEFINED FEATURE_AUTO_TRACE) +if(NOT DEFINED FEATURE_EVENTPIPE_STARTUP) set(FEATURE_EVENTPIPE_STARTUP 0) -endif(NOT DEFINED FEATURE_AUTO_TRACE) +endif(NOT DEFINED FEATURE_EVENTPIPE_STARTUP) From 75a53253dcf7fea5d622204bc20a73559f11367c Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 19 Feb 2020 18:18:22 -0800 Subject: [PATCH 14/18] Fix loop condition --- src/coreclr/src/vm/eventpipe.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 036fd716bb13c9..6e63c424a6fbe3 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -101,9 +101,7 @@ void EventPipe::Initialize() if (tracingInitialized) s_state = EventPipeState::Initialized; } -#ifdef FEATURE_EVENTPIPE_STARTUP EnableViaEnvironmentVariables(); -#endif // FEATURE_EVENTPIPE_STARTUP } // @@ -143,7 +141,7 @@ void EventPipe::EnableViaEnvironmentVariables() { // Count how many providers there are to parse static WCHAR comma = W(','); - while(configToParse != nullptr) + while (*configToParse != '\0') { providerCnt += 1; auto end = wcschr(configToParse, comma); @@ -156,7 +154,7 @@ void EventPipe::EnableViaEnvironmentVariables() configToParse = eventpipeConfig; pProviders = new EventPipeProviderConfiguration[providerCnt]; int i = 0; - while (configToParse != nullptr) + while (*configToParse != '\0') { auto end = wcschr(configToParse, comma); configuration.Parse(configToParse); From 0bbcaea75e0a2417384bfb5c756ea4b2bfc6a238 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Thu, 20 Feb 2020 15:58:43 -0800 Subject: [PATCH 15/18] ifdef out call to EnableViaEnvironmentVariables() --- src/coreclr/src/vm/eventpipe.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index 6e63c424a6fbe3..df33ccc99bb568 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -101,7 +101,9 @@ void EventPipe::Initialize() if (tracingInitialized) s_state = EventPipeState::Initialized; } +#ifdef FEATURE_EVENTPIPE_STARTUP EnableViaEnvironmentVariables(); +#endif // FEATURE_EVENTPIPE_STARTUP } // From 878e0f4336841d5bba19e8f08a9f9aa86f5eb3c2 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Thu, 20 Feb 2020 16:49:38 -0800 Subject: [PATCH 16/18] Use CLRConfigStringHolder --- src/coreclr/src/vm/eventpipe.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index df33ccc99bb568..c76bee937da05f 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -114,14 +114,18 @@ void EventPipe::EnableViaEnvironmentVariables() STANDARD_VM_CONTRACT; if (CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EnableEventPipe) != 0) { - LPWSTR eventpipeConfig = NULL; - CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeConfig, &eventpipeConfig); - LPCWSTR eventpipeOutputPath = Configuration::GetKnobStringValue(W("EventPipeOutputPath")); + CLRConfigStringHolder eventpipeConfig(CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeConfig)); + CLRConfigStringHolder configOutputPath(CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeOutputPath)); uint32_t eventpipeCircularBufferMB = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EventPipeCircularMB); + LPCWSTR outputPath = nullptr; - if (eventpipeOutputPath == NULL) + if (configOutputPath == NULL) { - eventpipeOutputPath = W("trace.nettrace"); + outputPath = W("trace.nettrace"); + } + else + { + outputPath = configOutputPath; } auto configuration = XplatEventLoggerConfiguration(); LPWSTR configToParse = eventpipeConfig; @@ -191,7 +195,7 @@ void EventPipe::EnableViaEnvironmentVariables() if (providerCnt != 0) { uint64_t sessionID = EventPipe::Enable( - eventpipeOutputPath, + outputPath, eventpipeCircularBufferMB, pProviders, providerCnt, From 8fcf4386f839763e4ec8ba9b723c79589515907b Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Fri, 21 Feb 2020 15:01:48 -0800 Subject: [PATCH 17/18] pr comments --- src/coreclr/src/inc/eventtracebase.h | 17 +++++++++-------- src/coreclr/src/vm/eventpipe.cpp | 12 ++++++------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index c3b6820f1b6b56..08edca4f97eee6 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -234,6 +234,14 @@ extern UINT32 g_nClrInstanceId; #if defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) #define KEYWORDZERO 0x0 +#define DEF_LTTNG_KEYWORD_ENABLED 1 +#include "clrproviders.h" +#include "clrconfig.h" + +#endif // defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) + +#if defined(FEATURE_PERFTRACING) + /***************************************/ /* Tracing levels supported by CLR ETW */ /***************************************/ @@ -244,13 +252,6 @@ extern UINT32 g_nClrInstanceId; #define TRACE_LEVEL_INFORMATION 4 // Includes non-error cases such as Entry-Exit #define TRACE_LEVEL_VERBOSE 5 // Detailed traces from intermediate steps -#define DEF_LTTNG_KEYWORD_ENABLED 1 -#include "clrproviders.h" -#include "clrconfig.h" - -#endif // defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT)) - -#if defined(FEATURE_PERFTRACING) class XplatEventLoggerConfiguration { public: @@ -367,7 +368,7 @@ class XplatEventLoggerConfiguration uint32_t ParseLevel(ComponentSpan const & component) const { - int level = 5; // Verbose + int level = TRACE_LEVEL_VERBOSE; // Verbose if ((component.End - component.Start) != 0) { level = _wtoi(component.Start); diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index c76bee937da05f..a67f63235029ae 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -18,7 +18,6 @@ #include "eventpipesession.h" #include "eventpipejsonfile.h" #include "eventtracebase.h" -#include "eventtracebase.h" #include "sampleprofiler.h" #include "win32threadpool.h" #include "ceemain.h" @@ -165,14 +164,15 @@ void EventPipe::EnableViaEnvironmentVariables() auto end = wcschr(configToParse, comma); configuration.Parse(configToParse); - // SampleProfiler can't be enabled on startup yet. - if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0) + // if we find any invalid configuration, do not trace. + if (!configuration.IsValid()) { - providerCnt -= 1; + return; } - else if (!configuration.IsValid()) // if we find any invalid configuration, do not trace. + // SampleProfiler can't be enabled on startup yet. + else if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0) { - return; + providerCnt -= 1; } else { From 3b0f1a80cd50c61405fc13559fee86c45418de9e Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Tue, 3 Mar 2020 14:56:02 -0800 Subject: [PATCH 18/18] use NewHolder to avoid leak --- src/coreclr/src/inc/eventtracebase.h | 12 ++++++------ src/coreclr/src/vm/eventpipe.cpp | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/src/inc/eventtracebase.h b/src/coreclr/src/inc/eventtracebase.h index 7580612f0d48dd..cde996a92185e2 100644 --- a/src/coreclr/src/inc/eventtracebase.h +++ b/src/coreclr/src/inc/eventtracebase.h @@ -345,9 +345,9 @@ class XplatEventLoggerConfiguration return ComponentSpan(start, end); } - LPCWSTR ParseProviderName(ComponentSpan const & component) const + NewArrayHolder ParseProviderName(ComponentSpan const & component) const { - auto providerName = (WCHAR*)nullptr; + NewArrayHolder providerName = nullptr; if ((component.End - component.Start) != 0) { auto const length = component.End - component.Start; @@ -378,9 +378,9 @@ class XplatEventLoggerConfiguration return level; } - LPCWSTR ParseArgument(ComponentSpan const & component) const + NewArrayHolder ParseArgument(ComponentSpan const & component) const { - auto argument = (WCHAR*)nullptr; + NewArrayHolder argument = nullptr; if ((component.End - component.Start) != 0) { auto const length = component.End - component.Start; @@ -391,10 +391,10 @@ class XplatEventLoggerConfiguration return argument; } - LPCWSTR _provider; + NewArrayHolder _provider; uint64_t _enabledKeywords; uint32_t _level; - LPCWSTR _argument; + NewArrayHolder _argument; bool _isValid; }; #endif // FEATURE_PERFTRACING diff --git a/src/coreclr/src/vm/eventpipe.cpp b/src/coreclr/src/vm/eventpipe.cpp index a67f63235029ae..06d0ed7ac2c90e 100644 --- a/src/coreclr/src/vm/eventpipe.cpp +++ b/src/coreclr/src/vm/eventpipe.cpp @@ -131,7 +131,7 @@ void EventPipe::EnableViaEnvironmentVariables() int providerCnt = 0; // Create EventPipeProviderConfiguration and start tracing. - EventPipeProviderConfiguration* pProviders = nullptr; + NewHolder pProviders = nullptr; // If COMPlus_EnableEventPipe is set to 1 but no configuration was specified, enable EventPipe session // with the default provider configurations.