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

[target-allocator] Use gin in release mode and without default logger middleware #1414

Merged
merged 2 commits into from
Feb 8, 2023

Conversation

matej-g
Copy link
Contributor

@matej-g matej-g commented Jan 31, 2023

Signed-off-by: Matej Gera [email protected]

This is a follow-up to #1383.

This PR changes the gin router configuration to be suitable for production use, namely:

  • It sets gin to be used in release mode (to my surprise, the default was debug)
  • it opts out of using the default logger middleware. The logging seems rather verbose (logging each request) and the middleware is not using logging format that is consistent with logging format of the rest of the components in target-allocator.

Eventually, it would be nice if we introduced our own logging middleware with formatting consistent with other components, but for now the default one seems noisy and not too useful to me.

cc @kristinapathak

@matej-g matej-g marked this pull request as ready for review January 31, 2023 09:57
@matej-g matej-g requested review from a team January 31, 2023 09:57
@matej-g
Copy link
Contributor Author

matej-g commented Jan 31, 2023

Is the issue number mandatory in changelog? 🤔

@frzifus
Copy link
Member

frzifus commented Jan 31, 2023

Is the issue number mandatory in changelog? thinking

yes :D

Signed-off-by: Matej Gera <[email protected]>
@matej-g matej-g force-pushed the adjust-gin-configuration branch from 5687302 to a536862 Compare January 31, 2023 10:05
@matej-g
Copy link
Contributor Author

matej-g commented Jan 31, 2023

I did not create one but I guess it still should be part of #1352, so linking it to that one. Thanks @frzifus 😊.

@frzifus
Copy link
Member

frzifus commented Jan 31, 2023

makes sense to me :)

@matej-g matej-g mentioned this pull request Feb 8, 2023
@pavolloffay pavolloffay merged commit 55eaffc into open-telemetry:main Feb 8, 2023
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
…er middleware (open-telemetry#1414)

* Use gin in release mode and without default logger middleware

Signed-off-by: Matej Gera <[email protected]>

* Add changelog

Signed-off-by: Matej Gera <[email protected]>

---------

Signed-off-by: Matej Gera <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants