Skip to content

Commit

Permalink
fix: Fix bug in distributed tracing when excludeNewrelicHeader is s…
Browse files Browse the repository at this point in the history
…et to true (#2457)

* fix: Fix exception thrown when disabling newrelic header with DT

* Exclude one supportability metric from assertions if NR header is excluded

Also tweak names for understandability

---------

Co-authored-by: chynesNR <chynes@newrelic.com>
  • Loading branch information
nr-ahemsath and chynesNR authored Apr 30, 2024
1 parent f097d37 commit 1f95c9c
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ public void InsertDistributedTraceHeaders<T>(IInternalTransaction transaction, T
setter(carrier, Constants.DistributedTracePayloadKeyAllLower, distributedTracePayload);
}
}
else
{
transaction.SetSampled(_adaptiveSampler);
}

var createOutboundTraceContextHeadersSuccess = false;
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,25 @@ public void SetOrDeleteDistributedTraceEnabled(bool? enabled)
}
}

/// <summary>
/// Sets or deletes the excludeNewrelicHeader setting in the newrelic.config.
/// </summary>
/// <param name="enabled">If null, the setting will be deleted; otherwise, the setting will be set to the value of this parameter.</param>
public void SetOrDeleteDistributedTraceExcludeNewRelicHeader(bool? exclude)
{
const string config = "configuration";
const string distributedTracing = "distributedTracing";
if (null == exclude)
{
CommonUtils.DeleteXmlNodeFromNewRelicConfig(_configFilePath, new[] { config }, distributedTracing);
}
else
{
CommonUtils.ModifyOrCreateXmlAttributeInNewRelicConfig(_configFilePath, new[] { config, distributedTracing },
"excludeNewrelicHeader", exclude.Value ? "true" : "false");
}
}

public NewRelicConfigModifier SetAllowAllHeaders(bool? enabled)
{
const string config = "configuration";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace NewRelic.Agent.IntegrationTests.DistributedTracing.W3CInstrumentationT
/// Instrumentations occur in this test are AspNetCore and HttpClient.
/// </summary>
[NetCoreTest]
public class HttpClientW3CTestsNetCore : NewRelicIntegrationTest<AspNetCoreDistTraceRequestChainFixture>
public abstract class HttpClientW3CTestsNetCore : NewRelicIntegrationTest<AspNetCoreDistTraceRequestChainFixture>
{
private readonly AspNetCoreDistTraceRequestChainFixture _fixture;

Expand All @@ -30,11 +30,12 @@ public class HttpClientW3CTestsNetCore : NewRelicIntegrationTest<AspNetCoreDistT
private const string TestTracingVendors = "rojo,congo";
private const string TestOtherVendorEntries = "rojo=1,congo=2";

public HttpClientW3CTestsNetCore(AspNetCoreDistTraceRequestChainFixture fixture, ITestOutputHelper output)
public HttpClientW3CTestsNetCore(AspNetCoreDistTraceRequestChainFixture fixture, ITestOutputHelper output, bool excludeNewRelicHeader)
: base(fixture)
{
_fixture = fixture;
_fixture.TestLogger = output;
_fixture.ExcludeNewRelicHeader = excludeNewRelicHeader;
_fixture.AddActions
(
exerciseApplication: () =>
Expand All @@ -46,7 +47,6 @@ public HttpClientW3CTestsNetCore(AspNetCoreDistTraceRequestChainFixture fixture,
};

_fixture.ExecuteTraceRequestChain("CallNext", "CallNext", "CallEnd", headers);

_fixture.FirstCallAppAgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromSeconds(15), ExpectedTransactionCount);
_fixture.SecondCallAppAgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromSeconds(15), ExpectedTransactionCount);
_fixture.AgentLog.WaitForLogLines(AgentLogBase.TransactionTransformCompletedLogLineRegex, TimeSpan.FromSeconds(15), ExpectedTransactionCount);
Expand Down Expand Up @@ -120,13 +120,17 @@ public void Test()

var senderExpectedMetrics = new List<Assertions.ExpectedMetric>
{
new Assertions.ExpectedMetric { metricName = @"Supportability/DistributedTrace/CreatePayload/Success", callCount = 1 },
new Assertions.ExpectedMetric { metricName = @"Supportability/SpanEvent/TotalEventsSeen", callCount = 4 },
new Assertions.ExpectedMetric { metricName = @"Supportability/TraceContext/Accept/Success", callCount = 1 },
new Assertions.ExpectedMetric { metricName = @"Supportability/TraceContext/Create/Success", callCount = 1 },
new Assertions.ExpectedMetric { metricName = @"Supportability/TraceContext/TraceState/NoNrEntry", callCount = 1 }
};

if (! _fixture.ExcludeNewRelicHeader)
{
senderExpectedMetrics.Add(new Assertions.ExpectedMetric { metricName = @"Supportability/DistributedTrace/CreatePayload/Success", callCount = 1 });
}

var accountId = _fixture.SecondCallAppAgentLog.GetAccountId();
var appId = _fixture.SecondCallAppAgentLog.GetApplicationId();

Expand Down Expand Up @@ -157,4 +161,22 @@ public void Test()
);
}
}

public class HttpClientW3CTestsNetCoreWithNRHeader : HttpClientW3CTestsNetCore
{
public HttpClientW3CTestsNetCoreWithNRHeader(AspNetCoreDistTraceRequestChainFixture fixture, ITestOutputHelper output)
: base(fixture, output, excludeNewRelicHeader: false)
{
}

}

public class HttpClientW3CTestsNetCoreWithoutNRHeader : HttpClientW3CTestsNetCore
{
public HttpClientW3CTestsNetCoreWithoutNRHeader(AspNetCoreDistTraceRequestChainFixture fixture, ITestOutputHelper output)
: base(fixture, output, excludeNewRelicHeader: true)
{
}
}

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0


using System;
using System.Collections.Generic;
using NewRelic.Agent.IntegrationTestHelpers;
Expand All @@ -18,6 +17,8 @@ public class AspNetCoreDistTraceRequestChainFixture : RemoteApplicationFixture
public RemoteService FirstCallApplication { get; set; }
public RemoteService SecondCallApplication { get; set; }

public bool ExcludeNewRelicHeader = false;

private AgentLogFile _firstCallAppAgentLog;
private AgentLogFile _secondCallAppAgentLog;

Expand All @@ -42,8 +43,8 @@ public AspNetCoreDistTraceRequestChainFixture()
configModifier.SetLogLevel("all");

//Do during setup so TestLogger is set.
FirstCallApplication = SetupDistributedTracingApplication();
SecondCallApplication = SetupDistributedTracingApplication();
FirstCallApplication = SetupDistributedTracingApplication(ExcludeNewRelicHeader);
SecondCallApplication = SetupDistributedTracingApplication(ExcludeNewRelicHeader);

var environmentVariables = new Dictionary<string, string>();

Expand Down Expand Up @@ -74,7 +75,7 @@ public void ExecuteTraceRequestChain(string firstAppAction, string secondAppActi
}
}

protected RemoteService SetupDistributedTracingApplication()
protected RemoteService SetupDistributedTracingApplication(bool excludeNewRelicHeader)
{
var service = new RemoteService(
ApplicationDirectoryName,
Expand All @@ -93,6 +94,7 @@ protected RemoteService SetupDistributedTracingApplication()
var configModifier = new NewRelicConfigModifier(service.DestinationNewRelicConfigFilePath);
configModifier.SetOrDeleteDistributedTraceEnabled(true);
configModifier.SetOrDeleteSpanEventsEnabled(true);
configModifier.SetOrDeleteDistributedTraceExcludeNewRelicHeader(excludeNewRelicHeader);
configModifier.SetLogLevel("all");

return service;
Expand Down

0 comments on commit 1f95c9c

Please sign in to comment.