diff --git a/internal/collector/instance.go b/internal/collector/instance.go index 3affe78888..970f9c9109 100644 --- a/internal/collector/instance.go +++ b/internal/collector/instance.go @@ -7,6 +7,7 @@ package collector import ( "context" "fmt" + "path" corev1 "k8s.io/api/core/v1" @@ -14,6 +15,7 @@ import ( "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/internal/shell" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -44,9 +46,12 @@ func AddToPod( outPod *corev1.PodSpec, volumeMounts []corev1.VolumeMount, sqlQueryPassword string, + logDirectories []string, includeLogrotate bool, ) { - if !(feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + if spec == nil || + !(feature.Enabled(ctx, feature.OpenTelemetryLogs) || + feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { return } @@ -84,7 +89,7 @@ func AddToPod( Name: naming.ContainerCollector, Image: config.CollectorContainerImage(spec), ImagePullPolicy: pullPolicy, - Command: startCommand(includeLogrotate), + Command: startCommand(logDirectories, includeLogrotate), Env: []corev1.EnvVar{ { Name: "K8S_POD_NAMESPACE", @@ -146,13 +151,23 @@ func AddToPod( } // startCommand generates the command script used by the collector container -func startCommand(includeLogrotate bool) []string { +func startCommand(logDirectories []string, includeLogrotate bool) []string { + var mkdirScript string + if len(logDirectories) != 0 { + for _, logDir := range logDirectories { + mkdirScript = mkdirScript + ` +` + shell.MakeDirectories(0o775, logDir, + path.Join(logDir, "receiver")) + } + } + var logrotateCommand string if includeLogrotate { logrotateCommand = `logrotate -s /tmp/logrotate.status /etc/logrotate.d/logrotate.conf` } var startScript = fmt.Sprintf(` +%s OTEL_PIDFILE=/tmp/otel.pid start_otel_collector() { @@ -175,7 +190,7 @@ while read -r -t 5 -u "${fd}" ||:; do start_otel_collector fi done -`, configDirectory, logrotateCommand) +`, mkdirScript, configDirectory, logrotateCommand) wrapper := `monitor() {` + startScript + `}; export directory="$1"; export -f monitor; exec -a "$0" bash -ceu monitor` diff --git a/internal/collector/pgbackrest.go b/internal/collector/pgbackrest.go index 569829bf0e..b847f854fe 100644 --- a/internal/collector/pgbackrest.go +++ b/internal/collector/pgbackrest.go @@ -47,10 +47,9 @@ func NewConfigForPgBackrestRepoHostPod( // Keep track of what log records and files have been processed. // Use a subdirectory of the logs directory to stay within the same failure domain. - // TODO(log-rotation): Create this directory during Collector startup. config.Extensions["file_storage/pgbackrest_logs"] = map[string]any{ "directory": directory + "/receiver", - "create_directory": true, + "create_directory": false, "fsync": true, } diff --git a/internal/collector/pgbackrest_test.go b/internal/collector/pgbackrest_test.go index b82afe4c23..55276c0c9b 100644 --- a/internal/collector/pgbackrest_test.go +++ b/internal/collector/pgbackrest_test.go @@ -39,7 +39,7 @@ exporters: verbosity: detailed extensions: file_storage/pgbackrest_logs: - create_directory: true + create_directory: false directory: /pgbackrest/repo1/log/receiver fsync: true processors: @@ -131,7 +131,7 @@ exporters: project: google-project-name extensions: file_storage/pgbackrest_logs: - create_directory: true + create_directory: false directory: /pgbackrest/repo1/log/receiver fsync: true processors: diff --git a/internal/collector/pgbouncer.go b/internal/collector/pgbouncer.go index 59ba0b7495..40a501e7f1 100644 --- a/internal/collector/pgbouncer.go +++ b/internal/collector/pgbouncer.go @@ -54,12 +54,11 @@ func EnablePgBouncerLogging(ctx context.Context, // Keep track of what log records and files have been processed. // Use a subdirectory of the logs directory to stay within the same failure domain. - // TODO(log-rotation): Create this directory during Collector startup. // // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/extension/storage/filestorage#readme outConfig.Extensions["file_storage/pgbouncer_logs"] = map[string]any{ "directory": directory + "/receiver", - "create_directory": true, + "create_directory": false, "fsync": true, } diff --git a/internal/collector/pgbouncer_test.go b/internal/collector/pgbouncer_test.go index 892e89e185..6e19ebdac2 100644 --- a/internal/collector/pgbouncer_test.go +++ b/internal/collector/pgbouncer_test.go @@ -35,7 +35,7 @@ exporters: verbosity: detailed extensions: file_storage/pgbouncer_logs: - create_directory: true + create_directory: false directory: /tmp/receiver fsync: true processors: @@ -124,7 +124,7 @@ exporters: project: google-project-name extensions: file_storage/pgbouncer_logs: - create_directory: true + create_directory: false directory: /tmp/receiver fsync: true processors: diff --git a/internal/collector/postgres.go b/internal/collector/postgres.go index 04379fe08e..38f680d369 100644 --- a/internal/collector/postgres.go +++ b/internal/collector/postgres.go @@ -27,10 +27,13 @@ func NewConfigForPostgresPod(ctx context.Context, ) *Config { config := NewConfig(inCluster.Spec.Instrumentation) + // Metrics EnablePostgresMetrics(ctx, inCluster, config) EnablePatroniMetrics(ctx, inCluster, config) - EnablePatroniLogging(ctx, inCluster, config) + + // Logging EnablePostgresLogging(ctx, inCluster, config, outParameters) + EnablePatroniLogging(ctx, inCluster, config) return config } @@ -229,10 +232,9 @@ func EnablePostgresLogging( } // pgBackRest pipeline - // TODO(log-rotation): Create this directory during Collector startup. outConfig.Extensions["file_storage/pgbackrest_logs"] = map[string]any{ "directory": naming.PGBackRestPGDataLogPath + "/receiver", - "create_directory": true, + "create_directory": false, "fsync": true, } diff --git a/internal/collector/postgres_test.go b/internal/collector/postgres_test.go index 9c55757fbd..1c09d32b28 100644 --- a/internal/collector/postgres_test.go +++ b/internal/collector/postgres_test.go @@ -40,7 +40,7 @@ exporters: verbosity: detailed extensions: file_storage/pgbackrest_logs: - create_directory: true + create_directory: false directory: /pgdata/pgbackrest/log/receiver fsync: true file_storage/postgres_logs: @@ -272,7 +272,7 @@ exporters: project: google-project-name extensions: file_storage/pgbackrest_logs: - create_directory: true + create_directory: false directory: /pgdata/pgbackrest/log/receiver fsync: true file_storage/postgres_logs: diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 8300da5d0f..6d6509eafb 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1219,8 +1219,10 @@ func (r *Reconciler) reconcileInstance( } // For now, we are not using logrotate to rotate postgres or patroni logs + // but we are using it for pgbackrest logs in the postgres pod collector.AddToPod(ctx, cluster.Spec.Instrumentation, cluster.Spec.ImagePullPolicy, instanceConfigMap, &instance.Spec.Template.Spec, - []corev1.VolumeMount{postgres.DataVolumeMount()}, pgPassword, false) + []corev1.VolumeMount{postgres.DataVolumeMount()}, pgPassword, + []string{naming.PGBackRestPGDataLogPath}, true) } // Add postgres-exporter to the instance Pod spec @@ -1425,8 +1427,24 @@ func (r *Reconciler) reconcileInstanceConfigMap( }) // If OTel logging or metrics is enabled, add collector config - if err == nil && (feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + if err == nil && + (feature.Enabled(ctx, feature.OpenTelemetryLogs) || + feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { err = collector.AddToConfigMap(ctx, otelConfig, instanceConfigMap) + + // Add pgbackrest logrotate if OpenTelemetryLogs is enabled and + // local volumes are available + if err == nil && + feature.Enabled(ctx, feature.OpenTelemetryLogs) && + pgbackrest.RepoHostVolumeDefined(cluster) && + cluster.Spec.Instrumentation != nil { + + collector.AddLogrotateConfigs(ctx, cluster.Spec.Instrumentation, + instanceConfigMap, + []collector.LogrotateConfig{{ + LogFiles: []string{naming.PGBackRestPGDataLogPath + "/*.log"}, + }}) + } } if err == nil { err = patroni.InstanceConfigMap(ctx, cluster, spec, instanceConfigMap) diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index fc8b25a80e..3645871bd5 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -688,18 +688,17 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster if pgbackrest.RepoHostVolumeDefined(postgresCluster) { // add the init container to make the pgBackRest repo volume log directory - pgbackrest.MakePGBackrestLogDir(&repo.Spec.Template, postgresCluster) + pgBackRestLogPath := pgbackrest.MakePGBackrestLogDir(&repo.Spec.Template, postgresCluster) containersToAdd := []string{naming.PGBackRestRepoContainerName} // If OpenTelemetryLogs is enabled, we want to add the collector to the pod // and also add the RepoVolumes to the container. - if feature.Enabled(ctx, feature.OpenTelemetryLogs) { - // TODO: Setting the includeLogrotate argument to false for now. This - // should be changed when we implement log rotation for pgbackrest + if postgresCluster.Spec.Instrumentation != nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) { collector.AddToPod(ctx, postgresCluster.Spec.Instrumentation, postgresCluster.Spec.ImagePullPolicy, &corev1.ConfigMap{ObjectMeta: naming.PGBackRestConfig(postgresCluster)}, - &repo.Spec.Template.Spec, []corev1.VolumeMount{}, "", false) + &repo.Spec.Template.Spec, []corev1.VolumeMount{}, "", + []string{pgBackRestLogPath}, true) containersToAdd = append(containersToAdd, naming.ContainerCollector) } diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index 75550f11c3..2b1dcae779 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -100,7 +100,8 @@ func (r *Reconciler) reconcilePGBouncerConfigMap( } // If OTel logging or metrics is enabled, add collector config if otelConfig != nil && - (feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + (feature.Enabled(ctx, feature.OpenTelemetryLogs) || + feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { err = collector.AddToConfigMap(ctx, otelConfig, configmap) } // If OTel logging is enabled, add logrotate config diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go index c3cc6f661c..2c9a17595d 100644 --- a/internal/controller/standalone_pgadmin/statefulset.go +++ b/internal/controller/standalone_pgadmin/statefulset.go @@ -132,7 +132,7 @@ func statefulset( } collector.AddToPod(ctx, pgadmin.Spec.Instrumentation, pgadmin.Spec.ImagePullPolicy, - configmap, &sts.Spec.Template.Spec, volumeMounts, "", false) + configmap, &sts.Spec.Template.Spec, volumeMounts, "", []string{}, false) } return sts diff --git a/internal/pgbackrest/config.go b/internal/pgbackrest/config.go index bfbf6f8d63..c14a264ce3 100644 --- a/internal/pgbackrest/config.go +++ b/internal/pgbackrest/config.go @@ -17,6 +17,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/collector" "github.com/crunchydata/postgres-operator/internal/config" + "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/internal/postgres" @@ -129,11 +130,32 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet postgresCluster.Spec.Backups.PGBackRest.Global, ).String() - err = collector.AddToConfigMap(ctx, collector.NewConfigForPgBackrestRepoHostPod( - ctx, - postgresCluster.Spec.Instrumentation, - postgresCluster.Spec.Backups.PGBackRest.Repos, - ), cm) + if RepoHostVolumeDefined(postgresCluster) && + (feature.Enabled(ctx, feature.OpenTelemetryLogs) || + feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + err = collector.AddToConfigMap(ctx, collector.NewConfigForPgBackrestRepoHostPod( + ctx, + postgresCluster.Spec.Instrumentation, + postgresCluster.Spec.Backups.PGBackRest.Repos, + ), cm) + + // If OTel logging is enabled, add logrotate config for the RepoHost + if err == nil && + postgresCluster.Spec.Instrumentation != nil && + feature.Enabled(ctx, feature.OpenTelemetryLogs) { + var pgBackRestLogPath string + for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos { + if repo.Volume != nil { + pgBackRestLogPath = fmt.Sprintf(naming.PGBackRestRepoLogPath, repo.Name) + break + } + } + + collector.AddLogrotateConfigs(ctx, postgresCluster.Spec.Instrumentation, cm, []collector.LogrotateConfig{{ + LogFiles: []string{pgBackRestLogPath + "/*.log"}, + }}) + } + } } cm.Data[ConfigHashKey] = configHash @@ -144,7 +166,7 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet // MakePGBackrestLogDir creates the pgBackRest default log path directory used when a // dedicated repo host is configured. func MakePGBackrestLogDir(template *corev1.PodTemplateSpec, - cluster *v1beta1.PostgresCluster) { + cluster *v1beta1.PostgresCluster) string { var pgBackRestLogPath string for _, repo := range cluster.Spec.Backups.PGBackRest.Repos { @@ -172,6 +194,8 @@ func MakePGBackrestLogDir(template *corev1.PodTemplateSpec, } } template.Spec.InitContainers = append(template.Spec.InitContainers, container) + + return pgBackRestLogPath } // RestoreCommand returns the command for performing a pgBackRest restore. In addition to calling diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index 3e45115e07..4181cea478 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -192,7 +192,7 @@ func Pod( if feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { collector.AddToPod(ctx, inCluster.Spec.Instrumentation, inCluster.Spec.ImagePullPolicy, inConfigMap, - outPod, []corev1.VolumeMount{configVolumeMount}, string(inSecret.Data["pgbouncer-password"]), + outPod, []corev1.VolumeMount{configVolumeMount}, string(inSecret.Data["pgbouncer-password"]), []string{naming.PGBouncerLogPath}, true) } }