Skip to content

Commit

Permalink
summary: ASP.NET Core 6+ Browser Injection Bug Fixes
Browse files Browse the repository at this point in the history
fix: Bypass gRPC requests in the browser injection middleware. Also added a configuration setting to disable ASP.NET Core 6+ browser injection; the setting enables browser injection by default but can be overridden if necessary.
  • Loading branch information
tippmar-nr committed Nov 9, 2023
1 parent 322a00e commit e571ac1
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 20 deletions.
Expand Up @@ -2085,7 +2085,6 @@ private void UpdateDiagnosticsAgentTimingSettings()
_diagnosticsCaptureAgentTimingFrequency = configFreq;
}


private bool? _forceSynchronousTimingCalculationHttpClient;
public bool ForceSynchronousTimingCalculationHttpClient
{
Expand All @@ -2097,6 +2096,17 @@ public bool ForceSynchronousTimingCalculationHttpClient
}
}

private bool? _enableAspNetCore6PlusBrowserInjection;
public bool EnableAspNetCore6PlusBrowserInjection
{
get
{
return _enableAspNetCore6PlusBrowserInjection.HasValue
? _enableAspNetCore6PlusBrowserInjection.Value
: (_enableAspNetCore6PlusBrowserInjection = TryGetAppSettingAsBoolWithDefault("EnableAspNetCore6PlusBrowserInjection", true)).Value;
}
}

private TimeSpan? _metricsHarvestCycleOverride = null;
public TimeSpan MetricsHarvestCycle
{
Expand Down
Expand Up @@ -575,6 +575,9 @@ public ReportedConfiguration(IConfiguration configuration)
[JsonProperty("agent.force_synchronous_timing_calculation_for_http_client")]
public bool ForceSynchronousTimingCalculationHttpClient => _configuration.ForceSynchronousTimingCalculationHttpClient;

[JsonProperty("agent.enable_asp_net_core_6plus_browser_injection")]
public bool EnableAspNetCore6PlusBrowserInjection => _configuration.EnableAspNetCore6PlusBrowserInjection;

[JsonProperty("agent.exclude_new_relic_header")]
public bool ExcludeNewrelicHeader => _configuration.ExcludeNewrelicHeader;

Expand Down
Expand Up @@ -182,6 +182,7 @@ public interface IConfiguration
string ProcessHostDisplayName { get; }
int DatabaseStatementCacheCapacity { get; }
bool ForceSynchronousTimingCalculationHttpClient { get; }
bool EnableAspNetCore6PlusBrowserInjection { get; }
bool ExcludeNewrelicHeader { get; }
bool ApplicationLoggingEnabled { get; }
bool LogMetricsCollectorEnabled { get; }
Expand Down
Expand Up @@ -29,7 +29,12 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
return builder =>
{
builder.UseMiddleware<WrapPipelineMiddleware>(_agent);
builder.UseMiddleware<BrowserInjectionMiddleware>(_agent);
// only inject the middleware if browser injection is enabled and the request is not a gRPC request.
builder.UseWhen(
context => _agent.Configuration.BrowserMonitoringAutoInstrument && _agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request.ContentType?.ToLower() != "application/grpc",
b => b.UseMiddleware<BrowserInjectionMiddleware>(_agent));
next(builder);
};
}
Expand Down
Expand Up @@ -42,16 +42,23 @@ public AfterWrappedMethodDelegate BeforeWrappedMethod(InstrumentedMethodCall ins
{
return Delegates.GetDelegateFor(onSuccess: () =>
{
// Wrap _compressionStream and replace the current value with our wrapped version
// check whether we've already wrapped the stream so we don't do it twice
var currentCompressionStream = _compressionStreamFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
if (currentCompressionStream != null && currentCompressionStream.GetType() != typeof(BrowserInjectingStreamWrapper))
var context = _contextFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
// only wrap the compression stream if browser injection is enabled and the request is not a gRPC request.
if (agent.Configuration.BrowserMonitoringAutoInstrument && agent.Configuration.EnableAspNetCore6PlusBrowserInjection && context.Request.ContentType?.ToLower() != "application/grpc")
{
var context = _contextFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
// Wrap _compressionStream and replace the current value with our wrapped version
// check whether we've already wrapped the stream so we don't do it twice
var currentCompressionStream = _compressionStreamFieldGetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget);
if (currentCompressionStream != null && currentCompressionStream.GetType() != typeof(BrowserInjectingStreamWrapper))
{
var responseWrapper = new BrowserInjectingStreamWrapper(agent, currentCompressionStream, context);
var responseWrapper = new BrowserInjectingStreamWrapper(agent, currentCompressionStream, context);
_compressionStreamFieldSetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget, responseWrapper);
_compressionStreamFieldSetter.Invoke(instrumentedMethodCall.MethodCall.InvocationTarget,
responseWrapper);
}
}
});
}
Expand Down
Expand Up @@ -405,5 +405,14 @@ public NewRelicConfigModifier ConfigureFasterUpdateLoadedModulesCycle(int second
CommonUtils.SetConfigAppSetting(_configFilePath, "OverrideUpdateLoadedModulesCycle", seconds.ToString(), "urn:newrelic-config");
return this;
}

public NewRelicConfigModifier EnableAspNetCore6PlusBrowserInjection(bool enableBrowserInjection)
{
CommonUtils.ModifyOrCreateXmlNodeInNewRelicConfig(_configFilePath, new[] { "configuration" }, "appSettings", string.Empty);
CommonUtils.ModifyOrCreateXmlNodeInNewRelicConfig(_configFilePath, new[] { "configuration", "appSettings" }, "add", string.Empty);
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(_configFilePath, new[] { "configuration", "appSettings", "add"}, "key", "EnableAspNetCore6PlusBrowserInjection");
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(_configFilePath, new[] { "configuration", "appSettings", "add"}, "value", $"{enableBrowserInjection}");
return this;
}
}
}
Expand Up @@ -15,11 +15,14 @@ public abstract class BrowserAgentAutoInjection6PlusBase : NewRelicIntegrationTe
private string _htmlContent;
private string _staticContent;
private string _fooContent;
private bool _browserInjectionEnabled;

protected BrowserAgentAutoInjection6PlusBase(BasicAspNetCoreRazorApplicationFixture fixture,
ITestOutputHelper output, bool enableResponseCompression, string loaderType = "rum")
ITestOutputHelper output, bool enableBrowserInjection, bool enableResponseCompression, string loaderType = "rum")
: base(fixture)
{
_browserInjectionEnabled = enableBrowserInjection;

_fixture = fixture;
_fixture.TestLogger = output;
_fixture.Actions
Expand All @@ -31,6 +34,7 @@ public abstract class BrowserAgentAutoInjection6PlusBase : NewRelicIntegrationTe
var configModifier = new NewRelicConfigModifier(configPath);
configModifier.AutoInstrumentBrowserMonitoring(true);
configModifier.BrowserMonitoringEnableAttributes(true);
configModifier.EnableAspNetCore6PlusBrowserInjection(enableBrowserInjection);
configModifier.BrowserMonitoringLoader(loaderType);
},
Expand Down Expand Up @@ -65,9 +69,18 @@ public void Test()
var jsAgentFromStaticContent = JavaScriptAgent.GetJavaScriptAgentScriptFromSource(_staticContent);
var jsAgentFromFooContent = JavaScriptAgent.GetJavaScriptAgentScriptFromSource(_fooContent);

Assert.Equal(jsAgentFromConnectResponse, jsAgentFromHtmlContent);
Assert.Equal(jsAgentFromConnectResponse, jsAgentFromStaticContent);
Assert.Null(jsAgentFromFooContent);
if (_browserInjectionEnabled)
{
Assert.Equal(jsAgentFromConnectResponse, jsAgentFromHtmlContent);
Assert.Equal(jsAgentFromConnectResponse, jsAgentFromStaticContent);
Assert.Null(jsAgentFromFooContent);
}
else
{
Assert.Null(jsAgentFromHtmlContent);
Assert.Null(jsAgentFromStaticContent);
Assert.Null(jsAgentFromFooContent);
}

// verify that the browser injecting stream wrapper didn't catch an exception and disable itself
var agentDisabledLogLine = _fixture.AgentLog.TryGetLogLine(AgentLogBase.ErrorLogLinePrefixRegex + "Unexpected exception. Browser injection will be disabled. *?");
Expand All @@ -79,7 +92,7 @@ public void Test()
public class BrowserAgentAutoInjection6PlusRumUnCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusRumUnCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, false)
: base(fixture, output, true, false)
{
}
}
Expand All @@ -88,17 +101,34 @@ public BrowserAgentAutoInjection6PlusRumUnCompressed(BasicAspNetCoreRazorApplica
public class BrowserAgentAutoInjection6PlusRumCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusRumCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true)
: base(fixture, output, true, true)
{
}
}

[NetCoreTest]
public class BrowserAgentAutoInjection6PlusInjectionDisabledRumUnCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusInjectionDisabledRumUnCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true, false)
{
}
}

[NetCoreTest]
public class BrowserAgentAutoInjection6PlusInjectionDisabledRumCompressed : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusInjectionDisabledRumCompressed(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, false, true)
{
}
}

[NetCoreTest]
public class BrowserAgentAutoInjection6PlusSpa : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusSpa(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true, "spa")
: base(fixture, output, true, true, "spa")
{
}
}
Expand All @@ -107,7 +137,7 @@ public BrowserAgentAutoInjection6PlusSpa(BasicAspNetCoreRazorApplicationFixture
public class BrowserAgentAutoInjection6PlusFull : BrowserAgentAutoInjection6PlusBase
{
public BrowserAgentAutoInjection6PlusFull(BasicAspNetCoreRazorApplicationFixture fixture, ITestOutputHelper output)
: base(fixture, output, true, "full")
: base(fixture, output, true, true, "full")
{
}
}
Expand Down
Expand Up @@ -3241,6 +3241,18 @@ public bool AsyncHttpClientSegmentsDoNotCountTowardsParentExclusiveTimeTests(str
return defaultConfig.ForceSynchronousTimingCalculationHttpClient;
}

[TestCase(null, ExpectedResult = true)]
[TestCase("not a bool", ExpectedResult = true)]
[TestCase("false", ExpectedResult = false)]
[TestCase("true", ExpectedResult = true)]
public bool AspNetCore6PlusBrowserInjectionTests(string localConfigValue)
{
_localConfig.appSettings.Add(new configurationAdd { key = "EnableAspNetCore6PlusBrowserInjection", value = localConfigValue });
var defaultConfig = new TestableDefaultConfiguration(_environment, _localConfig, _serverConfig, _runTimeConfig, _securityPoliciesConfiguration, _processStatic, _httpRuntimeStatic, _configurationManagerStatic, _dnsStatic);

return defaultConfig.EnableAspNetCore6PlusBrowserInjection;
}

[TestCase("true", true, ExpectedResult = true)]
[TestCase("true", false, ExpectedResult = true)]
[TestCase("true", null, ExpectedResult = true)]
Expand Down

0 comments on commit e571ac1

Please sign in to comment.