Skip to content

Move EventPipe environment variable parsing logic to native code #32516

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.

Sign up for GitHub

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

Merged
sywhang merged 22 commits into from
Mar 4, 2020
Merged

Move EventPipe environment variable parsing logic to native code #32516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
967e672
Move environment variable parsing logic to native code
sywhang Jan 10, 2020
7b5e97f
Fix multiple provider issue
sywhang Jan 15, 2020
5df4b8d
Merging with master
sywhang Feb 13, 2020
d2a787b
Use env vars to set up eventpipe config
sywhang Feb 14, 2020
9d2f0b8
Add default provider configuration if eventpipeconfig is not set
sywhang Feb 15, 2020
f2a9138
Remove EventPipeController
sywhang Feb 18, 2020
73dbabd
Add Argument parsing logic to XplatEventLoggerConfiguration
sywhang Feb 18, 2020
6098b6c
cleanup
sywhang Feb 18, 2020
d845bb0
Fix Linux build
sywhang Feb 19, 2020
6515b7b
Check provider configuration before enabling them
sywhang Feb 19, 2020
63a4a46
wrap new code under ifdef, put EventPipeController back
sywhang Feb 19, 2020
217fbd3
pr feedback
sywhang Feb 19, 2020
bba1446
fix x86 build
sywhang Feb 19, 2020
d1632c5
fix typo
sywhang Feb 19, 2020
75a5325
Fix loop condition
sywhang Feb 20, 2020
0bbcaea
ifdef out call to EnableViaEnvironmentVariables()
sywhang Feb 20, 2020
878e0f4
Use CLRConfigStringHolder
sywhang Feb 21, 2020
8fcf438
pr comments
sywhang Feb 21, 2020
e6fa99d
Merge branch 'master' into dev/suwhang/move-eventpipe-envvar-parsing
sywhang Feb 26, 2020
fc849a6
Fix merge conflict
sywhang Feb 26, 2020
eee8f0e
Merge branch 'dev/suwhang/move-eventpipe-envvar-parsing' of github.co…
sywhang Feb 27, 2020
3b0f1a8
use NewHolder to avoid leak
sywhang Mar 3, 2020
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
4 changes: 4 additions & 0 deletions src/coreclr/clrfeatures.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_EVENTPIPE_STARTUP)
set(FEATURE_EVENTPIPE_STARTUP 0)
endif(NOT DEFINED FEATURE_EVENTPIPE_STARTUP)
73 changes: 51 additions & 22 deletions src/coreclr/src/inc/eventtracebase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ 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
#ifdef FEATURE_EVENT_TRACE
#include "clrproviders.h"
#endif // FEATURE_EVENT_TRACE
#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 */
/***************************************/
Expand All @@ -244,12 +254,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
#ifdef FEATURE_EVENT_TRACE
#include "clrproviders.h"
#endif
#include "clrconfig.h"

class XplatEventLoggerConfiguration
{
public:
Expand Down Expand Up @@ -284,6 +288,10 @@ class XplatEventLoggerConfiguration

auto levelComponent = GetNextComponentString(keywordsComponent.End + 1);
_level = ParseLevel(levelComponent);

auto argumentComponent = GetNextComponentString(levelComponent.End + 1);
_argument = ParseArgument(argumentComponent);

_isValid = true;
}

Expand All @@ -297,16 +305,21 @@ class XplatEventLoggerConfiguration
return _provider;
}

ULONGLONG GetEnabledKeywordsMask() const
uint64_t GetEnabledKeywordsMask() const
{
return _enabledKeywords;
}

UINT GetLevel() const
uint32_t GetLevel() const
{
return _level;
}

LPCWSTR GetArgument() const
{
return _argument;
}

private:
struct ComponentSpan
{
Expand All @@ -322,9 +335,8 @@ class XplatEventLoggerConfiguration

ComponentSpan GetNextComponentString(LPCWSTR start) const
{
static WCHAR ComponentDelimiter = W(':');

auto end = wcschr(start, ComponentDelimiter);
const WCHAR ComponentDelimiter = W(':');
const WCHAR * end = wcschr(start, ComponentDelimiter);
if (end == nullptr)
{
end = start + wcslen(start);
Expand All @@ -333,44 +345,61 @@ class XplatEventLoggerConfiguration
return ComponentSpan(start, end);
}

LPCWSTR ParseProviderName(ComponentSpan const & component) const
NewArrayHolder<WCHAR> ParseProviderName(ComponentSpan const & component) const
{
auto providerName = (WCHAR*)nullptr;
NewArrayHolder<WCHAR> providerName = 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);
providerName[length] = '\0';
}
return providerName;
}

ULONGLONG ParseEnabledKeywordsMask(ComponentSpan const & component) const
uint64_t ParseEnabledKeywordsMask(ComponentSpan const & component) const
{
auto enabledKeywordsMask = (ULONGLONG)(-1);
auto enabledKeywordsMask = (uint64_t)(-1);
if ((component.End - component.Start) != 0)
{
enabledKeywordsMask = _wcstoui64(component.Start, nullptr, 16);
}
return enabledKeywordsMask;
}

UINT ParseLevel(ComponentSpan const & component) const
uint32_t ParseLevel(ComponentSpan const & component) const
{
auto level = TRACE_LEVEL_VERBOSE;
int level = TRACE_LEVEL_VERBOSE; // Verbose
if ((component.End - component.Start) != 0)
{
level = _wtoi(component.Start);
}
return level;
}

LPCWSTR _provider;
ULONGLONG _enabledKeywords;
UINT _level;
NewArrayHolder<WCHAR> ParseArgument(ComponentSpan const & component) const
{
NewArrayHolder<WCHAR> argument = 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;
}

NewArrayHolder<WCHAR> _provider;
uint64_t _enabledKeywords;
uint32_t _level;
NewArrayHolder<WCHAR> _argument;
bool _isValid;
};
#endif // FEATURE_PERFTRACING

#if defined(HOST_UNIX) && (defined(FEATURE_EVENT_TRACE) || defined(FEATURE_EVENTSOURCE_XPLAT))
Copy link
Member

@am11 am11 Mar 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sywhang, we updated the build in #32746, so that user can build coreclr without FEATURE_EVENT_TRACE enabled. After rebasing my other PR #32800 branch against master, I am getting errors that XplatEventLoggerConfiguration on line 408 is not defined. The reason is that FEATURE_PERFTRACING is only set when FEATURE_EVENT_TRACE is enabled, so the class definition is omitted from the compilation.

One fix could be to change line 245 to #if defined(FEATURE_PERFTRACING) || defined(FEATURE_EVENTSOURCE_XPLAT) , (along with #32800 current changes), just wanted to confirm if you have any thoughts on such a fix?

Thanks!

Copy link
Contributor Author

@sywhang sywhang Mar 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@am11 ah, sorry about that!

One fix could be to change line 245 to #if defined(FEATURE_PERFTRACING) || defined(FEATURE_EVENTSOURCE_XPLAT) , (along with #32800 current changes), just wanted to confirm if you have any thoughts on such a fix?

That sounds fine to me.

I will try to build without FEATURE_EVENT_TRACE before merging my PRs on EventPipe from future to prevent issues like that :)

Copy link
Member

@am11 am11 Mar 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! My coreclr build on Android was failing at 11% and with that (two liner) patch (updated #endif comment as well), it has just succeeded (all green). 👍

I was wondering if we could get to run Android build even on weekly bases (AzDo scheduled jobs), that would help preserving the healthy state for this platform? It will also help validating this feature matrix. :)


class XplatEventLoggerController
{
Expand Down Expand Up @@ -497,7 +526,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);
Expand Down
109 changes: 108 additions & 1 deletion src/coreclr/src/vm/eventpipe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "sampleprofiler.h"
#include "win32threadpool.h"
#include "ceemain.h"
#include "configuration.h"

#ifdef TARGET_UNIX
#include "pal.h"
Expand Down Expand Up @@ -94,12 +95,118 @@ void EventPipe::Initialize()
#endif
}


{
CrstHolder _crst(GetLock());
if (tracingInitialized)
s_state = EventPipeState::Initialized;
}
#ifdef FEATURE_EVENTPIPE_STARTUP
EnableViaEnvironmentVariables();
#endif // FEATURE_EVENTPIPE_STARTUP
}

//
// 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)
{
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 (configOutputPath == NULL)
{
outputPath = W("trace.nettrace");
}
else
{
outputPath = configOutputPath;
}
auto configuration = XplatEventLoggerConfiguration();
LPWSTR configToParse = eventpipeConfig;
int providerCnt = 0;

// Create EventPipeProviderConfiguration and start tracing.
NewHolder<EventPipeProviderConfiguration> pProviders = nullptr;

// If COMPlus_EnableEventPipe is set to 1 but no configuration was specified, enable EventPipe session
// with the default provider configurations.
if (configToParse == nullptr || *configToParse == L'\0')
{
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 != '\0')
{
providerCnt += 1;
auto end = wcschr(configToParse, comma);
if (end == nullptr)
{
break;
}
configToParse = end + 1;
}
configToParse = eventpipeConfig;
pProviders = new EventPipeProviderConfiguration[providerCnt];
int i = 0;
while (*configToParse != '\0')
{
auto end = wcschr(configToParse, comma);
configuration.Parse(configToParse);

// if we find any invalid configuration, do not trace.
if (!configuration.IsValid())
{
return;
}
// SampleProfiler can't be enabled on startup yet.
else if (wcscmp(W("Microsoft-DotNETCore-SampleProfiler"), configuration.GetProviderName()) == 0)
{
providerCnt -= 1;
}
else
{
pProviders[i++] = EventPipeProviderConfiguration(
configuration.GetProviderName(),
configuration.GetEnabledKeywordsMask(),
configuration.GetLevel(),
configuration.GetArgument()
);
}

if (end == nullptr)
{
break;
}
configToParse = end + 1;
}
}

if (providerCnt != 0)
{
uint64_t sessionID = EventPipe::Enable(
outputPath,
eventpipeCircularBufferMB,
pProviders,
providerCnt,
EventPipeSessionType::File,
EventPipeSerializationFormat::NetTraceV4,
true,
nullptr
);
EventPipe::StartStreaming(sessionID);
}
}
}

void EventPipe::Shutdown()
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/src/vm/eventpipe.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down