Skip to content

Commit b8c325b

Browse files
schmittjosephschmittjoseph
authored
[Parameter Capturing] Add feature flag (#4696)
1 parent 81cf506 commit b8c325b

38 files changed

+692
-46
lines changed

Diff for: ‎src/Microsoft.Diagnostics.Monitoring.HostingStartup/HostingStartup.cs

+13-2
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,29 @@
55
using Microsoft.Extensions.DependencyInjection;
66
using Microsoft.Diagnostics.Monitoring.HostingStartup;
77
using Microsoft.Diagnostics.Monitoring.HostingStartup.ParameterCapturing;
8+
using Microsoft.Diagnostics.Tools.Monitor.HostingStartup;
9+
using Microsoft.Diagnostics.Tools.Monitor;
10+
using System.Threading;
811

912
[assembly: HostingStartup(typeof(HostingStartup))]
1013
namespace Microsoft.Diagnostics.Monitoring.HostingStartup
1114
{
1215
internal sealed class HostingStartup : IHostingStartup
1316
{
17+
public static int InvocationCount;
18+
1419
public void Configure(IWebHostBuilder builder)
1520
{
1621
builder.ConfigureServices(services =>
1722
{
18-
// TODO: Gate this behind an environment variable.
19-
services.AddHostedService<ParameterCapturingService>();
23+
// Keep track of how many times this hosting startup has been invoked for easy
24+
// validation in tests.
25+
Interlocked.Increment(ref InvocationCount);
26+
27+
if (ToolIdentifiers.IsEnvVarEnabled(InProcessFeaturesIdentifiers.EnvironmentVariables.EnableParameterCapturing))
28+
{
29+
services.AddHostedService<ParameterCapturingService>();
30+
}
2031
});
2132
}
2233
}

Diff for: ‎src/Microsoft.Diagnostics.Monitoring.HostingStartup/Microsoft.Diagnostics.Monitoring.HostingStartup.csproj

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
<ItemGroup>
1616
<Compile Include="..\Tools\dotnet-monitor\ToolIdentifiers.cs" />
17+
<Compile Include="..\Tools\dotnet-monitor\InProcessFeatures\InProcessFeaturesIdentifiers.cs" Link="InProcessFeaturesIdentifiers.cs" />
1718
<Compile Include="..\Tools\dotnet-monitor\Profiler\ProfilerIdentifiers.cs" />
1819
<Compile Include="..\Tools\dotnet-monitor\DisposableHelper.cs" Link="DisposableHelper.cs" />
1920
</ItemGroup>

Diff for: ‎src/Microsoft.Diagnostics.Monitoring.WebApi/IExperimentalFlags.cs

+2
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,7 @@ internal interface IExperimentalFlags
1212
bool IsCallStacksEnabled { get; }
1313

1414
bool IsExceptionsEnabled { get; }
15+
16+
bool IsParameterCapturingEnabled { get; }
1517
}
1618
}

Diff for: ‎src/Microsoft.Diagnostics.Monitoring.WebApi/IInProcessFeatures.cs

+6
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,14 @@ public interface IInProcessFeatures
99

1010
bool IsExceptionsEnabled { get; }
1111

12+
bool IsParameterCapturingEnabled { get; }
13+
1214
bool IsProfilerRequired { get; }
1315

16+
bool IsStartupHookRequired { get; }
17+
18+
bool IsHostingStartupRequired { get; }
19+
1420
bool IsLibrarySharingRequired { get; }
1521
}
1622
}

Diff for: ‎src/MonitorProfiler/Environment/EnvironmentHelper.cpp

+33
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,36 @@ HRESULT EnvironmentHelper::GetTempFolder(tstring& tempFolder)
106106

107107
return S_OK;
108108
}
109+
110+
HRESULT EnvironmentHelper::GetIsParameterCapturingEnabled(bool& isEnabled)
111+
{
112+
return GetIsFeatureEnabled(EnableParameterCapturingEnvVar, isEnabled);
113+
}
114+
115+
HRESULT EnvironmentHelper::GetIsFeatureEnabled(const LPCWSTR featureName, bool& isEnabled)
116+
{
117+
HRESULT hr;
118+
119+
isEnabled = false;
120+
121+
tstring envValue;
122+
hr = _environment->GetEnvironmentVariable(featureName, envValue);
123+
if (FAILED(hr))
124+
{
125+
if (hr != HRESULT_FROM_WIN32(ERROR_ENVVAR_NOT_FOUND))
126+
{
127+
return hr;
128+
}
129+
}
130+
else
131+
{
132+
//
133+
// Case sensitive comparision is okay here as this value is "1"
134+
// and managed by dotnet-monitor.
135+
//
136+
isEnabled = (envValue == EnableEnvVarValue);
137+
}
138+
139+
140+
return S_OK;
141+
}

Diff for: ‎src/MonitorProfiler/Environment/EnvironmentHelper.h

+9
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@
1717
class EnvironmentHelper final
1818
{
1919
private:
20+
static constexpr LPCWSTR EnableEnvVarValue = _T("1");
21+
2022
static constexpr LPCWSTR DebugLoggerLevelEnvVar = _T("DotnetMonitor_Profiler_DebugLogger_Level");
2123
static constexpr LPCWSTR ProfilerVersionEnvVar = _T("DotnetMonitor_Profiler_ProductVersion");
2224
static constexpr LPCWSTR RuntimeInstanceEnvVar = _T("DotnetMonitor_Profiler_RuntimeInstanceId");
2325
static constexpr LPCWSTR SharedPathEnvVar = _T("DotnetMonitor_Profiler_SharedPath");
2426
static constexpr LPCWSTR StdErrLoggerLevelEnvVar = _T("DotnetMonitor_Profiler_StdErrLogger_Level");
2527

28+
static constexpr LPCWSTR EnableParameterCapturingEnvVar = _T("DotnetMonitor_InProcessFeatures_EnableParameterCapturing");
29+
2630
std::shared_ptr<IEnvironment> _environment;
2731
std::shared_ptr<ILogger> _logger;
2832

@@ -62,4 +66,9 @@ class EnvironmentHelper final
6266
HRESULT GetStdErrLoggerLevel(LogLevel& level);
6367

6468
HRESULT GetTempFolder(tstring& tempFolder);
69+
70+
HRESULT GetIsParameterCapturingEnabled(bool& isEnabled);
71+
72+
private:
73+
HRESULT GetIsFeatureEnabled(const LPCWSTR featureName, bool& isEnabled);
6574
};

Diff for: ‎src/MonitorProfiler/MainProfiler/MainProfiler.cpp

+28-9
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,11 @@ STDMETHODIMP MainProfiler::Shutdown()
4444
m_pLogger.reset();
4545
m_pEnvironment.reset();
4646

47-
m_pProbeInstrumentation->ShutdownBackgroundService();
48-
m_pProbeInstrumentation.reset();
47+
if (m_pProbeInstrumentation)
48+
{
49+
m_pProbeInstrumentation->ShutdownBackgroundService();
50+
m_pProbeInstrumentation.reset();
51+
}
4952

5053
_commandServer->Shutdown();
5154
_commandServer.reset();
@@ -169,9 +172,6 @@ HRESULT MainProfiler::InitializeCommon()
169172
IfNullRet(_exceptionTracker);
170173
#endif // DOTNETMONITOR_FEATURE_EXCEPTIONS
171174

172-
m_pProbeInstrumentation.reset(new (nothrow) ProbeInstrumentation(m_pLogger, m_pCorProfilerInfo));
173-
IfNullRet(m_pProbeInstrumentation);
174-
175175
// Set product version environment variable to allow discovery of if the profiler
176176
// as been applied to a target process. Diagnostic tools must use the diagnostic
177177
// communication channel's GetProcessEnvironment command to get this value.
@@ -184,15 +184,29 @@ HRESULT MainProfiler::InitializeCommon()
184184
#endif // DOTNETMONITOR_FEATURE_EXCEPTIONS
185185
StackSampler::AddProfilerEventMask(eventsLow);
186186

187-
m_pProbeInstrumentation->AddProfilerEventMask(eventsLow);
188-
189187
_threadNameCache = make_shared<ThreadNameCache>();
190188

189+
bool enableParameterCapturing;
190+
IfFailLogRet(_environmentHelper->GetIsParameterCapturingEnabled(enableParameterCapturing));
191+
if (enableParameterCapturing)
192+
{
193+
m_pProbeInstrumentation.reset(new (nothrow) ProbeInstrumentation(m_pLogger, m_pCorProfilerInfo));
194+
IfNullRet(m_pProbeInstrumentation);
195+
m_pProbeInstrumentation->AddProfilerEventMask(eventsLow);
196+
}
197+
else
198+
{
199+
ProbeInstrumentation::DisableIncomingRequests();
200+
}
201+
191202
IfFailRet(m_pCorProfilerInfo->SetEventMask2(
192203
eventsLow,
193204
COR_PRF_HIGH_MONITOR::COR_PRF_HIGH_MONITOR_NONE));
194205

195-
IfFailLogRet(m_pProbeInstrumentation->InitBackgroundService());
206+
if (enableParameterCapturing)
207+
{
208+
IfFailLogRet(m_pProbeInstrumentation->InitBackgroundService());
209+
}
196210

197211
//Initialize this last. The CommandServer creates secondary threads, which will be difficult to cleanup if profiler initialization fails.
198212
IfFailLogRet(InitializeCommandServer());
@@ -330,5 +344,10 @@ HRESULT MainProfiler::ProcessCallstackMessage()
330344

331345
HRESULT STDMETHODCALLTYPE MainProfiler::GetReJITParameters(ModuleID moduleId, mdMethodDef methodId, ICorProfilerFunctionControl* pFunctionControl)
332346
{
333-
return m_pProbeInstrumentation->GetReJITParameters(moduleId, methodId, pFunctionControl);
347+
if (m_pProbeInstrumentation)
348+
{
349+
return m_pProbeInstrumentation->GetReJITParameters(moduleId, methodId, pFunctionControl);
350+
}
351+
352+
return S_OK;
334353
}

Diff for: ‎src/MonitorProfiler/ProbeInstrumentation/ProbeInstrumentation.cpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,14 @@ void ProbeInstrumentation::WorkerThread()
105105
}
106106
}
107107

108-
void ProbeInstrumentation::ShutdownBackgroundService()
108+
void ProbeInstrumentation::DisableIncomingRequests()
109109
{
110110
g_probeManagementQueue.Complete();
111+
}
112+
113+
void ProbeInstrumentation::ShutdownBackgroundService()
114+
{
115+
DisableIncomingRequests();
111116
m_probeManagementThread.join();
112117
}
113118

Diff for: ‎src/MonitorProfiler/ProbeInstrumentation/ProbeInstrumentation.h

+3
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,7 @@ class ProbeInstrumentation
7979
void AddProfilerEventMask(DWORD& eventsLow);
8080

8181
HRESULT STDMETHODCALLTYPE GetReJITParameters(ModuleID moduleId, mdMethodDef methodId, ICorProfilerFunctionControl* pFunctionControl);
82+
83+
public:
84+
static void DisableIncomingRequests();
8285
};

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/Runners/AppRunner.cs

+13
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ public Architecture? Architecture
8181

8282
public string ProfilerLogLevel { get; set; }
8383

84+
public bool EnableMonitorStartupHook { get; set; }
85+
8486
public AppRunner(ITestOutputHelper outputHelper, Assembly testAssembly, int appId = 1, TargetFrameworkMoniker tfm = TargetFrameworkMoniker.Current)
8587
{
8688
AppId = appId;
@@ -150,12 +152,23 @@ public async Task StartAsync(CancellationToken token)
150152
ToolIdentifiers.EnvironmentVariables.RuntimeIdentifier,
151153
NativeLibraryHelper.GetTargetRuntimeIdentifier(Architecture));
152154
}
155+
153156
if (ProfilerLogLevel != null)
154157
{
155158
_adapter.Environment.Add(
156159
ProfilerIdentifiers.EnvironmentVariables.StdErrLogger_Level, ProfilerLogLevel);
157160
}
158161

162+
if (EnableMonitorStartupHook)
163+
{
164+
string startupHookPath = AssemblyHelper.GetAssemblyArtifactBinPath(
165+
Assembly.GetExecutingAssembly(),
166+
"Microsoft.Diagnostics.Monitoring.StartupHook",
167+
TargetFrameworkMoniker.Net60);
168+
169+
_adapter.Environment.Add(ToolIdentifiers.EnvironmentVariables.StartupHooks, startupHookPath);
170+
}
171+
159172
await _adapter.StartAsync(token).ConfigureAwait(false);
160173

161174
await _readySource.WithCancellation(token);

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TestAppScenarios.cs

+12
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@ public static class SubScenarios
9393
}
9494
}
9595

96+
public static class HostingStartup
97+
{
98+
public const string Name = nameof(HostingStartup);
99+
100+
public static class SubScenarios
101+
{
102+
public const string VerifyAspNetAppWithoutHostingStartup = nameof(VerifyAspNetAppWithoutHostingStartup);
103+
public const string VerifyAspNetApp = nameof(VerifyAspNetApp);
104+
public const string VerifyNonAspNetAppNotImpacted = nameof(VerifyNonAspNetAppNotImpacted);
105+
}
106+
}
107+
96108
public static class Stacks
97109
{
98110
public const string Name = nameof(Stacks);

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/FunctionProbesTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ await ScenarioRunner.SingleTarget(
7575
configureTool: runner =>
7676
{
7777
runner.ConfigurationFromEnvironment.EnableInProcessFeatures();
78-
runner.EnableCallStacksFeature = true;
78+
runner.EnableParameterCapturingFeature = true;
7979
},
8080
profilerLogLevel: LogLevel.Trace,
8181
subScenarioName: subScenario);

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HostingStartupTests.cs

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.Diagnostics.Monitoring.TestCommon;
5+
using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Fixtures;
6+
using Microsoft.Extensions.DependencyInjection;
7+
using System.Net.Http;
8+
using System.Threading.Tasks;
9+
using Xunit;
10+
using Xunit.Abstractions;
11+
using Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.Runners;
12+
using Microsoft.Diagnostics.Monitoring.WebApi;
13+
using Microsoft.Diagnostics.Monitoring.TestCommon.Options;
14+
using System.Collections.Generic;
15+
using System.Linq;
16+
using Microsoft.Extensions.Logging;
17+
18+
namespace Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests
19+
{
20+
[TargetFrameworkMonikerTrait(TargetFrameworkMonikerExtensions.CurrentTargetFrameworkMoniker)]
21+
[Collection(DefaultCollectionFixture.Name)]
22+
public class HostingStartupTests
23+
{
24+
private readonly IHttpClientFactory _httpClientFactory;
25+
private readonly ITestOutputHelper _outputHelper;
26+
27+
public HostingStartupTests(ITestOutputHelper outputHelper, ServiceProviderFixture serviceProviderFixture)
28+
{
29+
_httpClientFactory = serviceProviderFixture.ServiceProvider.GetService<IHttpClientFactory>();
30+
_outputHelper = outputHelper;
31+
}
32+
33+
34+
[Theory]
35+
[InlineData(TestAppScenarios.HostingStartup.SubScenarios.VerifyAspNetApp, true)]
36+
[InlineData(TestAppScenarios.HostingStartup.SubScenarios.VerifyAspNetAppWithoutHostingStartup, false)]
37+
[InlineData(TestAppScenarios.HostingStartup.SubScenarios.VerifyNonAspNetAppNotImpacted, true)]
38+
39+
public async Task HostingStartupLoadTests(string subScenario, bool tryLoadHostingStartup)
40+
{
41+
await ScenarioRunner.SingleTarget(
42+
_outputHelper,
43+
_httpClientFactory,
44+
DiagnosticPortConnectionMode.Listen,
45+
TestAppScenarios.HostingStartup.Name,
46+
appValidate: (runner, client) => { return Task.CompletedTask; },
47+
configureApp: runner =>
48+
{
49+
runner.EnableMonitorStartupHook = true;
50+
},
51+
configureTool: runner =>
52+
{
53+
runner.ConfigurationFromEnvironment.EnableInProcessFeatures();
54+
// Enable a feature that requires the hosting startup assembly.
55+
runner.EnableParameterCapturingFeature = tryLoadHostingStartup;
56+
},
57+
profilerLogLevel: LogLevel.Trace,
58+
subScenarioName: subScenario);
59+
}
60+
}
61+
}

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.csproj

+2
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@
8585
</ItemGroup>
8686

8787
<ItemGroup>
88+
<ProjectReference Include="..\..\Microsoft.Diagnostics.Monitoring.HostingStartup\Microsoft.Diagnostics.Monitoring.HostingStartup.csproj" />
8889
<ProjectReference Include="..\..\Microsoft.Diagnostics.Monitoring.Options\Microsoft.Diagnostics.Monitoring.Options.csproj" />
90+
<ProjectReference Include="..\..\Microsoft.Diagnostics.Monitoring.StartupHook\Microsoft.Diagnostics.Monitoring.StartupHook.csproj" />
8991
<ProjectReference Include="..\..\Microsoft.Diagnostics.Monitoring.WebApi\Microsoft.Diagnostics.Monitoring.WebApi.csproj" />
9092
<ProjectReference Include="..\..\Tools\dotnet-monitor\dotnet-monitor.csproj">
9193
<ReferenceOutputAssembly>false</ReferenceOutputAssembly>

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/MonitorCollectRunner.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public MonitorCollectRunner(ITestOutputHelper outputHelper)
105105

106106
public override async ValueTask DisposeAsync()
107107
{
108-
if (!DisposableHelper.CanDispose(ref _disposedState))
108+
if (!TestCommon.DisposableHelper.CanDispose(ref _disposedState))
109109
{
110110
return;
111111
}

Diff for: ‎src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/MonitorRunner.cs

+13-2
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ internal class MonitorRunner : IAsyncDisposable
4848
/// </summary>
4949
public bool EnableCallStacksFeature { get; set; }
5050

51+
/// <summary>
52+
/// Determines whether the parameter capturing feature is enabled.
53+
/// </summary>
54+
public bool EnableParameterCapturingFeature { get; set; }
55+
5156
/// <summary>
5257
/// Gets the task for the underlying <see cref="DotNetRunner"/>'s
5358
/// <see cref="DotNetRunner.ExitedTask"/> which is used to wait for process exit.
@@ -108,7 +113,7 @@ public MonitorRunner(ITestOutputHelper outputHelper)
108113

109114
public virtual async ValueTask DisposeAsync()
110115
{
111-
if (!DisposableHelper.CanDispose(ref _disposedState))
116+
if (!TestCommon.DisposableHelper.CanDispose(ref _disposedState))
112117
{
113118
return;
114119
}
@@ -162,7 +167,13 @@ public virtual async Task StartAsync(string command, string[] args, Cancellation
162167
// Enable experimental stacks feature
163168
if (EnableCallStacksFeature)
164169
{
165-
_adapter.Environment.Add(ExperimentalFlags.Feature_CallStacks, "true");
170+
_adapter.Environment.Add(ExperimentalFlags.Feature_CallStacks, ToolIdentifiers.EnvVarEnabledValue);
171+
}
172+
173+
// Enable experimental parameter capturing feature
174+
if (EnableParameterCapturingFeature)
175+
{
176+
_adapter.Environment.Add(ExperimentalFlags.Feature_ParameterCapturing, ToolIdentifiers.EnvVarEnabledValue);
166177
}
167178

168179
// Ensures that the TestStartupHook is loaded early so it helps resolve other test assemblies

0 commit comments

Comments
 (0)