Skip to content

HTTPSysServer does not use LoggerMessage.Define #31313

Closed

Description

@davidfowl

Describe the bug

LoggerMessage.Define is the preferred approach to logging as it saves re-parsing format strings and doing dictionary lookups (and potential allocations if the cache isn't full). We should update the log calls to use it.

cc @sebastienros

Activity

  1. changed the title [-]HTTPSys server does not use LoggerMessage.Define[/-] [+]HTTPSysServer does not use LoggerMessage.Define[/+] on Mar 28, 2021
  2. martincostello commented on Mar 28, 2021

    @martincostello
    Member

    Happy to pick this up if you've not already started on it.

  3. davidfowl commented on Mar 28, 2021

    @davidfowl
    MemberAuthor

    Sounds good. Are you aware of the latest pattern we've been using? It's a nested static Log class with static methods that do logging. If the class is big enough we make a partial class and put the log methods in there m. If that's not clear, take a look at SignalR

  4. martincostello commented on Mar 28, 2021

    @martincostello
    Member

    I took a look at the repo and saw that there's a class called LoggingExtensions (example) and then the messages are defined in there.

    If you're moving to nested per-class instead I'll do it that way instead 👍

  5. davidfowl commented on Mar 28, 2021

    @davidfowl
    MemberAuthor
  6. davidfowl commented on Mar 28, 2021

    @davidfowl
    MemberAuthor

    Assigned the issue to you @martincostello

  7. added this to the 6.0-preview4 milestone on Mar 28, 2021
  8. linked a pull request that will close this issueUse LoggerMessage.Define in HttpSys #31333on Mar 28, 2021
  9. added a commit that references this issue on Mar 28, 2021
    020745c
  10. davidfowl commented on Mar 28, 2021

    @davidfowl
    MemberAuthor

    Thanks!

  11. added a commit that references this issue on Apr 10, 2021
    c07f867
  12. ghost locked as resolved and limited conversation to collaborators on Apr 28, 2021
  13. added
    area-networkingIncludes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
    and removed on Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Perfarea-networkingIncludes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractionsfeature-httpsyshelp wantedUp for grabs. We would accept a PR to help resolve this issue

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions