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

🚀 [Feature]: middleware/logger: use Logger interface #2737

Open
3 tasks done
folliehiyuki opened this issue Nov 21, 2023 · 6 comments · May be fixed by #3153
Open
3 tasks done

🚀 [Feature]: middleware/logger: use Logger interface #2737

folliehiyuki opened this issue Nov 21, 2023 · 6 comments · May be fixed by #3153

Comments

@folliehiyuki
Copy link

Feature Description

With the introduction of Logger interface (#2499), it is easier now to plug my own log library into Fiber. With that said, logger middleware still uses fmt.Sprintf().

It would be nice if logger middleware is extended to accept logging via Logger interface. This creates an easier method to have structure log for requests/responses without the need of making a custom external middleware. Judging from #1293 (comment), if we want to keep the current logger middleware as simple as it is, making a new logger middleware would also be reasonable.

Additional Context (optional)

No response

Code Snippet (optional)

No response

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have checked for existing issues that describe my suggestion prior to opening this one.
  • I understand that improperly formatted feature requests may be closed without explanation.
Copy link

welcome bot commented Nov 21, 2023

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

@efectn
Copy link
Member

efectn commented Nov 21, 2023

Hi. Can you provide a proof-of-concept? If we can do it without breaking something on v2, it's OK to me. Otherwise, we have to adjust logger middleware on v3.

@ReneWerner87
Copy link
Member

@folliehiyuki

@folliehiyuki
Copy link
Author

Can you provide a proof-of-concept? If we can do it without breaking something on v2, it's OK to me

I don't have a concrete idea how to do this cleanly right now.

Otherwise, we have to adjust logger middleware on v3.

That sounds reasonable to me. It's nothing urgent (at least for me) and also takes less effort (no need to keep the compatibility with the current template interface).

@haochunchang
Copy link

Hi, I would like to contribute this task as my first contribution to gofiber.

From what I read about the logger middleware and the Logger interface,
My first thought is that the target can be:
"making the logger.Config struct’s LoggerFunc field to accept the Logger interface."

This could include some modifications on the defaultLoggerInstance in the logger middleware.
If there is anything I need to be aware of, please feel free to tell me.
Thanks!

@efectn
Copy link
Member

efectn commented Sep 30, 2024

Hi, I would like to contribute this task as my first contribution to gofiber.

From what I read about the logger middleware and the Logger interface, My first thought is that the target can be: "making the logger.Config struct’s LoggerFunc field to accept the Logger interface."

This could include some modifications on the defaultLoggerInstance in the logger middleware. If there is anything I need to be aware of, please feel free to tell me. Thanks!

Hi @haochunchang feel free to create a PR

@haochunchang haochunchang linked a pull request Oct 2, 2024 that will close this issue
11 tasks
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.

6 participants