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

Regression with v0.29.4 and console decision logger being tied to general log level #3654

Closed
tsandall opened this issue Jul 15, 2021 · 0 comments · Fixed by #3684
Closed
Assignees
Labels

Comments

@tsandall
Copy link
Member

As recently as v0.28.0 the console decision logger was decoupled from the general log level (by design). With v0.29.4 that's no longer the case. For example, w/ v0.28.0 this works fine:

$ docker run -it --rm -p 8181:8181 openpolicyagent/opa:0.28.0 run --server --log-level=error --set=decision_logs.console=true

In another terminal run curl localhost:8181/v1/data/system/version. Observe the log below:

{"decision_id":"0d542d8f-a073-4f1c-a3c8-bbea4b39e284","labels":{"id":"f337a30d-d38b-4d7a-97ad-80660d40ee13","version":"0.28.0"},"level":"info","metrics":{"counter_server_query_cache_hit":0,"timer_rego_external_resolve_ns":300,"timer_rego_input_parse_ns":200,"timer_rego_query_compile_ns":59400,"timer_rego_query_eval_ns":17900,"timer_rego_query_parse_ns":226700,"timer_server_handler_ns":390100},"msg":"Decision Log","path":"system/version","requested_by":"172.17.0.1:42380","result":{"build_commit":"3fbcd71","build_hostname":"5c684f011e1a","build_timestamp":"2021-04-27T13:52:19Z","version":"0.28.0"},"time":"2021-07-15T18:15:27Z","timestamp":"2021-07-15T18:15:27.2753534Z","type":"openpolicyagent.org/decision_logs"}

With v0.29.4 no log gets emitted. I'm guessing the SDK changes are to blame.

@tsandall tsandall added the bug label Jul 15, 2021
@tsandall tsandall self-assigned this Jul 27, 2021
tsandall added a commit to tsandall/opa that referenced this issue Jul 27, 2021
This commit fixes the console loggers so that messages are emitted
regardless of the debug log level. The problem was that in 3fcc875 we
updated the plugins to use a console logger obtained from the plugin
manager as opposed to a global logger instantiated in the plugins
package--the console logger obtained from the plugin manager was
instantiated in the runtime package by calling
logging.NewStandardLogger. Unfortunately, logging.NewStandardLogger
does not create a new logger--it returns the global logrus
logger.

This commit fixes the issue by deprecating logging.NewStandardLogger
and introducing two new functions in the logging package:

* logging.Get() - this replaces the old logging.NewStandardLogger
  function--this function should be called to obtain the debug logger
  used throughout OPA.

* logging.New() - this actually returns a new logger that can be
  configured independently from the debug logger used throughout
  OPA.

The runtime and sdk packages have been updated to call logging.New()
to obtain console loggers and the rest of the codebase has been
updated to call logging.Get() in place of logging.NewStandardLogger().

Fixes open-policy-agent#3654

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
tsandall added a commit that referenced this issue Jul 27, 2021
This commit fixes the console loggers so that messages are emitted
regardless of the debug log level. The problem was that in 3fcc875 we
updated the plugins to use a console logger obtained from the plugin
manager as opposed to a global logger instantiated in the plugins
package--the console logger obtained from the plugin manager was
instantiated in the runtime package by calling
logging.NewStandardLogger. Unfortunately, logging.NewStandardLogger
does not create a new logger--it returns the global logrus
logger.

This commit fixes the issue by deprecating logging.NewStandardLogger
and introducing two new functions in the logging package:

* logging.Get() - this replaces the old logging.NewStandardLogger
  function--this function should be called to obtain the debug logger
  used throughout OPA.

* logging.New() - this actually returns a new logger that can be
  configured independently from the debug logger used throughout
  OPA.

The runtime and sdk packages have been updated to call logging.New()
to obtain console loggers and the rest of the codebase has been
updated to call logging.Get() in place of logging.NewStandardLogger().

Fixes #3654

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
dolevf pushed a commit to dolevf/opa that referenced this issue Nov 4, 2021
This commit fixes the console loggers so that messages are emitted
regardless of the debug log level. The problem was that in 3fcc875 we
updated the plugins to use a console logger obtained from the plugin
manager as opposed to a global logger instantiated in the plugins
package--the console logger obtained from the plugin manager was
instantiated in the runtime package by calling
logging.NewStandardLogger. Unfortunately, logging.NewStandardLogger
does not create a new logger--it returns the global logrus
logger.

This commit fixes the issue by deprecating logging.NewStandardLogger
and introducing two new functions in the logging package:

* logging.Get() - this replaces the old logging.NewStandardLogger
  function--this function should be called to obtain the debug logger
  used throughout OPA.

* logging.New() - this actually returns a new logger that can be
  configured independently from the debug logger used throughout
  OPA.

The runtime and sdk packages have been updated to call logging.New()
to obtain console loggers and the rest of the codebase has been
updated to call logging.Get() in place of logging.NewStandardLogger().

Fixes open-policy-agent#3654

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Signed-off-by: Dolev Farhi <farhi.dolev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant