Rotate pgAdmin/gunicorn logs according to retentionPeriod

[benjaminjb]

Checklist:

• Have you added an explanation of what your changes do and why you'd like them to be included?
• Have you updated or added documentation for the change, as applicable?
• Have you tested your changes on all related environments with successful results, as applicable?
  • Have you added automated tests?

Type of Changes:

• New feature
• Bug fix
• Documentation
• Testing enhancement
• Other

What is the current behavior (link to any open issues here)?

pgAdmin/Gunicorn log rotation is hard-coded

What is the new behavior (if this is a feature change)?

• Breaking change (fix or feature that would cause existing functionality to change)

pgAdmin has a built in logfile rotation system, as does gunicorn. We want to use those systems, but respect the retention setting that a user sets in the instrumentation section. This PR uses the RetentionPeriod to

• set the number of backups for gunicorn and pgAdmin;
• set the rotation cycle for pgAdmin.

Other Information:

Issues: [PGO-2168]

[dsessler7]

LGTM

[dsessler7]

Thinking out loud: So it seems like we are defaulting to retaining a day's worth of logs whether we have the OpenTelemetryLogs feature enabled or not? Which looks like it was already the case for pgAdmin, but for gunicorn the behavior is changing from retaining based on size to time?

[benjaminjb]

🤔 Yeah -- and actually if we go back far enough, we weren't actually making any changes to the default logging until we added OTel. Which makes me think we should maybe only add these logging configs if OTel is enabled...

[dsessler7]

mmhmm

[benjaminjb]

OK, we made some changes about where we're logging, so I couldn't put all the logging stuff in as optional, but made some changes to get us closer to that original state if you're not using the new telemetry.

[cbandy]

🤔 What are these handlers before we overwrite them?

[benjaminjb]

We start with the default settings from here, so

    "loggers": {
        "gunicorn.error": {
            "level": "INFO",
            "handlers": ["error_console"],
            "propagate": True,
            "qualname": "gunicorn.error"
        },

        "gunicorn.access": {
            "level": "INFO",
            "handlers": ["console"],
            "propagate": True,
            "qualname": "gunicorn.access"
        }
    },

So we set all our logger.*.handlers to file, and then below we set the handlers.file, so we skip using the default handlers (right below in the default settings):

    "handlers": {
        "console": {
            "class": "logging.StreamHandler",
            "formatter": "generic",
            "stream": "ext://sys.stdout"
        },
        "error_console": {
            "class": "logging.StreamHandler",
            "formatter": "generic",
            "stream": "ext://sys.stderr"
        },
    },

[cbandy]

Thank you for making those changes. One question, but not a blocker.

[cbandy]

👍🏻 Nice! We can do a lot with config files.

❓ My read of reload_extra_files is that it requires reload to be enabled. Do we use that?

❓ I see reload_extra_files and logconfig_json mentioned in the config file settings. Is it possible to set these through settings in generateGunicornConfig()? 🤔 If so, we might end up with all JSON and no change to the entrypoint?

[benjaminjb]

See benoitc/gunicorn#3353 for an issue I ran into.

Not in that GH issue, but as a precursor to it: all the reload stuff is expressly for development. So I was thinking of adding a SIGHUP signal into our mtime checking loop, only SIGHUP doesn't work -- but SIGKILL might if we want to do that every time a cluster changes or the config changes. (The mtime for all those files change together since they are in the same configmap.)