-
Notifications
You must be signed in to change notification settings - Fork 150
[DO NOT MERGE] workload selection #7668
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
base: andrew/win-ssi/convert-iis-to-global
Are you sure you want to change the base?
Changes from all commits
3c52fbe
a3c2fdb
2bc31af
4f895fd
813b52d
8d12752
785976d
e101350
5526390
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,15 @@ | |
| #ifdef _WIN32 | ||
| #include <windows.h> | ||
| EXTERN_C IMAGE_DOS_HEADER __ImageBase; | ||
| #define HINST_THISCOMPONENT ((HINSTANCE) &__ImageBase) | ||
| #define HINST_THISCOMPONENT ((HINSTANCE) & __ImageBase) | ||
| #else | ||
| #define _GNU_SOURCE | ||
| #include <dlfcn.h> | ||
| #endif | ||
|
|
||
| #include <algorithm> | ||
| #include <optional> | ||
|
|
||
| #include "../../../shared/src/native-src/dd_filesystem.hpp" | ||
| // namespace fs is an alias defined in "dd_filesystem.hpp" | ||
| #include "../../../shared/src/native-src/pal.h" | ||
|
|
@@ -26,13 +29,26 @@ static fs::path GetCurrentModuleFolderPath() | |
| } | ||
| #else | ||
| Dl_info info; | ||
| if (dladdr((void*)GetCurrentModuleFolderPath, &info)) | ||
| if (dladdr((void*) GetCurrentModuleFolderPath, &info)) | ||
| { | ||
| return fs::path(info.dli_fname).remove_filename(); | ||
| } | ||
| #endif | ||
| return {}; | ||
| } | ||
| #ifdef _WIN32 | ||
| static fs::path GetPoliciesPath() | ||
| { | ||
| fs::path program_data_path = shared::GetEnvironmentValue(WStr("PROGRAMDATA")); | ||
|
|
||
| if (program_data_path.empty()) | ||
| { | ||
| program_data_path = WStr(R"(C:\ProgramData)"); | ||
| } | ||
|
|
||
| return program_data_path / "Datadog" / "workload_selection.fb"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be consistent with what's done on linux, I think this should be named: |
||
| } | ||
| #endif | ||
|
|
||
| static ::shared::WSTRING GetDatadogLogsDirectoryPath() | ||
| { | ||
|
|
@@ -78,12 +94,44 @@ static fs::path GetConfigurationFilePath() | |
| return GetCurrentModuleFolderPath() / conf_filename; | ||
| } | ||
|
|
||
| inline bool IsSingleStepInstrumentation() | ||
| { | ||
| const auto isSingleStepVariable = ::shared::GetEnvironmentValue(environment::single_step_instrumentation_enabled); | ||
| return !isSingleStepVariable.empty(); | ||
| } | ||
|
|
||
| inline bool IsRunningOnIIS() | ||
| { | ||
| const auto& process_name = ::shared::GetCurrentProcessName(); | ||
| return process_name == WStr("w3wp.exe") || process_name == WStr("iisexpress.exe"); | ||
| } | ||
|
|
||
| inline std::optional<::shared::WSTRING> GetApplicationPool() | ||
| { | ||
| if (const auto& app_pool_id = ::shared::GetEnvironmentValue(environment::azure_app_services_app_pool_id); | ||
| !app_pool_id.empty()) | ||
| { | ||
| return app_pool_id; | ||
| } | ||
|
|
||
| // Try to infer the Application Pool from the command line. w3wp.exe (IIS Worker Process) | ||
| // can be started with an Application Pool by using `-ap` argument. | ||
| const auto [_, argv] = ::shared::GetCurrentProcessCommandLine(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already grab this in cor_profiler, maybe we can just pass it in?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed in 813b52d |
||
| auto it = std::find(argv.cbegin(), argv.cend(), WStr("-ap")); | ||
| if (it == argv.cend()) | ||
| { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| it = std::next(it); | ||
| if (it == argv.cend() || it->empty()) | ||
| { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| return *it; | ||
| } | ||
|
|
||
| inline std::string GetCurrentOsArch(bool isRunningOnAlpine) | ||
| { | ||
| #if AMD64 | ||
|
|
@@ -137,4 +185,4 @@ inline std::string GetCurrentOsArch(bool isRunningOnAlpine) | |
| #else | ||
| #error "currentOsArch not defined." | ||
| #endif | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| #pragma once | ||
| #ifdef BUILD_WITH_WORKLOAD_SELECTION | ||
| #include "workload_selection_impl.h" | ||
| #else | ||
| #include "workload_selection_noop.h" | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought - I'd suggest that we only keep this calculation as a "fallback" path, and instead that we set this path via an env variable in fleet installer? That way if, for whatever reason, we need to move where the agent puts this, we're still backwards compatible? WDYT @bmermet ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will it help maintaining backward compatibility? Since,
FleetInstallerand the native profiler are shipped together in the OCI, it seems to me it doesn't make a big difference if the path is computed in either of them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking was that this would be the fleet installer (not the .NET
FleetInstaller.exe😅 ) that sets the path. Because the location of the flatbuffers file is essentially managed completely independently, by the agent(?), so what happens if we need to put it somewhere else, or rename it or etc? A new version of the agent/installer might need to change the paths, say for security reasons?As an aside, purely for integration testing, it makes this actually testable in CI 😄 So I'd advocate for an env var override either way tbh, even if we want to have the fallback be the "normal" case