Skip to content

Commit

Permalink
[Profiler] Disable SSI telemetry by default (#6020)
Browse files Browse the repository at this point in the history
## Summary of changes
Enable SSI telemetry only by env var 

## Reason for change
Avoid conflicts with Tracer telemetry

## Implementation details
Enable SSI telemetry only when
DD_INTERNAL_PROFILING_SSI_TELEMETRY_ENABLED is set AND deployed via SSI

## Test coverage
Corresponding tests have been updated and incremented to cover the new
scenarios

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->
  • Loading branch information
chrisnas authored Sep 12, 2024
1 parent 99d8543 commit fe099b0
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ ApplicationStore::ApplicationStore(IConfiguration* configuration, IRuntimeInfo*
_pSsiManager{ssiManager},
_pRuntimeInfo{runtimeInfo}
{
// SSI telemetry is enabled if the configuration says so and the profiler has been deployed via SSI
_isSsiTelemetryEnabled = _pConfiguration->IsSsiTelemetryEnabled() ? (_pSsiManager->GetDeploymentMode() == DeploymentMode::SingleStepInstrumentation) : false;
}

ApplicationStore::~ApplicationStore() = default;
Expand Down Expand Up @@ -86,7 +88,7 @@ bool ApplicationStore::StopImpl()

void ApplicationStore::InitializeTelemetryMetricsWorker(std::string const& runtimeId, ApplicationInfo& info)
{
if (_pSsiManager->GetDeploymentMode() != DeploymentMode::SingleStepInstrumentation)
if (!_isSsiTelemetryEnabled)
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,5 @@ class ApplicationStore
IRuntimeInfo* _pRuntimeInfo;
std::unordered_map<std::string, ApplicationInfo> _infos;
std::mutex _infosLock;
bool _isSsiTelemetryEnabled;
};
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ Configuration::Configuration()
_enablementStatus = ExtractEnablementStatus();
_ssiLongLivedThreshold = ExtractSsiLongLivedThreshold();
_isTelemetryToDiskEnabled = GetEnvironmentValue(EnvironmentVariables::TelemetryToDiskEnabled, false);
_isSsiTelemetryEnabled = GetEnvironmentValue(EnvironmentVariables::SsiTelemetryEnabled, false);
_cpuProfilerType = GetEnvironmentValue(EnvironmentVariables::CpuProfilerType, CpuProfilerType::ManualCpuTime);
}

Expand Down Expand Up @@ -730,3 +731,8 @@ bool Configuration::IsTelemetryToDiskEnabled() const
{
return _isTelemetryToDiskEnabled;
}

bool Configuration::IsSsiTelemetryEnabled() const
{
return _isSsiTelemetryEnabled;
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class Configuration final : public IConfiguration
DeploymentMode GetDeploymentMode() const override;
std::chrono::milliseconds GetSsiLongLivedThreshold() const override;
bool IsTelemetryToDiskEnabled() const override;
bool IsSsiTelemetryEnabled() const override;
CpuProfilerType GetCpuProfilerType() const override;
std::chrono::milliseconds GetCpuProfilingInterval() const override;

Expand Down Expand Up @@ -171,6 +172,8 @@ class Configuration final : public IConfiguration
EnablementStatus _enablementStatus;
std::chrono::milliseconds _ssiLongLivedThreshold;
bool _isTelemetryToDiskEnabled;
bool _isSsiTelemetryEnabled;

CpuProfilerType _cpuProfilerType;
std::chrono::milliseconds _cpuProfilingInterval;
};
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class EnvironmentVariables final
inline static const shared::WSTRING CpuProfilingInterval = WStr("DD_INTERNAL_CPU_PROFILING_INTERVAL");
inline static const shared::WSTRING SsiLongLivedThreshold = WStr("DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD");
inline static const shared::WSTRING TelemetryToDiskEnabled = WStr("DD_INTERNAL_PROFILING_TELEMETRY_TO_DISK_ENABLED");
inline static const shared::WSTRING SsiTelemetryEnabled = WStr("DD_INTERNAL_PROFILING_SSI_TELEMETRY_ENABLED");

inline static const shared::WSTRING CIVisibilityEnabled = WStr("DD_CIVISIBILITY_ENABLED");
inline static const shared::WSTRING InternalCIVisibilitySpanId = WStr("DD_INTERNAL_CIVISIBILITY_SPANID");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,5 @@ class IConfiguration
virtual std::chrono::milliseconds GetCpuProfilingInterval() const = 0;
virtual std::chrono::milliseconds GetSsiLongLivedThreshold() const = 0;
virtual bool IsTelemetryToDiskEnabled() const = 0;
virtual bool IsSsiTelemetryEnabled() const = 0;
};
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ internal class EnvironmentVariables
public const string CpuProfilingInterval = "DD_INTERNAL_CPU_PROFILING_INTERVAL";
public const string SsiShortLivedThreshold = "DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD";
public const string TelemetryToDiskEnabled = "DD_INTERNAL_PROFILING_TELEMETRY_TO_DISK_ENABLED";
public const string SsiTelemetryEnabled = "DD_INTERNAL_PROFILING_SSI_TELEMETRY_ENABLED";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public void SsiAndProfilingEnvVarTrue(string appName, string framework, string a

// deployed with SSI
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "tracer");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand Down Expand Up @@ -125,6 +126,7 @@ public void SsiAndProfilingEnvVarFalse(string appName, string framework, string
// deployed with SSI
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "tracer");
runner.Environment.SetVariable(EnvironmentVariables.ProfilerEnabled, "false");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand Down Expand Up @@ -169,6 +171,7 @@ public void SsiAndProfilingNotSsiEnabled_ShortLivedAndNoSpan(string appName, str
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "tracer");
// short lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "600000");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand All @@ -193,6 +196,7 @@ public void SsiAndProfilingNotSsiEnabled_NoSpan(string appName, string framework

// simulate long lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "1");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand All @@ -216,6 +220,7 @@ public void SsiAndProfilingNotSsiEnabled_ShortLived(string appName, string frame
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "tracer");
// short lived with span
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "600000");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand All @@ -239,6 +244,7 @@ public void SsiAndProfilingNotSsiEnabled_AllTriggered(string appName, string fra
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "tracer");
// simulate long lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "1");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand Down Expand Up @@ -276,6 +282,7 @@ public void SsiAndProfilingSsiEnabled_ShortLivedAndNoSpan(string appName, string
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "profiler");
// short lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "600000");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand All @@ -300,6 +307,7 @@ public void SsiAndProfilingSsiEnabled_NoSpan(string appName, string framework, s

// simulate long lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "1");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand All @@ -323,6 +331,7 @@ public void SsiAndProfilingSsiEnabled_ShortLived(string appName, string framewor
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "profiler");
// short lived with span
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "600000");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand All @@ -346,6 +355,7 @@ public void SsiAndProfilingSsiEnabled_AllTriggered(string appName, string framew
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "profiler");
// simulate long lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, "1");
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand Down Expand Up @@ -384,6 +394,7 @@ public void SsiAndProfilingSsiEnabled_Delayed(string appName, string framework,
runner.Environment.SetVariable(EnvironmentVariables.SsiDeployed, "profiler");
// simulate long lived
runner.Environment.SetVariable(EnvironmentVariables.SsiShortLivedThreshold, TimeSpan.FromSeconds(6).TotalMilliseconds.ToString());
runner.Environment.SetVariable(EnvironmentVariables.SsiTelemetryEnabled, "1");
runner.Environment.SetVariable(EnvironmentVariables.TelemetryToDiskEnabled, "1");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ TEST(ApplicationStoreTest, CheckTelemetryMetricsWorkerCreation)
EXPECT_CALL(mockConfiguration, GetAgentUrl()).WillRepeatedly(ReturnRef(agentUrl));
EXPECT_CALL(mockConfiguration, GetEnablementStatus()).WillRepeatedly(Return(EnablementStatus::SsiEnabled));
// for telemetry metrics worker, profiles output directory and telemetry to disk flag are checked
EXPECT_CALL(mockConfiguration, IsSsiTelemetryEnabled()).WillRepeatedly(Return(true));
EXPECT_CALL(mockConfiguration, IsTelemetryToDiskEnabled()).WillRepeatedly(Return(false));
EXPECT_CALL(mockConfiguration, GetProfilesOutputDirectory()).WillRepeatedly(ReturnRef(emptyString));

Expand All @@ -177,4 +178,33 @@ TEST(ApplicationStoreTest, CheckTelemetryMetricsWorkerCreation)
ASSERT_EQ(info.RepositoryUrl, expectedGitRepository);
ASSERT_EQ(info.CommitSha, expectedGitCommitSha);
ASSERT_NE(info.Worker, nullptr);
}

TEST(ApplicationStoreTest, CheckTelemetryMetricsWorkerNotCreatedIfNotExplicitelyEnabled)
{
auto [configuration, mockConfiguration] = CreateConfiguration();

const std::string expectedServiceName = "DefaultServiceName";
const std::string expectedVersion = "DefaultVersion";
const std::string expectedEnvironment = "DefaultEnvironment";
const std::string expectedGitRepository = "DefaultGitRepository";
const std::string expectedGitCommitSha = "DefaultGitCommitSha";

EXPECT_CALL(mockConfiguration, GetServiceName()).WillRepeatedly(ReturnRef(expectedServiceName));
EXPECT_CALL(mockConfiguration, GetVersion()).WillRepeatedly(ReturnRef(expectedVersion));
EXPECT_CALL(mockConfiguration, GetEnvironment()).WillRepeatedly(ReturnRef(expectedEnvironment));
EXPECT_CALL(mockConfiguration, GetGitRepositoryUrl()).WillRepeatedly(ReturnRef(expectedGitRepository));
EXPECT_CALL(mockConfiguration, GetGitCommitSha()).WillRepeatedly(ReturnRef(expectedGitCommitSha));
EXPECT_CALL(mockConfiguration, GetEnablementStatus()).WillRepeatedly(Return(EnablementStatus::SsiEnabled));
EXPECT_CALL(mockConfiguration, IsSsiTelemetryEnabled()).WillRepeatedly(Return(false));

auto [ssiManager, mockSsiManager] = CreateSsiManager();
EXPECT_CALL(mockSsiManager, GetDeploymentMode()).WillRepeatedly(Return(DeploymentMode::SingleStepInstrumentation));
RuntimeInfoHelper helper(6, 0, false);

ApplicationStore applicationStore(configuration.get(), helper.GetRuntimeInfo(), ssiManager.get());

const auto& info = applicationStore.GetApplicationInfo("{82F18E9B-138D-4202-8D21-7BE1AF82EC8B}");

ASSERT_EQ(info.Worker, nullptr);
}
31 changes: 31 additions & 0 deletions profiler/test/Datadog.Profiler.Native.Tests/ConfigurationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1114,4 +1114,35 @@ TEST_F(ConfigurationTest, CheckLongLivedThresholdIsDefaultIfSetToNegativeValue)
EnvironmentHelper::EnvironmentVariable ar(EnvironmentVariables::SsiLongLivedThreshold, WStr("-1"));
auto configuration = Configuration{};
ASSERT_THAT(configuration.GetSsiLongLivedThreshold(), 30'000ms);
}

TEST_F(ConfigurationTest, CheckSsiTelemetryIsDisabledByDefault)
{
auto configuration = Configuration{};
auto expectedValue = false;
ASSERT_THAT(configuration.IsSsiTelemetryEnabled(), expectedValue);
}

TEST_F(ConfigurationTest, CheckSsiTelemetryIsDisabledIfTelemetryEnvVarIsDisabled)
{
EnvironmentHelper::EnvironmentVariable ar(EnvironmentVariables::SsiTelemetryEnabled, WStr("0"));
auto configuration = Configuration{};
auto expectedValue = false;
ASSERT_THAT(configuration.IsSsiTelemetryEnabled(), expectedValue);
}

TEST_F(ConfigurationTest, CheckSsiTelemetryIsDisabledByDefaultEvenIfSsiDeployed)
{
EnvironmentHelper::EnvironmentVariable ar(EnvironmentVariables::SsiDeployed, WStr("profiler,tracer"));
auto configuration = Configuration{};
auto expectedValue = false;
ASSERT_THAT(configuration.IsSsiTelemetryEnabled(), expectedValue);
}

TEST_F(ConfigurationTest, CheckSsiTelemetryIsEnabledIfTelemetryEnvVarIsEnabled)
{
EnvironmentHelper::EnvironmentVariable ar(EnvironmentVariables::SsiTelemetryEnabled, WStr("1"));
auto configuration = Configuration{};
auto expectedValue = true;
ASSERT_THAT(configuration.IsSsiTelemetryEnabled(), expectedValue);
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class MockConfiguration : public IConfiguration
MOCK_METHOD(std::chrono::milliseconds, GetCpuProfilingInterval, (), (const override));
MOCK_METHOD(std::chrono::milliseconds, GetSsiLongLivedThreshold, (), (const override));
MOCK_METHOD(bool, IsTelemetryToDiskEnabled, (), (const override));
MOCK_METHOD(bool, IsSsiTelemetryEnabled, (), (const override));
};

class MockExporter : public IExporter
Expand Down

0 comments on commit fe099b0

Please sign in to comment.