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

Don't instrument most dotnet SDK calls #5564

Merged
merged 15 commits into from
May 16, 2024
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
77 changes: 77 additions & 0 deletions shared/src/Datadog.Trace.ClrProfiler.Native/cor_profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,83 @@ namespace datadog::shared::nativeloader
return CORPROF_E_PROFILER_CANCEL_ACTIVATION;
}
}

// If we weren't on the explicit include list, then try to filter out `dotnet build` etc.
// We don't want to instrument _build_ processes in dotnet by default, as they generally
// don't give useful information, add latency, and risk triggering bugs in the runtime,
// particularly around shutdown, like this one: https://github.com/dotnet/runtime/issues/55441
const auto [process_command_line , tokenized_command_line] = GetCurrentProcessCommandLine();
Log::Info("Process CommandLine: ", process_command_line);

if (!process_command_line.empty())
{
const auto isDotNetProcess = process_name == WStr("dotnet") || process_name == WStr("dotnet.exe");
const auto token_count = tokenized_command_line.size();
if (isDotNetProcess && token_count > 1)
{
// Exclude:
// - dotnet build, dotnet build myproject.csproj etc
// - dotnet build-server
// - (... dotnet COMMAND commands listed below)
// - dotnet tool ...
// - dotnet new ...
//
// There are other commands we're choosing not to check because they
// wouldn't normally be called on a build or production server, just locally
// - dotnet add package, dotnet add reference
// - dotnet sln
// - dotnet workload
// - etc
//
// There are some commands that we explicitly DO want to instrument
// i.e. dotnet run
// i.e. dotnet test
// i.e. dotnet vstest
// i.e. dotnet exec (except specific commands)
bool is_ignored_command = false;
const auto token1 = tokenized_command_line[1];
if(token1 == WStr("exec"))
{
// compiler is invoked with arguments something like this:
// dotnet exec /usr/share/dotnet/sdk/6.0.400/Roslyn/bincore/csc.dll /noconfig @/tmp/tmp8895f601306443a6a54388ecc6dcfc44.rsp
// so we check the arguments to see if any of them are an invocation of one of the dlls we want to ignore.
// We don't just check the second argument because the command could set additional flags
// for the exec function
for (int i = 2; i < token_count; ++i)
{
const auto current_token = tokenized_command_line[i];
if(!current_token.empty() &&
(EndsWith(current_token, WStr("csc.dll"))
|| EndsWith(current_token, WStr("VBCSCompiler.dll"))))
{
is_ignored_command = true;
break;
}
}
}
else if(!token1.empty())
{
is_ignored_command =
token1 == WStr("build") ||
token1 == WStr("build-server") ||
token1 == WStr("clean") ||
token1 == WStr("msbuild") ||
token1 == WStr("new") ||
token1 == WStr("nuget") ||
token1 == WStr("pack") ||
token1 == WStr("publish") ||
token1 == WStr("restore") ||
token1 == WStr("tool");
}

if (is_ignored_command)
{
Log::Info("The Tracer Profiler has been disabled because the process is 'dotnet' "
"but an unsupported command was detected");
andrewlock marked this conversation as resolved.
Show resolved Hide resolved
return CORPROF_E_PROFILER_CANCEL_ACTIVATION;
}
}
}
}

//
Expand Down
33 changes: 25 additions & 8 deletions shared/src/native-src/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "windows.h"
#include <process.h>
#include <shellapi.h>

#else

Expand Down Expand Up @@ -129,20 +130,33 @@ inline WSTRING GetCurrentProcessName()
#endif
}

inline WSTRING GetCurrentProcessCommandLine()
inline std::tuple<WSTRING, std::vector<WSTRING>> GetCurrentProcessCommandLine()
{
std::vector<WSTRING> args;
#ifdef _WIN32
return WSTRING(GetCommandLine());
const auto cmdLine = GetCommandLine();
int argCount;
const auto arguments = CommandLineToArgvW(cmdLine, &argCount);

for (int i = 0; i < argCount; i++)
{
args.push_back(Trim(arguments[i]));
}

return { cmdLine, args };
#elif MACOS
std::string name;
int argCount = *_NSGetArgc();
char** arguments = *_NSGetArgv();

for (int i = 0; i < argCount; i++)
{
char* currentArg = arguments[i];
name = name + " " + std::string(currentArg);
const auto currentArg = std::string(arguments[i]);
args.push_back(Trim(ToWSTRING(currentArg)));
name = name + " " + currentArg;
}
return Trim(ToWSTRING(name));

return { Trim(ToWSTRING(name)), args };
#else
std::string cmdline;
char buf[1024];
Expand All @@ -154,6 +168,8 @@ inline WSTRING GetCurrentProcessCommandLine()
{
cmdline.append(buf, len);
}

fclose(fp);
}

std::string name;
Expand All @@ -162,13 +178,14 @@ inline WSTRING GetCurrentProcessCommandLine()
while (getline(tokens, tmp, '\0'))
{
name = name + " " + tmp;
args.push_back(Trim(ToWSTRING(tmp)));
}
fclose(fp);
andrewlock marked this conversation as resolved.
Show resolved Hide resolved

return Trim(ToWSTRING(name));
return { Trim(ToWSTRING(name)), args };

#endif

return EmptyWStr;
return {EmptyWStr, args};
}

inline int GetPID()
Expand Down
10 changes: 10 additions & 0 deletions shared/src/native-src/string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,19 @@ namespace shared {
return str.size() >= suffix.size() && str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0;
}

bool EndsWith(const WSTRING& str, const WSTRING& suffix)
{
return str.size() >= suffix.size() && str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0;
}

bool StartsWith(const std::string &str, const std::string &prefix)
{
return str.size() >= prefix.size() && str.compare(0, prefix.size(), prefix) == 0;
}

bool StartsWith(const WSTRING &str, const WSTRING &prefix)
{
return str.size() >= prefix.size() && str.compare(0, prefix.size(), prefix) == 0;
}

} // namespace trace
4 changes: 3 additions & 1 deletion shared/src/native-src/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ WSTRING ToWSTRING(uint64_t i);
bool TryParse(WSTRING const& s, int& result);

bool EndsWith(const std::string& str, const std::string& suffix);

bool StartsWith(const std::string& str, const std::string& prefix);

bool EndsWith(const WSTRING& str, const WSTRING& suffix);
bool StartsWith(const WSTRING& str, const WSTRING& prefix);

template <typename TChar>
std::basic_string<TChar> ReplaceString(std::basic_string<TChar> subject, const std::basic_string<TChar>& search,
const std::basic_string<TChar>& replace)
Expand Down
18 changes: 6 additions & 12 deletions tracer/src/Datadog.Tracer.Native/cor_profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Initialize(IUnknown* cor_profiler_info_un
const auto process_name = shared::GetCurrentProcessName();
Logger::Info("ProcessName: ", process_name);

const auto process_command_line = shared::GetCurrentProcessCommandLine();
const auto [process_command_line , tokenized_command_line ] = GetCurrentProcessCommandLine();
Logger::Info("Process CommandLine: ", process_command_line);

// CI visibility checks
Expand All @@ -83,19 +83,13 @@ HRESULT STDMETHODCALLTYPE CorProfiler::Initialize(IUnknown* cor_profiler_info_un
is_ci_visibility_enabled)
{
const auto isDotNetProcess = process_name == WStr("dotnet") || process_name == WStr("dotnet.exe");

const auto token_count = tokenized_command_line.size();
if (isDotNetProcess &&
token_count > 1 &&
tokenized_command_line[1] != WStr("test") &&
// these are executed with exec, so we could check for that, but the
// below check is more conservative, so leaving at that
process_command_line.find(WStr("testhost")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet\" test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet' test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet.exe test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet.exe\" test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet.exe' test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet.dll test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet.dll\" test")) == WSTRING::npos &&
process_command_line.find(WStr("dotnet.dll' test")) == WSTRING::npos &&
process_command_line.find(WStr(" test ")) == WSTRING::npos &&
process_command_line.find(WStr("datacollector")) == WSTRING::npos)
{
Logger::Info("The Tracer Profiler has been disabled because the process is running in CI Visibility "
Expand Down
Loading
Loading