Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include IIS virtual directory path in request.uri attribute of transactions in ASP.NET Core #1624

Open
tYoshiyuki opened this issue May 14, 2023 · 2 comments
Labels
community To tag external issues and PRs feature request To tag an issue after triage that is a feature instead of TD version: major To tag issues that may cause breaking changes or removal of deprecated apis/config

Comments

@tYoshiyuki
Copy link

tYoshiyuki commented May 14, 2023

Is your feature request related to a problem? Please describe.

Include IIS virtual directory path in request.uri attribute of transactions in ASP.NET Core

Feature Description

thank you always.

When using .NET Agent, I felt that there was a difference in the transaction request.uri attribute between ASP.NET Core and ASP.NET MVC (ASP.NET Web API2).

In ASP.NET MVC, the IIS virtual directory path is included in the request.uri attribute. In contrast, ASP.NET Core doesn't seem to include the IIS virtual directory path in the request.uri attribute.

I'm using both his ASP.NET MVC and ASP.NET Core IIS applications with .NET Agent, but I think it would be more natural for users to have the same request.uri attribute settings.

Evidence is provided below.

I have created two sample applications.
image

aspnet-sample : ASP.NET MVC application
Access with a URL like [http://localhost/aspnet-sample/api/values/].

aspnet-core-sample : ASP.NET Core application
Access with a URL like [http://localhost/aspnet-core-sample/weatherforecast].

NRQL execution result.
image

For ASP.NET MVC applications, the IIS virtual directory path [aspnet-sample] is included in the request.uri.

On the other hand, for ASP.NET Core applications, the client path [aspnet-core-sample] is not included in the request.uri.

Describe Alternatives

You can change the default behavior by using SetTransactionUri (.NET agent API).
https://docs.newrelic.com/jp/docs/apm/agents/net-agent/net-agent-api/set-transaction-uri/

Here's an example of setting it in ASP.NET Core middleware.
GetDisplayUrl (Microsoft.AspNetCore.Http.Extensions.UriHelper) is a method that combines PathBase and Path to create an entire URL.

// Add configure the HTTP request pipeline.
app.Use(async (context, next) =>
{
    NewRelic.Api.Agent.NewRelic.SetTransactionUri(new Uri(context.Request.GetDisplayUrl()));
    await next.Invoke();
});

I can change the safe behavior with the above additional code, but I would be very happy if it was realized as the default behavior of .NET Agent.

Additional context

It's just a guess, but I think it's because the URL path information from ASP.NET Core is stored separately in HttpRequest.PathBase and HttpRequest.Path.
https://stackoverflow.com/questions/58614864/whats-the-difference-between-httprequest-path-and-httprequest-pathbase-in-asp-n

Therefore, I think that it will be improved if you change the part that sets request.uri of transaction in .NET Agent of ASP.NET Core to HttpRequest.PathBase and HttpRequest.Path.

// WrapPipelineMiddleware.cs
private ITransaction SetupTransaction(HttpRequest request)
{
    var path = request.Path.Value; // <- This code
    path = "/".Equals(path) ? "ROOT" : path.Substring(1);

    var transaction = _agent.CreateTransaction(
        isWeb: true,
        category: EnumNameCache<WebTransactionType>.GetName(WebTransactionType.ASP),
        transactionDisplayName: path,
        doNotTrackAsUnitOfWork: true);

    transaction.SetRequestMethod(request.Method);
    transaction.SetUri(request.Path); // <- This code

    if (request.QueryString.HasValue)
    {
        var parameters = new Dictionary<string, string>();
        foreach (var keyValuePair in request.Query)
        {
            parameters.Add(keyValuePair.Key, keyValuePair.Value);
        }

        transaction.SetRequestParameters(parameters);
    }

    return transaction;
}

Priority

Really Want

@workato-integration
Copy link

@github-actions github-actions bot added the community To tag external issues and PRs label May 14, 2023
@nr-ahemsath
Copy link
Member

@tYoshiyuki: Thank you for taking the time to write up a very well documented and described feature request!

I agree in general that behavioral consistency between our ASP.NET and ASP.NET Core instrumentation would make sense. However, for what it's worth, ASP.NET Core applications aren't always IIS-hosted, and so by making this change we could end up creating behavioral inconsistency for ASP.NET Core instrumentation depending on hosting model.

At any rate, I think that making this change in request.uri attribute settings for ASP.NET Core instrumentation might be considered a breaking change by existing customers, so this would have to wait for the next major release version (11). Our product manager would make the determination of whether or not to include this work in that release.

@nr-ahemsath nr-ahemsath added version: major To tag issues that may cause breaking changes or removal of deprecated apis/config feature request To tag an issue after triage that is a feature instead of TD labels May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community To tag external issues and PRs feature request To tag an issue after triage that is a feature instead of TD version: major To tag issues that may cause breaking changes or removal of deprecated apis/config
Projects
None yet
Development

No branches or pull requests

2 participants