Skip to content

Commit

Permalink
Refactor implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
@stevejgordon
stevejgordon committed Apr 16, 2024
1 parent ac343a3 commit d2a2eb1
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 98 deletions.
21 changes: 21 additions & 0 deletions src/Elastic.Apm/Model/Transaction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,28 @@ public void End()
}

if (IsSampled || _apmServerInfo?.Version < new ElasticVersion(8, 0, 0, string.Empty))
{
// Apply any transaction name groups
if (Configuration.TransactionNameGroups.Count > 0)
{
var matched = WildcardMatcher.AnyMatch(Configuration.TransactionNameGroups, Name, null);
if (matched is not null)
{
var matchedTransactionNameGroup = matched.GetMatcher();

if (!string.IsNullOrEmpty(matchedTransactionNameGroup))
{
_logger?.Trace()?.Log("Transaction name '{TransactionName}' matched transaction " +
"name group '{TransactionNameGroup}' from configuration",
Name, matchedTransactionNameGroup);

Name = matchedTransactionNameGroup;
}
}
}

_sender.QueueTransaction(this);
}
else
{
_logger?.Debug()
Expand Down
6 changes: 5 additions & 1 deletion src/integrations/Elastic.Apm.AspNetCore/WebRequestTransactionCreator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ internal static ITransaction StartTransactionAsync(HttpContext context, IApmLogg
return null;
}

// For completeness we set the initial transaction name based on the config.
// I don't believe there are any valid scenarios where this will not be overwritten later.
ITransaction transaction;
var transactionName = $"{context.Request.Method} {context.Request.Path}";
var transactionName = configuration?.UsePathAsTransactionName ?? ConfigConsts.DefaultValues.UsePathAsTransactionName
? $"{context.Request.Method} {context.Request.Path}"
: $"{context.Request.Method} unknown route";

var containsTraceParentHeader =
context.Request.Headers.TryGetValue(TraceContext.TraceParentHeaderName, out var traceParentHeader);
Expand Down
162 changes: 67 additions & 95 deletions src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Data.SqlClient;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Security.Claims;
Expand All @@ -20,6 +21,7 @@
using Elastic.Apm.Logging;
using Elastic.Apm.Model;
using Elastic.Apm.Reflection;
using Environment = System.Environment;
using TraceContext = Elastic.Apm.DistributedTracing.TraceContext;

namespace Elastic.Apm.AspNetFullFramework
Expand Down Expand Up @@ -506,130 +508,100 @@ private void ProcessEndRequest(object sender)

var response = context.Response;

var realTransaction = transaction as Transaction;

// Update the transaction name based on transaction name groups, route values or SOAP headers, if applicable.
if (realTransaction is not null && !realTransaction.HasCustomName)
// update the transaction name based on route values, if applicable
if (transaction is Transaction t && !t.HasCustomName)
{
var transactionRenamed = false;

// Attempt to match the URL against a configured transaction name group.
// This takes precedence over further attempts at renaming if a match is found.
if (Agent.Instance.Configuration.TransactionNameGroups.Count > 0)
var values = request.RequestContext?.RouteData?.Values;
if (values?.Count > 0)
{
var matched = WildcardMatcher.AnyMatch(Agent.Instance.Configuration.TransactionNameGroups, request.Unvalidated.Path, null);
if (matched is not null)
// Determine if the route data *actually* routed to a controller action or not i.e.
// we need to differentiate between
// 1. route data that didn't route to a controller action and returned a 404
// 2. route data that did route to a controller action, and the action result returned a 404
//
// In normal MVC setup, the former will set a HttpException with a 404 status code with System.Web.Mvc as the source.
// We need to check the source of the exception because we want to differentiate between a 404 HttpException from the
// framework and a 404 HttpException from the application.
if (context.Error is not HttpException httpException ||
httpException.Source != "System.Web.Mvc" ||
httpException.GetHttpCode() != 404)
{
var matchedTransactionNameGroup = matched.GetMatcher();
transaction.Name = $"{context.Request.HttpMethod} {matchedTransactionNameGroup}";
transactionRenamed = true;

_logger?.Trace()?.Log("Path '{Path}' matched transaction name group '{TransactionNameGroup}' from configuration",
request.Unvalidated.Path, matchedTransactionNameGroup);
}
}

if (!transactionRenamed)
{
// If we didn't match a transaction name group, attempt to rename the transaction based on route data.
var values = request.RequestContext?.RouteData?.Values;
if (values?.Count > 0)
{
// Determine if the route data *actually* routed to a controller action or not i.e.
// we need to differentiate between
// 1. route data that didn't route to a controller action and returned a 404
// 2. route data that did route to a controller action, and the action result returned a 404
//
// In normal MVC setup, the former will set a HttpException with a 404 status code with System.Web.Mvc as the source.
// We need to check the source of the exception because we want to differentiate between a 404 HttpException from the
// framework and a 404 HttpException from the application.
if (context.Error is not HttpException httpException ||
httpException.Source != "System.Web.Mvc" ||
httpException.GetHttpCode() != 404)
// handle MVC areas. The area name will be included in the DataTokens.
object area = null;
request.RequestContext?.RouteData?.DataTokens?.TryGetValue("area", out area);
IDictionary<string, object> routeData;
if (area != null)
{
// handle MVC areas. The area name will be included in the DataTokens.
object area = null;
request.RequestContext?.RouteData?.DataTokens?.TryGetValue("area", out area);
IDictionary<string, object> routeData;
if (area != null)
{
routeData = new Dictionary<string, object>(values.Count + 1);
foreach (var value in values)
routeData.Add(value.Key, value.Value);
routeData.Add("area", area);
}
else
routeData = values;
routeData = new Dictionary<string, object>(values.Count + 1);
foreach (var value in values)
routeData.Add(value.Key, value.Value);
routeData.Add("area", area);
}
else
routeData = values;

string name = null;
string name = null;

// if we're dealing with Web API attribute routing, get transaction name from the route template
if (routeData.TryGetValue("MS_SubRoutes", out var template) && _httpRouteDataInterfaceType.Value != null)
// if we're dealing with Web API attribute routing, get transaction name from the route template
if (routeData.TryGetValue("MS_SubRoutes", out var template) && _httpRouteDataInterfaceType.Value != null)
{
if (template is IEnumerable enumerable)
{
if (template is IEnumerable enumerable)
var minPrecedence = decimal.MaxValue;
var enumerator = enumerable.GetEnumerator();
while (enumerator.MoveNext())
{
var minPrecedence = decimal.MaxValue;
var enumerator = enumerable.GetEnumerator();
while (enumerator.MoveNext())
var subRoute = enumerator.Current;
if (subRoute != null && _httpRouteDataInterfaceType.Value.IsInstanceOfType(subRoute))
{
var subRoute = enumerator.Current;
if (subRoute != null && _httpRouteDataInterfaceType.Value.IsInstanceOfType(subRoute))
var precedence = _routePrecedenceGetter(subRoute);
if (precedence < minPrecedence)
{
var precedence = _routePrecedenceGetter(subRoute);
if (precedence < minPrecedence)
{
_logger?.Trace()
?.Log(
$"Calculating transaction name from web api attribute routing (route precedence: {precedence})");
minPrecedence = precedence;
name = _routeDataTemplateGetter(subRoute);
}
_logger?.Trace()
?.Log(
$"Calculating transaction name from web api attribute routing (route precedence: {precedence})");
minPrecedence = precedence;
name = _routeDataTemplateGetter(subRoute);
}
}
}
}
else
{
_logger?.Trace()?.Log("Calculating transaction name based on route data");
name = Transaction.GetNameFromRouteContext(routeData);
}

if (!string.IsNullOrWhiteSpace(name))
{
transaction.Name = $"{context.Request.HttpMethod} {name}";
transactionRenamed = true;
}
}
else
{
// dealing with a 404 HttpException that came from System.Web.Mvc
_logger?.Trace()?.Log(
"Route data found but a HttpException with 404 status code was thrown " +
"from System.Web.Mvc - setting transaction name to 'unknown route'.");

transaction.Name = $"{context.Request.HttpMethod} unknown route";
transactionRenamed = true;
_logger?.Trace()?.Log("Calculating transaction name based on route data");
name = Transaction.GetNameFromRouteContext(routeData);
}
}
}

// Final attempt to rename the transaction based on the SOAP action if it's a SOAP request and we haven't already
// matched a transaction name group or route data.
if (!transactionRenamed && SoapRequest.TryExtractSoapAction(_logger, context.Request, out var soapAction))
{
if (!Agent.Instance.Configuration.UsePathAsTransactionName)
transaction.Name = $"{context.Request.HttpMethod} {request.Unvalidated.Path} {soapAction}";
if (!string.IsNullOrWhiteSpace(name))
transaction.Name = $"{context.Request.HttpMethod} {name}";
}
else
transaction.Name += $" {soapAction}";
{
// dealing with a 404 HttpException that came from System.Web.Mvc
_logger?.Trace()
?
.Log(
"Route data found but a HttpException with 404 status code was thrown from System.Web.Mvc - setting transaction name to 'unknown route");
transaction.Name = $"{context.Request.HttpMethod} unknown route";
}
}
}

transaction.Result = Transaction.StatusCodeToResult("HTTP", response.StatusCode);

var realTransaction = transaction as Transaction;
realTransaction?.SetOutcome(response.StatusCode >= 500
? Outcome.Failure
: Outcome.Success);

// Try and update transaction name with SOAP action if applicable.
if (realTransaction == null || !realTransaction.HasCustomName)
{
if (SoapRequest.TryExtractSoapAction(_logger, context.Request, out var soapAction))
transaction.Name += $" {soapAction}";
}

if (transaction.IsSampled)
{
FillSampledTransactionContextResponse(response, transaction);
Expand Down
44 changes: 44 additions & 0 deletions test/Elastic.Apm.Tests/TransactionGroupNamesTests.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to Elasticsearch B.V under
// one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using System.Linq;
using Elastic.Apm.Tests.Utilities;
using FluentAssertions;
using Xunit;

namespace Elastic.Apm.Tests;

public class TransactionGroupNamesTests
{
[Fact]
public void TransactionGroupNames_AreApplied()
{
const string groupName = "GET /customer/*";

var payloadSender = new MockPayloadSender();

using var agent = new ApmAgent(new TestAgentComponents(payloadSender: payloadSender,
configuration: new MockConfiguration(transactionNameGroups: groupName)));

agent.Tracer.CaptureTransaction("GET /order/1", "Test",
transaction => { });

payloadSender.WaitForTransactions();

payloadSender.Transactions.Count.Should().Be(1);
var transaction = payloadSender.Transactions.Single();
transaction.Name.Should().Be("GET /order/1");

payloadSender.Clear();

agent.Tracer.CaptureTransaction("GET /customer/1", "Test",
transaction => { });

payloadSender.WaitForTransactions();
payloadSender.Transactions.Count.Should().Be(1);
transaction = payloadSender.Transactions.Single();
transaction.Name.Should().Be(groupName);
}
}
9 changes: 7 additions & 2 deletions test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameGroupsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ namespace Elastic.Apm.AspNetFullFramework.Tests;
[Collection(Consts.AspNetFullFrameworkTestsCollection)]
public class TransactionNameGroupsTests : TestsBase
{
private const string GroupName = "*/myArea/*";
// NOTE: The main situation where ASP.NET instrumentation may produce high-cardinality transaction names is when the application
// is using WCF. In such cases, the transaction name will default to being the path of the request. We can't easily spin up WCF in
// CI so this test simply ensures that any transaction name group configuration is working as expected by setting a transaction
// group name that matches the transaction name of a request that hits an MVC controller action from an Area.

private const string GroupName = "GET MyArea/*";

public TransactionNameGroupsTests(ITestOutputHelper xUnitOutputHelper)
: base(xUnitOutputHelper,
Expand All @@ -36,7 +41,7 @@ await WaitAndCustomVerifyReceivedData(receivedData =>
{
receivedData.Transactions.Count.Should().Be(1);
var transaction = receivedData.Transactions.Single();
transaction.Name.Should().Be($"GET {GroupName}");
transaction.Name.Should().Be(GroupName);
});
}
}

0 comments on commit d2a2eb1

Please sign in to comment.