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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃 Strange behaviour of the logger when adding tag "method" TagMethod into formatting string #1609

Closed
mihailstefanov opened this issue Oct 31, 2021 · 5 comments 路 Fixed by #1645

Comments

@mihailstefanov
Copy link

v 2.21.0

Adding tag "method" TagMethod in custom format, force using of the default formatting and completely ignoring custom one

Magic is in a function

func validCustomFormat(format string) bool {

checking for 2 keywords one of them is "method", used in configDefault
if validCustomFormat(cfg.Format) && cfg.Output == nil {

A decision is made to enable colors in configuration.

cfg.enableColors = true

Which is the root of the problem because same flag is used in decision, default formatting to be used. 馃槼

if cfg.enableColors {

Code snippet

package main

import (
    "github.com/gofiber/fiber/v2"
    "github.com/gofiber/fiber/v2/middleware/logger"
)
func main() {
  app := fiber.New()

  // Steps to reproduce
  app.Use(logger.New(logger.Config{
      Format:     "[${ip}]:${port} ${status} - ${method} ${path}\n",
  }))
  log.Fatal(app.Listen(":3000"))
}
@welcome
Copy link

welcome bot commented Oct 31, 2021

Thanks for opening your first issue here! 馃帀 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

@amalshaji
Copy link
Contributor

interesting. I will look into it.

@amalshaji
Copy link
Contributor

Previous solution to fixing the custom color issue was okay, but did not take into account how the coloring was done in the background. Here, if colors are enabled, it colors the default format. Because the previous fix did not account for the custom template.

Solution is to make changes to this switch tag

if validCustomFormat(cfg.Format) && cfg.Output == nil {
    cfg.enableColors = 0
}
if cfg.Format == "" && cfg.Output == nil {
    cfg.enableColors = 1
}

Line 177 becomes,

if cfg.enableColors == 1 {
    ....
}

Line 240 becomes,

case TagStatus:
    if cfg.enableColors == 0 {
        resp := fmt.Sprintf("%s %3d %s", statusColor(c.Response().StatusCode()), c.Response().StatusCode(), cReset)
        return appendInt(buf, resp)
    }
    return appendInt(buf, c.Response().StatusCode())

Similar change to Line 246

A lot of changes here. There's also a case of tests. What do you think?

@mihailstefanov
Copy link
Author

mihailstefanov commented Nov 4, 2021

Probably question is not for me, but I think that flag cfg.enableColors has no place in configuration and the way it is tested (cfg.Format == "" && cfg.Output == nil) does not correspond to it's name. Maybe there is a need for deeper refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants