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

💄 feat(middleware/logger): ip match gin-gonic/gin formatter #2610

Closed
wants to merge 2 commits into from
Closed

💄 feat(middleware/logger): ip match gin-gonic/gin formatter #2610

wants to merge 2 commits into from

Conversation

raulaguila
Copy link

Description

TagIP to match gin-gonic/gin defaultLogFormatter, specifically %15v instead of %v.

With %v:

| 127.0.0.1 |
| 192.168.1.10 |
| 105.103.142.227 |

With %15v, like gin https://github.com/gin-gonic/gin/blob/62b50cfbc0de877207ff74c160a23dff6394f563/logger.go#L143:

|       127.0.0.1 |
|    192.168.1.10 |
| 105.103.142.227 |

And, adds Test_Logger_LenTagIP.

Type of change

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@welcome
Copy link

welcome bot commented Aug 30, 2023

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

is it also working with ipv6 ?

@gaby
Copy link
Member

gaby commented Sep 9, 2023

@raulaguila See the comment above

@raulaguila
Copy link
Author

Tested only with IPv4.

@gaby
Copy link
Member

gaby commented Sep 10, 2023

@raulaguila The unit-tests are failing here:

=== CONT  Test_Logger_All
    logger_test.go:206: 
        Test:       Test_Logger_All
        Trace:      logger_test.go:206
        Expect:     2226Host=example.comhttp0.0.0.0example.com/?foo=bar/Cannot GET /             (string)
        Result:     2226Host=example.comhttp        0.0.0.0example.com/?foo=bar/Cannot GET /     (string)

@ReneWerner87
Copy link
Member

Tested only with IPv4.

Pls also consider this and add unittests for it

@raulaguila raulaguila closed this Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants