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

Sentry does not get rest api title correct on performance metric transaction when using Microsoft.AspNetCore.Mvc.Versioning #1637

Closed
tlf30 opened this issue May 3, 2022 · 4 comments · Fixed by #1731
Assignees
Labels
Bug Something isn't working

Comments

@tlf30
Copy link

tlf30 commented May 3, 2022

Package

Sentry.AspNetCore

.NET Flavor

.NET Core

.NET Version

6.0.4

OS

Windows

SDK Version

3.16.0

Self-Hosted Sentry Version

22.2.0

Steps to Reproduce

  1. Create a controller using Microsoft.AspNetCore.Mvc.Versioning
    Example:
[Authorize]
[ApiController]
[ApiVersion("1")]
[Route("api/v{version:apiVersion}/[controller]")]
[Produces("application/json")]
public class AuthController : Controller 
{
    [MapToApiVersion("1")]
    [HttpPost]
    [AllowAnonymous]
    [Route("Login")]
    [ProducesResponseType(typeof(AuthResponse), StatusCodes.Status200OK)]
    [ProducesResponseType(typeof(AuthDeniedResponse), StatusCodes.Status401Unauthorized)]
    public IActionResult Login()
    {
        return Ok(); //TODO: Do something
    }
}
  1. Perform request against endpoint
    image

Expected Result

It is expected that the correct version is inserted into the request metadata sent back to the server.

Actual Result

image

@tlf30 tlf30 added Bug Something isn't working Platform: .NET labels May 3, 2022
@tlf30 tlf30 changed the title Sentry does not get rest api title correct on performance metrics when using Microsoft.AspNetCore.Mvc.Versioning Sentry does not get rest api title correct on performance metric transaction when using Microsoft.AspNetCore.Mvc.Versioning May 3, 2022
@mattjohnsonpint
Copy link
Contributor

Thanks for raising this. I belive @lucas-zimerman might have some input on this one?

@lucas-zimerman
Copy link
Collaborator

Thanks for raising this. I belive @lucas-zimerman might have some input on this one?

It has been like that for a while.
One way this could be fixed is by using the IApiVersioningFeature to retrieve the correct version number and replace the {version:ApiVersoin}, but that will generate a new group of transactions to the users once they update the SDK with this feature.

@tlf30
Copy link
Author

tlf30 commented May 11, 2022

Perhaps it would be possible to make it configurable from the application using sentry options?
In my use case, it would be nice to have the version id populated to create separated transaction groups, but I can also understand where others would wish to keep the existing behavior, and/or have the desire to keep their existing transaction groups. Perhaps the default can be to keep the existing behavior with a way to enable resolving the api version as an option.

I have not looked under the hood of api versioning, but I am aware that for dotnet 6, some things with the api version have changed and it is now no longer a Microsoft project, more info here: dotnet/aspnet-api-versioning#808. And currently there is no release yet for dotnet 6, yet many people (including myself) are using the 5.x version for dotnet 6 without issues. Just something to keep in mind if adding integration for.

Perhaps if adding integration, it could be included in a separate package, this would allow for not including the api versioning as a dependency for projects that do not use it. It could also be the means for configuring that you would like to resolve the api version. The old behavior would be maintained if the dependency is not included, but with the dependency then the api version would be resolved. Just another idea for how to address the issue.

Thank you,
Trevor

@mattjohnsonpint
Copy link
Contributor

Yes, this may be considered a breaking behavioral change, but my thinking is this:

  • If you're not using Microsoft.AspNetCore.Mvc.Versioning, then there's not breaking change for you.
  • If you are using, then you probably want this bug to be fixed, even if that affects your groupings in Sentry.

So, let's get it fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants