dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.4k stars 10k forks source link

asp.net mvc returns wrong trace id in response body #44679

Open VladislavMorozovIDT opened 2 years ago

VladislavMorozovIDT commented 2 years ago

Is there an existing issue for this?

Describe the bug

.net returnsActivity.Current?.Id as trace id. I think it isn't correct. .net should return Activity.Current?.TraceId. https://www.w3.org/TR/trace-context/#trace-id Other 3rd parties such as logs, new relic use trace id(Activity.Current.TraceId), not Activity.Current.Id. https://github.com/dotnet/aspnetcore/blob/e543c705e9e691fc2cf8c0b014f186480c291844/src/Mvc/Mvc.Core/src/Infrastructure/DefaultProblemDetailsFactory.cs#L95

Expected Behavior

Returns Activity.Current?.TraceId as traceId

Steps To Reproduce

Throw some MVC validation error

Exceptions (if any)

No response

.NET Version

6

Anything else?

No response

VladislavMorozovIDT commented 2 years ago

PR - https://github.com/dotnet/aspnetcore/pull/44680

amcasey commented 1 year ago

Triage: PR is under review

captainsafia commented 1 year ago

Triage: The PR associated with this issue came up during triage. We discussed whether it makes sense to modify the trace IDs on HttpContext itself instead of changing the behavior on the surface of problem details. It seems like that would make things more consistent across the stack.

cc: @halter73

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

adityamandaleeka commented 1 year ago

Assigning @halter73 and moving to .NET 8 Planning to follow up on this as discussed last week.

amcasey commented 8 months ago

Looks like the associated PR has been closed.