Rotate pgAdmin/gunicorn logs according to retentionPeriod (#4101)

Add pgadmin/gunicorn log rotation configuration

This PR continues the project of adding log retention configuration
for users who have turned on the OTEL logging feature gate. This PR
also makes some changes to our pgAdmin configuration.

* Parse log retention in spec to use in pgadmin and gunicorn configuration
* Restart gunicorn if logging changes to create new logger. (This is due to gunicorn behavior.)
* Change the way we add a /tmp dir to pgAdmin to add to all containers.
* Have the collector container create the receiver dir it needs.
* Change the way we add config: add it to the configmap rather than the pod.

Issues: [PGO-2168]

Author: benjaminjb

internal/collector/config.go

@@ -205,7 +205,7 @@ func AddLogrotateConfigs(ctx context.Context, spec *v1beta1.InstrumentationSpec,
 func generateLogrotateConfig(
 	config LogrotateConfig, retentionPeriod metav1.Duration,
 ) string {
-	number, interval := parseDurationForLogrotate(retentionPeriod)
+	number, interval := ParseDurationForLogrotate(retentionPeriod)
 
 	return fmt.Sprintf(
 		logrotateConfigFormatString,
@@ -216,12 +216,12 @@ func generateLogrotateConfig(
 	)
 }
 
-// parseDurationForLogrotate takes a retention period and returns the rotate
+// ParseDurationForLogrotate takes a retention period and returns the rotate
 // number and interval string that should be used in the logrotate config.
 // If the retentionPeriod is less than 24 hours, the function will return the
 // number of hours and "hourly"; otherwise, we will round up to the nearest day
 // and return the day count and "daily"
-func parseDurationForLogrotate(retentionPeriod metav1.Duration) (int, string) {
+func ParseDurationForLogrotate(retentionPeriod metav1.Duration) (int, string) {
 	hours := math.Ceil(retentionPeriod.Hours())
 	if hours < 24 {
 		return int(hours), "hourly"

internal/collector/config_test.go

@@ -279,7 +279,7 @@ func TestParseDurationForLogrotate(t *testing.T) {
 		t.Run(tt.retentionPeriod, func(t *testing.T) {
 			duration, err := v1beta1.NewDuration(tt.retentionPeriod)
 			assert.NilError(t, err)
-			number, interval := parseDurationForLogrotate(duration.AsDuration())
+			number, interval := ParseDurationForLogrotate(duration.AsDuration())
 			assert.Equal(t, tt.number, number)
 			assert.Equal(t, tt.interval, interval)
 		})

internal/controller/postgrescluster/instance.go

@@ -1242,7 +1242,7 @@ func (r *Reconciler) reconcileInstance(
 	// add an emptyDir volume to the PodTemplateSpec and an associated '/tmp' volume mount to
 	// all containers included within that spec
 	if err == nil {
-		addTMPEmptyDir(&instance.Spec.Template)
+		AddTMPEmptyDir(&instance.Spec.Template)
 	}
 
 	// mount shared memory to the Postgres instance

internal/controller/postgrescluster/pgadmin.go

@@ -365,7 +365,7 @@ func (r *Reconciler) reconcilePGAdminStatefulSet(
 
 	// add an emptyDir volume to the PodTemplateSpec and an associated '/tmp'
 	// volume mount to all containers included within that spec
-	addTMPEmptyDir(&sts.Spec.Template)
+	AddTMPEmptyDir(&sts.Spec.Template)
 
 	return errors.WithStack(r.apply(ctx, sts))
 }

internal/controller/postgrescluster/pgbackrest.go

@@ -720,7 +720,7 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster
 		postgresCluster.Spec.ImagePullPolicy,
 		&repo.Spec.Template)
 
-	addTMPEmptyDir(&repo.Spec.Template)
+	AddTMPEmptyDir(&repo.Spec.Template)
 
 	// set ownership references
 	if err := r.setControllerReference(postgresCluster, repo); err != nil {
@@ -1272,7 +1272,7 @@ func (r *Reconciler) reconcileRestoreJob(ctx context.Context,
 		cluster.Spec.ImagePullPolicy,
 		&restoreJob.Spec.Template)
 
-	addTMPEmptyDir(&restoreJob.Spec.Template)
+	AddTMPEmptyDir(&restoreJob.Spec.Template)
 
 	return errors.WithStack(r.apply(ctx, restoreJob))
 }

internal/controller/postgrescluster/pgbouncer.go

@@ -476,7 +476,7 @@ func (r *Reconciler) generatePGBouncerDeployment(
 	}
 
 	// Add tmp directory and volume for log files
-	addTMPEmptyDir(&deploy.Spec.Template)
+	AddTMPEmptyDir(&deploy.Spec.Template)
 
 	return deploy, true, err
 }

internal/controller/postgrescluster/snapshots.go

@@ -394,7 +394,7 @@ func (r *Reconciler) dedicatedSnapshotVolumeRestore(ctx context.Context,
 		cluster.Spec.ImagePullPolicy,
 		&restoreJob.Spec.Template)
 
-	addTMPEmptyDir(&restoreJob.Spec.Template)
+	AddTMPEmptyDir(&restoreJob.Spec.Template)
 
 	restoreJob.Annotations[naming.PGBackRestBackupJobCompletion] = backupJob.Status.CompletionTime.Format(time.RFC3339)
 	return errors.WithStack(r.apply(ctx, restoreJob))

internal/controller/postgrescluster/util.go

@@ -134,13 +134,13 @@ func addDevSHM(template *corev1.PodTemplateSpec) {
 	}
 }
 
-// addTMPEmptyDir adds a "tmp" EmptyDir volume to the provided Pod template, while then also adding a
+// AddTMPEmptyDir adds a "tmp" EmptyDir volume to the provided Pod template, while then also adding a
 // volume mount at /tmp for all containers defined within the Pod template
 // The '/tmp' directory is currently utilized for the following:
 //   - As the pgBackRest lock directory (this is the default lock location for pgBackRest)
 //   - The location where the replication client certificates can be loaded with the proper
 //     permissions set
-func addTMPEmptyDir(template *corev1.PodTemplateSpec) {
+func AddTMPEmptyDir(template *corev1.PodTemplateSpec) {
 
 	template.Spec.Volumes = append(template.Spec.Volumes, corev1.Volume{
 		Name: "tmp",

internal/controller/standalone_pgadmin/configmap.go

@@ -19,6 +19,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/crunchydata/postgres-operator/internal/collector"
+	"github.com/crunchydata/postgres-operator/internal/feature"
 	"github.com/crunchydata/postgres-operator/internal/initialize"
 	"github.com/crunchydata/postgres-operator/internal/naming"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
@@ -32,7 +33,7 @@ func (r *PGAdminReconciler) reconcilePGAdminConfigMap(
 	ctx context.Context, pgadmin *v1beta1.PGAdmin,
 	clusters map[string][]*v1beta1.PostgresCluster,
 ) (*corev1.ConfigMap, error) {
-	configmap, err := configmap(pgadmin, clusters)
+	configmap, err := configmap(ctx, pgadmin, clusters)
 	if err != nil {
 		return configmap, err
 	}
@@ -50,7 +51,7 @@ func (r *PGAdminReconciler) reconcilePGAdminConfigMap(
 }
 
 // configmap returns a v1.ConfigMap for pgAdmin.
-func configmap(pgadmin *v1beta1.PGAdmin,
+func configmap(ctx context.Context, pgadmin *v1beta1.PGAdmin,
 	clusters map[string][]*v1beta1.PostgresCluster,
 ) (*corev1.ConfigMap, error) {
 	configmap := &corev1.ConfigMap{ObjectMeta: naming.StandalonePGAdmin(pgadmin)}
@@ -63,7 +64,38 @@ func configmap(pgadmin *v1beta1.PGAdmin,
 
 	// TODO(tjmoore4): Populate configuration details.
 	initialize.Map(&configmap.Data)
-	configSettings, err := generateConfig(pgadmin)
+	var (
+		logRetention             bool
+		maxBackupRetentionNumber = 1
+		// One day in minutes for pgadmin rotation
+		pgAdminRetentionPeriod = 24 * 60
+		// Daily rotation for gunicorn rotation
+		gunicornRetentionPeriod = "D"
+	)
+	// If OTel logs feature gate is enabled, we want to change the pgAdmin/gunicorn logging
+	if feature.Enabled(ctx, feature.OpenTelemetryLogs) && pgadmin.Spec.Instrumentation != nil {
+		logRetention = true
+
+		// If the user has set a retention period, we will use those values for log rotation,
+		// which is otherwise managed by python.
+		if pgadmin.Spec.Instrumentation.Logs != nil &&
+			pgadmin.Spec.Instrumentation.Logs.RetentionPeriod != nil {
+
+			retentionNumber, period := collector.ParseDurationForLogrotate(pgadmin.Spec.Instrumentation.Logs.RetentionPeriod.AsDuration())
+			// `LOG_ROTATION_MAX_LOG_FILES`` in pgadmin refers to the already rotated logs.
+			// `backupCount` for gunicorn is similar.
+			// Our retention unit is for total number of log files, so subtract 1 to account
+			// for the currently-used log file.
+			maxBackupRetentionNumber = retentionNumber - 1
+			if period == "hourly" {
+				// If the period is hourly, set the pgadmin
+				// and gunicorn retention periods to hourly.
+				pgAdminRetentionPeriod = 60
+				gunicornRetentionPeriod = "H"
+			}
+		}
+	}
+	configSettings, err := generateConfig(pgadmin, logRetention, maxBackupRetentionNumber, pgAdminRetentionPeriod)
 	if err == nil {
 		configmap.Data[settingsConfigMapKey] = configSettings
 	}
@@ -73,16 +105,19 @@ func configmap(pgadmin *v1beta1.PGAdmin,
 		configmap.Data[settingsClusterMapKey] = clusterSettings
 	}
 
-	gunicornSettings, err := generateGunicornConfig(pgadmin)
+	gunicornSettings, err := generateGunicornConfig(pgadmin,
+		logRetention, maxBackupRetentionNumber, gunicornRetentionPeriod)
 	if err == nil {
 		configmap.Data[gunicornConfigKey] = gunicornSettings
 	}
 
 	return configmap, err
 }
 
-// generateConfig generates the config settings for the pgAdmin
-func generateConfig(pgadmin *v1beta1.PGAdmin) (string, error) {
+// generateConfigs generates the config settings for the pgAdmin and gunicorn
+func generateConfig(pgadmin *v1beta1.PGAdmin,
+	logRetention bool, maxBackupRetentionNumber, pgAdminRetentionPeriod int) (
+	string, error) {
 	settings := map[string]any{
 		// Bind to all IPv4 addresses by default. "0.0.0.0" here represents INADDR_ANY.
 		// - https://flask.palletsprojects.com/en/2.2.x/api/#flask.Flask.run
@@ -102,6 +137,22 @@ func generateConfig(pgadmin *v1beta1.PGAdmin) (string, error) {
 	settings["UPGRADE_CHECK_ENABLED"] = false
 	settings["UPGRADE_CHECK_URL"] = ""
 	settings["UPGRADE_CHECK_KEY"] = ""
+	settings["DATA_DIR"] = dataMountPath
+	settings["LOG_FILE"] = LogFileAbsolutePath
+
+	if logRetention {
+		settings["LOG_ROTATION_AGE"] = pgAdminRetentionPeriod
+		settings["LOG_ROTATION_MAX_LOG_FILES"] = maxBackupRetentionNumber
+		settings["JSON_LOGGER"] = true
+		settings["CONSOLE_LOG_LEVEL"] = "WARNING"
+		settings["FILE_LOG_LEVEL"] = "INFO"
+		settings["FILE_LOG_FORMAT_JSON"] = map[string]string{
+			"time":    "created",
+			"name":    "name",
+			"level":   "levelname",
+			"message": "message",
+		}
+	}
 
 	// To avoid spurious reconciles, the following value must not change when
 	// the spec does not change. [json.Encoder] and [json.Marshal] do this by
@@ -185,7 +236,9 @@ func generateClusterConfig(
 
 // generateGunicornConfig generates the config settings for the gunicorn server
 // - https://docs.gunicorn.org/en/latest/settings.html
-func generateGunicornConfig(pgadmin *v1beta1.PGAdmin) (string, error) {
+func generateGunicornConfig(pgadmin *v1beta1.PGAdmin,
+	logRetention bool, maxBackupRetentionNumber int, gunicornRetentionPeriod string,
+) (string, error) {
 	settings := map[string]any{
 		// Bind to all IPv4 addresses and set 25 threads by default.
 		// - https://docs.gunicorn.org/en/latest/settings.html#bind
@@ -202,6 +255,69 @@ func generateGunicornConfig(pgadmin *v1beta1.PGAdmin) (string, error) {
 	// Write mandatory settings over any specified ones.
 	// - https://docs.gunicorn.org/en/latest/settings.html#workers
 	settings["workers"] = 1
+	// Gunicorn logging dict settings
+	logSettings := map[string]any{}
+
+	// If OTel logs feature gate is enabled, we want to change the gunicorn logging
+	if logRetention {
+
+		// Gunicorn uses the Python logging package, which sets the following attributes:
+		// https://docs.python.org/3/library/logging.html#logrecord-attributes.
+		// JsonFormatter is used to format the log: https://pypi.org/project/jsonformatter/
+		// We override the gunicorn defaults (using `logconfig_dict`) to set our own file handler.
+		// - https://docs.gunicorn.org/en/stable/settings.html#logconfig-dict
+		// - https://github.com/benoitc/gunicorn/blob/23.0.0/gunicorn/glogging.py#L47
+		logSettings = map[string]any{
+
+			"loggers": map[string]any{
+				"gunicorn.access": map[string]any{
+					"handlers":  []string{"file"},
+					"level":     "INFO",
+					"propagate": true,
+					"qualname":  "gunicorn.access",
+				},
+				"gunicorn.error": map[string]any{
+					"handlers":  []string{"file"},
+					"level":     "INFO",
+					"propagate": true,
+					"qualname":  "gunicorn.error",
+				},
+			},
+			"handlers": map[string]any{
+				"file": map[string]any{
+					"class":       "logging.handlers.TimedRotatingFileHandler",
+					"filename":    GunicornLogFileAbsolutePath,
+					"backupCount": maxBackupRetentionNumber,
+					"interval":    1,
+					"when":        gunicornRetentionPeriod,
+					"formatter":   "json",
+				},
+				"console": map[string]any{
+					"class":     "logging.StreamHandler",
+					"formatter": "generic",
+					"stream":    "ext://sys.stdout",
+				},
+			},
+			"formatters": map[string]any{
+				"generic": map[string]any{
+					"class":   "logging.Formatter",
+					"datefmt": "[%Y-%m-%d %H:%M:%S %z]",
+					"format":  "%(asctime)s [%(process)d] [%(levelname)s] %(message)s",
+				},
+				"json": map[string]any{
+					"class":      "jsonformatter.JsonFormatter",
+					"separators": []string{",", ":"},
+					"format": map[string]string{
+						"time":    "created",
+						"name":    "name",
+						"level":   "levelname",
+						"message": "message",
+					},
+				},
+			},
+		}
+	}
+	settings["logconfig_dict"] = logSettings
 
 	// To avoid spurious reconciles, the following value must not change when
 	// the spec does not change. [json.Encoder] and [json.Marshal] do this by