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

Activity Display Name: Route Template instead of path #4525

Open
elvince opened this issue May 30, 2023 · 5 comments
Open

Activity Display Name: Route Template instead of path #4525

elvince opened this issue May 30, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@elvince
Copy link

elvince commented May 30, 2023

Bug Report

We have api versioning for our endpoints which .net supports so for example: [Route("api/v{apiVersion:apiVersion}/[controller]")] which is being reported as api/v{apiVersion:apiVersion}/users

Version

.net Core 6.x

Symptom

What is the expected behavior?

api/v1/user
api/v2/user

What is the actual behavior?

api/v{apiVersion:apiVersion}/users

we have the partial route template and {apiVersion:apiVersion} is not transformed where as [Controller] is.

If it is by design, could you please tell me why you choose to not have the version in the display name?

We are using opentelemetry and the displayname is mapped to trace group. At the end, we don't know what version of our APIs had bad performance or issues as everything is aggregated.

Code

builder.Services.AddOpenTelemetry()
  .WithTracing(b =>
  {
      b
      .AddHttpClientInstrumentation()
      .AddAspNetCoreInstrumentation()
      .AddConsoleExporter();
  })
```;

Thanks,
@elvince elvince added the bug Something isn't working label May 30, 2023
@dhabierre
Copy link

Same behavior here with dotnet 7 and latest OpenTelemetry nugets.

http.flavor: 1.1 http.method: GET http.route: api/v{version:apiVersion}/Tiers/{userId} http.scheme: http http.status_code: 200 net.host.name: localhost net.host.port: 8080 Histogram

The {version:apiVersion} should be substituted because of multiple versions.
The {userId} shouldn't?

Controller:

namespace My.WebApi.Controllers.V1
{
    ...

    [ApiController]
    [Route("api/v{version:apiVersion}/[controller]")]
    [ApiVersion("1.0")]
    public sealed class TiersController : ControllerBase
    {
        ...

        [HttpGet("{userId}")]
        [ProducesResponseType(typeof(GetTiersListByUserIdResponse), StatusCodes.Status200OK)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        public async Task<IActionResult> GetTiersByUserId(int userId)
        {
            ...
        }
    }
}

Thanks.

@Want100Cookies
Copy link

This also makes it pretty hard to search traces in Jaeger. The displayName is used as operation name results in a very bloated index which can be prevented if we use the route template.

image

Instead of using the httpContext.Request.Path property here can't we use the httpContext.GetEndpoint().DisplayName property? This always contains a readable route without the actual parameter contents to my knowledge.

@Want100Cookies
Copy link

@DevOnRun
Copy link

Do we have any plan to include version number in route path like api/v1/users instead of api/v{apiVersion:apiVersion}/users?

@ScarletKuro
Copy link

Is there any workaround how to make open telemetry to show actual version instead of {apiVersion:apiVersion}? I see that #4917 was rejected.

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
None yet
Development

Successfully merging a pull request may close this issue.

5 participants