roles: add log_requests option

[p0rtale]

Currently, HTTP requests are logged at the default level log.info, which can create noise (e.g. for metrics endpoints).
This change allows setting custom log levels (e.g. log.verbose) for incoming requests.

Example:

roles_cfg:
  roles.httpd:
    default:
      listen: 8081
      log_requests: log.verbose
  roles.metrics-export:
    http:
    - endpoints:
      - path: /metrics
        format: prometheus

Closes TNTP-3229

[a1div0]

Нужно покрыть минимум одним тестом

[themilchenko]

Thanks for the patch! Please fix some non-critical comments:

• Amend commits into one if it is about one feature or bug. For example in your case it should be in one commit, because it is all about log_requests option.
• Prefer prefix with logical module name in commit title, so it can be something like roles: add log_requests option.
• Add CHANGELOG entry with info about added functional.

[themilchenko]

Why do we need to change default behavior?

[p0rtale]

To reduce log noise by default. Many endpoints (like metrics) don't need info-level logging

[themilchenko]

Anyway I think that default log level of http server should be info. For example we can have routes of server that user should see in log file in info level.

If we need to decrease its level in case of metrics-export-role, it will be better to have related option for only /metrics route or set it in config here (in httpd role) globally.

[a1div0]

Merge request appeared in response to a request from the operation workers to remove logging of requests to metrics. Now the idea is to reduce the importance of logging requests from "info" to "debug", remembering that there are audit logs and that it will be possible to change this behavior through configuration settings.

https://jira.vk.team/browse/TNTP-3229

[themilchenko]

Now the idea is to reduce the importance of logging requests from "info" to "debug"

To be honest, I didn't get a motivation of it. Why does default info level not satisfy us? Maybe we can provide non-default level we want manually or even for separate route (for /metrics as an example)?

[DifferentialOrange]

I agree that for now changing the log level for a server (or for a \metrics handle) show be the option with the same default as it behaves now. Changing the default is a separate question of breaking backward compatibility that must be processed separately (after some discussion regarding whether or not we should actually do it).

[a1div0]

The main motivation is to avoid clogging the logs at the Info level with this by default:

It just interferes with debugging and investigations. Regarding backward compatibility, it is probably worth discussing what can break. As far as I know, in the 3rd version this is only used in metrics.

See TNTP-3229

[themilchenko]

Please, add note about new parameter somewhere here.

[DifferentialOrange]

parse_params supports functions and booleans for log_requests, but doesn't support strings. So it won't work this way with Tarantool 3 config.

Let's support it then. And we also should add an integration tests to check that it works. And since the string parsing is on us, I think we may use verbose instead of log.verbose and so on.

[DifferentialOrange]

Current solution doesn't seem to work.

[themilchenko]

Let's make all levels compatible with Tarantool: https://www.tarantool.io/en/doc/latest/reference/configuration/#confval-log_level

[p0rtale]

Does it make sense to log regular requests at levels above info like warn, crit or error? Those levels are typically reserved for exceptional or error situations, not for normal request logging.