The Wayback Machine - https://web.archive.org/web/20220209182430/https://github.com/dotnet/aspnetcore/issues/39792
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

Jump to bottom

Improve endpoint metadata debugging by overriding ToString #39792

Open
JamesNK opened this issue Jan 26, 2022 · 5 comments
Open

Improve endpoint metadata debugging by overriding ToString #39792

JamesNK opened this issue Jan 26, 2022 · 5 comments
Labels
api-approved api-suggestion area-runtime
Milestone
.NET 7 Planning

Comments

@JamesNK
Copy link
Member

@JamesNK JamesNK commented Jan 26, 2022 •
edited

Background and Motivation

It is useful to understand what behavior endpoint metadata is adding to an endpoint. A human-readable summary of metadata would make this much easier. It can be viewed when debugging, and in logs when metadata is logged.

#34604 is another potential use of human-readable endpoint metadata.

Proposed API

I propose overriding ToString on metadata types. See changes in #35231 and #39753 as examples.

override Microsoft.AspNetCore.Routing.DataTokensMetadata.ToString() -> string!
override Microsoft.AspNetCore.Routing.EndpointNameMetadata.ToString() -> string!
override Microsoft.AspNetCore.Routing.HostAttribute.ToString() -> string!
override Microsoft.AspNetCore.Routing.HttpMethodMetadata.ToString() -> string!
override Microsoft.AspNetCore.Routing.RouteNameMetadata.ToString() -> string!
override Microsoft.AspNetCore.Routing.SuppressLinkGenerationMetadata.ToString() -> string!
override Microsoft.AspNetCore.Routing.SuppressMatchingMetadata.ToString() -> string!

Alternative Designs

An alternative approach is to add an interface to get display information:

public interface IEndpointDebugText
{
    string DebugText { get; }
}

I don't think this adds anything. We already have ToString that is pretty much for this situation.

DebuggerDisplayAttribute is not appropriate because there are situations where it is useful to get the debug value at runtime. e.g. logging. It is only useful if we provide debug text using something other than ToString.

Risks

Someone uses endpoint metadata ToString to drive application behavior, and we change the ToString value. I don't think this is a valid concern. Someone could depend on text in exception messages and logging, and those change from time to time.
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Jan 26, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@adityamandaleeka adityamandaleeka added this to the .NET 7 Planning milestone Jan 26, 2022
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Jan 26, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 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.

@davidfowl
Copy link
Member

@davidfowl davidfowl commented Jan 26, 2022

Where does this show up to the user or when debugging?

@JamesNK
Copy link
Member Author

@JamesNK JamesNK commented Jan 26, 2022 •
edited 

endpoints.MapGet("/", c =>
{
    var endpoint = c.GetEndpoint();

    var metadata = endpoint.Metadata; // view all metadata
    var nameMetadata = metadata.GetMetadata<EndpointNameMetadata>(); // view one metadata
}

@halter73 halter73 mentioned this issue Jan 31, 2022
Merged
4 tasks
@pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Feb 7, 2022

API review: API approved. We're happy to consider using ToString instead of alternate ways such as interface implementation. We do not guarantee that the returned string would be consistent across releases. We should try and make the strings appear similar without dictating a serialization format. Legibility is the primary motive here.

@pranavkm pranavkm mentioned this issue Feb 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api-approved api-suggestion area-runtime
Projects
None yet
Milestone
.NET 7 Planning
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@davidfowl @pranavkm @adityamandaleeka @JamesNK