diff --git a/internal/collector/instance.go b/internal/collector/instance.go index 9c83f11f3a..54081b2684 100644 --- a/internal/collector/instance.go +++ b/internal/collector/instance.go @@ -50,9 +50,7 @@ func AddToPod( includeLogrotate bool, thisPodServesMetrics bool, ) { - if spec == nil || - !(feature.Enabled(ctx, feature.OpenTelemetryLogs) || - feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + if !OpenTelemetryLogsOrMetricsEnabled(ctx, spec) { return } diff --git a/internal/collector/naming.go b/internal/collector/naming.go index c8db6d6f21..801d61e8ce 100644 --- a/internal/collector/naming.go +++ b/internal/collector/naming.go @@ -15,6 +15,7 @@ const PGBouncerMetrics = "metrics/pgbouncer" const PostgresMetrics = "metrics/postgres" const PatroniMetrics = "metrics/patroni" const ResourceDetectionProcessor = "resourcedetection" +const MonitoringUser = "ccp_monitoring" const SqlQuery = "sqlquery" diff --git a/internal/collector/patroni.go b/internal/collector/patroni.go index 532d103db7..6b22df6a09 100644 --- a/internal/collector/patroni.go +++ b/internal/collector/patroni.go @@ -9,7 +9,6 @@ import ( "slices" "strconv" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -23,7 +22,7 @@ func EnablePatroniLogging(ctx context.Context, spec = inCluster.Spec.Instrumentation.Logs } - if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if OpenTelemetryLogsEnabled(ctx, inCluster) { directory := naming.PatroniPGDataLogPath // Keep track of what log records and files have been processed. @@ -134,7 +133,7 @@ func EnablePatroniMetrics(ctx context.Context, inCluster *v1beta1.PostgresCluster, outConfig *Config, ) { - if feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if OpenTelemetryMetricsEnabled(ctx, inCluster) { // Add Prometheus exporter outConfig.Exporters[Prometheus] = map[string]any{ "endpoint": "0.0.0.0:" + strconv.Itoa(PrometheusPort), diff --git a/internal/collector/patroni_test.go b/internal/collector/patroni_test.go index e2d3a84e58..2f73374109 100644 --- a/internal/collector/patroni_test.go +++ b/internal/collector/patroni_test.go @@ -11,6 +11,7 @@ import ( "gotest.tools/v3/assert" "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/internal/testing/require" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -23,8 +24,14 @@ func TestEnablePatroniLogging(t *testing.T) { ctx := feature.NewContext(context.Background(), gate) config := NewConfig(nil) + cluster := new(v1beta1.PostgresCluster) + require.UnmarshalInto(t, &cluster.Spec, `{ + instrumentation: { + logs: { retentionPeriod: 5h }, + }, + }`) - EnablePatroniLogging(ctx, new(v1beta1.PostgresCluster), config) + EnablePatroniLogging(ctx, cluster, config) result, err := config.ToYAML() assert.NilError(t, err) diff --git a/internal/collector/pgadmin.go b/internal/collector/pgadmin.go index e22ed621f0..1f82115703 100644 --- a/internal/collector/pgadmin.go +++ b/internal/collector/pgadmin.go @@ -10,7 +10,6 @@ import ( corev1 "k8s.io/api/core/v1" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -18,9 +17,10 @@ import ( func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec, configmap *corev1.ConfigMap, ) error { - if !feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if !OpenTelemetryLogsEnabled(ctx, spec) { return nil } + otelConfig := NewConfig(spec) otelConfig.Extensions["file_storage/pgadmin_data_logs"] = map[string]any{ @@ -125,5 +125,6 @@ func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec if err == nil { configmap.Data["collector.yaml"] = otelYAML } + return err } diff --git a/internal/collector/pgadmin_test.go b/internal/collector/pgadmin_test.go index c4d5acfab6..e5db11f587 100644 --- a/internal/collector/pgadmin_test.go +++ b/internal/collector/pgadmin_test.go @@ -12,7 +12,6 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/crunchydata/postgres-operator/internal/collector" - pgadmin "github.com/crunchydata/postgres-operator/internal/controller/standalone_pgadmin" "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/testing/cmp" @@ -31,7 +30,11 @@ func TestEnablePgAdminLogging(t *testing.T) { configmap := new(corev1.ConfigMap) initialize.Map(&configmap.Data) - err := collector.EnablePgAdminLogging(ctx, nil, configmap) + var instrumentation *v1beta1.InstrumentationSpec + require.UnmarshalInto(t, &instrumentation, `{ + logs: { retentionPeriod: 12h }, + }`) + err := collector.EnablePgAdminLogging(ctx, instrumentation, configmap) assert.NilError(t, err) assert.Assert(t, cmp.MarshalMatches(configmap.Data, ` @@ -44,7 +47,7 @@ collector.yaml: | extensions: file_storage/pgadmin_data_logs: create_directory: false - directory: `+pgadmin.LogDirectoryAbsolutePath+`/receiver + directory: /var/lib/pgadmin/logs/receiver fsync: true processors: batch/1s: @@ -90,11 +93,11 @@ collector.yaml: | receivers: filelog/gunicorn: include: - - `+pgadmin.GunicornLogFileAbsolutePath+` + - /var/lib/pgadmin/logs/gunicorn.log storage: file_storage/pgadmin_data_logs filelog/pgadmin: include: - - `+pgadmin.LogFileAbsolutePath+` + - /var/lib/pgadmin/logs/pgadmin.log storage: file_storage/pgadmin_data_logs service: extensions: @@ -165,7 +168,7 @@ collector.yaml: | extensions: file_storage/pgadmin_data_logs: create_directory: false - directory: `+pgadmin.LogDirectoryAbsolutePath+`/receiver + directory: /var/lib/pgadmin/logs/receiver fsync: true processors: batch/1s: @@ -211,11 +214,11 @@ collector.yaml: | receivers: filelog/gunicorn: include: - - `+pgadmin.GunicornLogFileAbsolutePath+` + - /var/lib/pgadmin/logs/gunicorn.log storage: file_storage/pgadmin_data_logs filelog/pgadmin: include: - - `+pgadmin.LogFileAbsolutePath+` + - /var/lib/pgadmin/logs/pgadmin.log storage: file_storage/pgadmin_data_logs service: extensions: diff --git a/internal/collector/pgbackrest.go b/internal/collector/pgbackrest.go index 569748ed9c..009ec0c825 100644 --- a/internal/collector/pgbackrest.go +++ b/internal/collector/pgbackrest.go @@ -11,7 +11,6 @@ import ( "fmt" "slices" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -29,7 +28,7 @@ func NewConfigForPgBackrestRepoHostPod( ) *Config { config := NewConfig(spec) - if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if OpenTelemetryLogsEnabled(ctx, spec) { var directory string for _, repo := range repos { diff --git a/internal/collector/pgbackrest_test.go b/internal/collector/pgbackrest_test.go index f1ebf14e4f..e8a5a4d2dd 100644 --- a/internal/collector/pgbackrest_test.go +++ b/internal/collector/pgbackrest_test.go @@ -11,6 +11,7 @@ import ( "gotest.tools/v3/assert" "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/internal/testing/require" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -27,8 +28,12 @@ func TestNewConfigForPgBackrestRepoHostPod(t *testing.T) { Volume: new(v1beta1.RepoPVC), }, } + var instrumentation *v1beta1.InstrumentationSpec + require.UnmarshalInto(t, &instrumentation, `{ + logs: { retentionPeriod: 12h }, + }`) - config := NewConfigForPgBackrestRepoHostPod(ctx, nil, repos) + config := NewConfigForPgBackrestRepoHostPod(ctx, instrumentation, repos) result, err := config.ToYAML() assert.NilError(t, err) diff --git a/internal/collector/pgbouncer.go b/internal/collector/pgbouncer.go index 9133bd6813..375d2b9bab 100644 --- a/internal/collector/pgbouncer.go +++ b/internal/collector/pgbouncer.go @@ -12,7 +12,6 @@ import ( "slices" "strconv" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -40,7 +39,7 @@ func NewConfigForPgBouncerPod( config := NewConfig(cluster.Spec.Instrumentation) EnablePgBouncerLogging(ctx, cluster, config) - EnablePgBouncerMetrics(ctx, config, sqlQueryUsername) + EnablePgBouncerMetrics(ctx, cluster, config, sqlQueryUsername) return config } @@ -56,7 +55,7 @@ func EnablePgBouncerLogging(ctx context.Context, spec = inCluster.Spec.Instrumentation.Logs } - if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if OpenTelemetryLogsEnabled(ctx, inCluster) { directory := naming.PGBouncerLogPath // Keep track of what log records and files have been processed. @@ -171,8 +170,10 @@ func EnablePgBouncerLogging(ctx context.Context, // EnablePgBouncerMetrics adds necessary configuration to the collector config to scrape // metrics from pgBouncer when the OpenTelemetryMetrics feature flag is enabled. -func EnablePgBouncerMetrics(ctx context.Context, config *Config, sqlQueryUsername string) { - if feature.Enabled(ctx, feature.OpenTelemetryMetrics) { +func EnablePgBouncerMetrics(ctx context.Context, inCluster *v1beta1.PostgresCluster, + config *Config, sqlQueryUsername string) { + + if OpenTelemetryMetricsEnabled(ctx, inCluster) { // Add Prometheus exporter config.Exporters[Prometheus] = map[string]any{ "endpoint": "0.0.0.0:" + strconv.Itoa(PrometheusPort), diff --git a/internal/collector/pgbouncer_test.go b/internal/collector/pgbouncer_test.go index df8427fbbd..74aed710da 100644 --- a/internal/collector/pgbouncer_test.go +++ b/internal/collector/pgbouncer_test.go @@ -11,6 +11,7 @@ import ( "gotest.tools/v3/assert" "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/internal/testing/require" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -23,8 +24,13 @@ func TestEnablePgBouncerLogging(t *testing.T) { ctx := feature.NewContext(context.Background(), gate) config := NewConfig(nil) - - EnablePgBouncerLogging(ctx, new(v1beta1.PostgresCluster), config) + cluster := new(v1beta1.PostgresCluster) + require.UnmarshalInto(t, &cluster.Spec, `{ + instrumentation: { + logs: { retentionPeriod: 5h }, + }, + }`) + EnablePgBouncerLogging(ctx, cluster, config) result, err := config.ToYAML() assert.NilError(t, err) diff --git a/internal/collector/postgres.go b/internal/collector/postgres.go index cfc0b88245..5d419f85ea 100644 --- a/internal/collector/postgres.go +++ b/internal/collector/postgres.go @@ -15,7 +15,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" @@ -86,7 +85,7 @@ func EnablePostgresLogging( spec = inCluster.Spec.Instrumentation.Logs } - if inCluster != nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if OpenTelemetryLogsEnabled(ctx, inCluster) { directory := postgres.LogDirectory() version := inCluster.Spec.PostgresVersion diff --git a/internal/collector/postgres_metrics.go b/internal/collector/postgres_metrics.go index b6bd39cd87..4530c431a3 100644 --- a/internal/collector/postgres_metrics.go +++ b/internal/collector/postgres_metrics.go @@ -12,9 +12,7 @@ import ( "slices" "strconv" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/logging" - "github.com/crunchydata/postgres-operator/internal/pgmonitor" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -59,7 +57,7 @@ type metric struct { } func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresCluster, config *Config) { - if feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if OpenTelemetryMetricsEnabled(ctx, inCluster) { log := logging.FromContext(ctx) var err error @@ -131,7 +129,7 @@ func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresClust "driver": "postgres", "datasource": fmt.Sprintf( `host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`, - pgmonitor.MonitoringUser), + MonitoringUser), "collection_interval": "5s", // Give Postgres time to finish setup. "initial_delay": "10s", @@ -142,7 +140,7 @@ func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresClust "driver": "postgres", "datasource": fmt.Sprintf( `host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`, - pgmonitor.MonitoringUser), + MonitoringUser), "collection_interval": "300s", // Give Postgres time to finish setup. "initial_delay": "10s", @@ -172,7 +170,7 @@ func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresClust "driver": "postgres", "datasource": fmt.Sprintf( `host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`, - pgmonitor.MonitoringUser), + MonitoringUser), "collection_interval": querySet.CollectionInterval, // Give Postgres time to finish setup. "initial_delay": "10s", diff --git a/internal/collector/postgres_test.go b/internal/collector/postgres_test.go index a6736d66cc..3bdf33c61a 100644 --- a/internal/collector/postgres_test.go +++ b/internal/collector/postgres_test.go @@ -12,6 +12,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/postgres" + "github.com/crunchydata/postgres-operator/internal/testing/require" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -25,6 +26,11 @@ func TestEnablePostgresLogging(t *testing.T) { cluster := new(v1beta1.PostgresCluster) cluster.Spec.PostgresVersion = 99 + require.UnmarshalInto(t, &cluster.Spec, `{ + instrumentation: { + logs: { retentionPeriod: 5h }, + }, + }`) config := NewConfig(nil) params := postgres.NewParameterSet() diff --git a/internal/collector/util.go b/internal/collector/util.go new file mode 100644 index 0000000000..72cf8641ef --- /dev/null +++ b/internal/collector/util.go @@ -0,0 +1,56 @@ +// Copyright 2025 Crunchy Data Solutions, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +package collector + +import ( + "context" + + "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" +) + +type CrunchyCRD interface { + *v1beta1.PostgresCluster | *v1beta1.PGAdmin | *v1beta1.InstrumentationSpec +} + +func OpenTelemetrySpecPresent[T CrunchyCRD](object T) bool { + + switch v := any(object).(type) { + case *v1beta1.InstrumentationSpec: + return v != nil + case *v1beta1.PostgresCluster: + return v.Spec.Instrumentation != nil + case *v1beta1.PGAdmin: + return v.Spec.Instrumentation != nil + default: + return false + } + +} + +func OpenTelemetryLogsOrMetricsEnabled[T CrunchyCRD]( + ctx context.Context, + object T, +) bool { + return OpenTelemetrySpecPresent(object) && + (feature.Enabled(ctx, feature.OpenTelemetryLogs) || + feature.Enabled(ctx, feature.OpenTelemetryMetrics)) +} + +func OpenTelemetryLogsEnabled[T CrunchyCRD]( + ctx context.Context, + object T, +) bool { + return OpenTelemetrySpecPresent(object) && + feature.Enabled(ctx, feature.OpenTelemetryLogs) +} + +func OpenTelemetryMetricsEnabled[T CrunchyCRD]( + ctx context.Context, + object T, +) bool { + return OpenTelemetrySpecPresent(object) && + feature.Enabled(ctx, feature.OpenTelemetryMetrics) +} diff --git a/internal/controller/postgrescluster/cluster.go b/internal/controller/postgrescluster/cluster.go index ead4881b1e..2ceb30453a 100644 --- a/internal/controller/postgrescluster/cluster.go +++ b/internal/controller/postgrescluster/cluster.go @@ -15,7 +15,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/internal/collector" "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/patroni" @@ -75,7 +75,7 @@ func (r *Reconciler) patroniLogSize(ctx context.Context, cluster *v1beta1.Postgr sizeInBytes = 25000000 } return sizeInBytes - } else if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + } else if collector.OpenTelemetryLogsEnabled(ctx, cluster) { return 25000000 } return 0 diff --git a/internal/controller/postgrescluster/cluster_test.go b/internal/controller/postgrescluster/cluster_test.go index 6882cfa27b..a38a128086 100644 --- a/internal/controller/postgrescluster/cluster_test.go +++ b/internal/controller/postgrescluster/cluster_test.go @@ -870,6 +870,11 @@ func TestPatroniLogSize(t *testing.T) { reconciler := &Reconciler{Recorder: recorder} cluster.Spec.Patroni = nil + require.UnmarshalInto(t, &cluster.Spec, `{ + instrumentation: { + logs: { retentionPeriod: 5h }, + }, + }`) size := reconciler.patroniLogSize(ctx, &cluster) diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index d6fc6158e8..cb3e079e08 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1202,7 +1202,7 @@ func (r *Reconciler) reconcileInstance( // If either OpenTelemetry feature is enabled, we want to add the collector config to the pod if err == nil && - (feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + collector.OpenTelemetryLogsOrMetricsEnabled(ctx, cluster) { // If the OpenTelemetryMetrics feature is enabled, we need to get the pgpassword from the // monitoring user secret @@ -1428,8 +1428,8 @@ 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)) { + collector.OpenTelemetryLogsOrMetricsEnabled(ctx, cluster) { + err = collector.AddToConfigMap(ctx, otelConfig, instanceConfigMap) // Add pgbackrest logrotate if OpenTelemetryLogs is enabled and diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index 41d1b942a1..b7de247a5d 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -694,7 +694,7 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster // If OpenTelemetryLogs is enabled, we want to add the collector to the pod // and also add the RepoVolumes to the container. - if postgresCluster.Spec.Instrumentation != nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if collector.OpenTelemetryLogsEnabled(ctx, postgresCluster) { collector.AddToPod(ctx, postgresCluster.Spec.Instrumentation, postgresCluster.Spec.ImagePullPolicy, &corev1.ConfigMap{ObjectMeta: naming.PGBackRestConfig(postgresCluster)}, &repo.Spec.Template, []corev1.VolumeMount{}, "", diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index 660572005a..671b284299 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -19,7 +19,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "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/logging" "github.com/crunchydata/postgres-operator/internal/naming" @@ -99,13 +98,11 @@ func (r *Reconciler) reconcilePGBouncerConfigMap( pgbouncer.ConfigMap(ctx, cluster, configmap) } // If OTel logging or metrics is enabled, add collector config - if otelConfig != nil && - (feature.Enabled(ctx, feature.OpenTelemetryLogs) || - feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + if collector.OpenTelemetryLogsOrMetricsEnabled(ctx, cluster) { err = collector.AddToConfigMap(ctx, otelConfig, configmap) } // If OTel logging is enabled, add logrotate config - if err == nil && otelConfig != nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if err == nil && collector.OpenTelemetryLogsEnabled(ctx, cluster) { logrotateConfig := collector.LogrotateConfig{ LogFiles: []string{naming.PGBouncerFullLogPath}, PostrotateScript: collector.PGBouncerPostRotateScript, diff --git a/internal/controller/postgrescluster/pgmonitor.go b/internal/controller/postgrescluster/pgmonitor.go index 84b955559a..48d15d1e6d 100644 --- a/internal/controller/postgrescluster/pgmonitor.go +++ b/internal/controller/postgrescluster/pgmonitor.go @@ -16,6 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "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" @@ -62,7 +63,7 @@ func (r *Reconciler) reconcilePGMonitorExporter(ctx context.Context, // the `EnableExporterInPostgreSQL` funcs; that way we are always running // that function against an updated and running pod. - if pgmonitor.ExporterEnabled(ctx, cluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if pgmonitor.ExporterEnabled(ctx, cluster) || collector.OpenTelemetryMetricsEnabled(ctx, cluster) { sql, err := os.ReadFile(fmt.Sprintf("%s/pg%d/setup.sql", pgmonitor.GetQueriesConfigDir(ctx), cluster.Spec.PostgresVersion)) if err != nil { return err @@ -99,7 +100,7 @@ func (r *Reconciler) reconcilePGMonitorExporter(ctx context.Context, return pgmonitor.EnableExporterInPostgreSQL(ctx, exec, monitoringSecret, pgmonitor.ExporterDB, setup) } - if !pgmonitor.ExporterEnabled(ctx, cluster) && !feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if !pgmonitor.ExporterEnabled(ctx, cluster) && !collector.OpenTelemetryMetricsEnabled(ctx, cluster) { action = func(ctx context.Context, exec postgres.Executor) error { return pgmonitor.DisableMonitoringUserInPostgres(ctx, exec) } @@ -161,7 +162,7 @@ func (r *Reconciler) reconcileMonitoringSecret( // is enabled to determine when monitoring secret should be created, // since our implementation of the SqlQuery receiver in the OTel Collector // uses the monitoring user as well. - if !pgmonitor.ExporterEnabled(ctx, cluster) && !feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if !pgmonitor.ExporterEnabled(ctx, cluster) && !collector.OpenTelemetryMetricsEnabled(ctx, cluster) { if err == nil { err = errors.WithStack(r.deleteControlled(ctx, cluster, existing)) } @@ -234,7 +235,7 @@ func addPGMonitorExporterToInstancePodSpec( template *corev1.PodTemplateSpec, exporterQueriesConfig, exporterWebConfig *corev1.ConfigMap) { - if !pgmonitor.ExporterEnabled(ctx, cluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if !pgmonitor.ExporterEnabled(ctx, cluster) || collector.OpenTelemetryMetricsEnabled(ctx, cluster) { return } @@ -374,7 +375,7 @@ func addPGMonitorExporterToInstancePodSpec( func (r *Reconciler) reconcileExporterWebConfig(ctx context.Context, cluster *v1beta1.PostgresCluster) (*corev1.ConfigMap, error) { - if feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if collector.OpenTelemetryMetricsEnabled(ctx, cluster) { return nil, nil } @@ -384,7 +385,9 @@ func (r *Reconciler) reconcileExporterWebConfig(ctx context.Context, return nil, err } - if !pgmonitor.ExporterEnabled(ctx, cluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) || cluster.Spec.Monitoring.PGMonitor.Exporter.CustomTLSSecret == nil { + if !pgmonitor.ExporterEnabled(ctx, cluster) || + collector.OpenTelemetryMetricsEnabled(ctx, cluster) || + cluster.Spec.Monitoring.PGMonitor.Exporter.CustomTLSSecret == nil { // We could still have a NotFound error here so check the err. // If no error that means the configmap is found and needs to be deleted if err == nil { @@ -441,7 +444,7 @@ func (r *Reconciler) reconcileExporterQueriesConfig(ctx context.Context, return nil, err } - if !pgmonitor.ExporterEnabled(ctx, cluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if !pgmonitor.ExporterEnabled(ctx, cluster) || collector.OpenTelemetryMetricsEnabled(ctx, cluster) { // We could still have a NotFound error here so check the err. // If no error that means the configmap is found and needs to be deleted if err == nil { diff --git a/internal/controller/standalone_pgadmin/configmap.go b/internal/controller/standalone_pgadmin/configmap.go index 72a95b14db..5078e0e9fa 100644 --- a/internal/controller/standalone_pgadmin/configmap.go +++ b/internal/controller/standalone_pgadmin/configmap.go @@ -19,7 +19,6 @@ 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" @@ -73,7 +72,7 @@ func configmap(ctx context.Context, pgadmin *v1beta1.PGAdmin, 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 { + if collector.OpenTelemetryLogsEnabled(ctx, pgadmin) { logRetention = true // If the user has set a retention period, we will use those values for log rotation, diff --git a/internal/controller/standalone_pgadmin/pod_test.go b/internal/controller/standalone_pgadmin/pod_test.go index b414a7bab0..bc8a32da49 100644 --- a/internal/controller/standalone_pgadmin/pod_test.go +++ b/internal/controller/standalone_pgadmin/pod_test.go @@ -16,6 +16,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/initialize" "github.com/crunchydata/postgres-operator/internal/kubernetes" "github.com/crunchydata/postgres-operator/internal/testing/cmp" + "github.com/crunchydata/postgres-operator/internal/testing/require" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -211,13 +212,9 @@ volumes: pgadmin.Spec.Resources.Requests = corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("100m"), } - retentionPeriod, err := v1beta1.NewDuration("12 hours") - assert.NilError(t, err) - pgadmin.Spec.Instrumentation = &v1beta1.InstrumentationSpec{ - Logs: &v1beta1.InstrumentationLogsSpec{ - RetentionPeriod: retentionPeriod, - }, - } + require.UnmarshalInto(t, &pgadmin.Spec.Instrumentation, `{ + logs: { retentionPeriod: 12h }, + }`) call() diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go index 6e606b0867..6783780eae 100644 --- a/internal/controller/standalone_pgadmin/statefulset.go +++ b/internal/controller/standalone_pgadmin/statefulset.go @@ -17,7 +17,6 @@ import ( "github.com/crunchydata/postgres-operator/internal/collector" "github.com/crunchydata/postgres-operator/internal/controller/postgrescluster" - "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" @@ -122,7 +121,7 @@ func statefulset( pod(pgadmin, configmap, &sts.Spec.Template.Spec, dataVolume) - if pgadmin.Spec.Instrumentation != nil && feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if collector.OpenTelemetryLogsEnabled(ctx, pgadmin) { // Logs for gunicorn and pgadmin write to /var/lib/pgadmin/logs // so the collector needs access to that that path. dataVolumeMount := corev1.VolumeMount{ diff --git a/internal/pgbackrest/config.go b/internal/pgbackrest/config.go index c14a264ce3..498be32d3b 100644 --- a/internal/pgbackrest/config.go +++ b/internal/pgbackrest/config.go @@ -17,7 +17,6 @@ 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" @@ -131,8 +130,8 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet ).String() if RepoHostVolumeDefined(postgresCluster) && - (feature.Enabled(ctx, feature.OpenTelemetryLogs) || - feature.Enabled(ctx, feature.OpenTelemetryMetrics)) { + collector.OpenTelemetryLogsOrMetricsEnabled(ctx, postgresCluster) { + err = collector.AddToConfigMap(ctx, collector.NewConfigForPgBackrestRepoHostPod( ctx, postgresCluster.Spec.Instrumentation, @@ -141,8 +140,7 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet // If OTel logging is enabled, add logrotate config for the RepoHost if err == nil && - postgresCluster.Spec.Instrumentation != nil && - feature.Enabled(ctx, feature.OpenTelemetryLogs) { + collector.OpenTelemetryLogsEnabled(ctx, postgresCluster) { var pgBackRestLogPath string for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos { if repo.Volume != nil { diff --git a/internal/pgbouncer/config.go b/internal/pgbouncer/config.go index 257dc63dbd..99bcac0399 100644 --- a/internal/pgbouncer/config.go +++ b/internal/pgbouncer/config.go @@ -12,7 +12,7 @@ import ( corev1 "k8s.io/api/core/v1" - "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/internal/collector" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -127,13 +127,13 @@ func clusterINI(ctx context.Context, cluster *v1beta1.PostgresCluster) string { } // If OpenTelemetryLogs feature is enabled, enable logging to file - if feature.Enabled(ctx, feature.OpenTelemetryLogs) { + if collector.OpenTelemetryLogsEnabled(ctx, cluster) { global["logfile"] = naming.PGBouncerLogPath + "/pgbouncer.log" } // When OTel metrics are enabled, allow pgBouncer's postgres user // to run read-only console queries on pgBouncer's virtual db - if feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if collector.OpenTelemetryMetricsEnabled(ctx, cluster) { global["stats_users"] = PostgresqlUser } diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index b663596ed7..8eed54a3b6 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -207,7 +207,7 @@ func Pod( template.Spec.Volumes = []corev1.Volume{configVolume} - if feature.Enabled(ctx, feature.OpenTelemetryLogs) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if collector.OpenTelemetryLogsOrMetricsEnabled(ctx, inCluster) { collector.AddToPod(ctx, inCluster.Spec.Instrumentation, inCluster.Spec.ImagePullPolicy, inConfigMap, template, []corev1.VolumeMount{configVolumeMount}, string(inSecret.Data["pgbouncer-password"]), []string{naming.PGBouncerLogPath}, true, true) diff --git a/internal/pgmonitor/postgres.go b/internal/pgmonitor/postgres.go index 1d7817c9a3..3ef83cd2e0 100644 --- a/internal/pgmonitor/postgres.go +++ b/internal/pgmonitor/postgres.go @@ -10,7 +10,7 @@ import ( corev1 "k8s.io/api/core/v1" - "github.com/crunchydata/postgres-operator/internal/feature" + "github.com/crunchydata/postgres-operator/internal/collector" "github.com/crunchydata/postgres-operator/internal/logging" "github.com/crunchydata/postgres-operator/internal/postgres" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" @@ -24,7 +24,8 @@ const ( // PostgreSQLHBAs provides the Postgres HBA rules for allowing the monitoring // exporter to be accessible func PostgreSQLHBAs(ctx context.Context, inCluster *v1beta1.PostgresCluster, outHBAs *postgres.HBAs) { - if ExporterEnabled(ctx, inCluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if ExporterEnabled(ctx, inCluster) || + collector.OpenTelemetryMetricsEnabled(ctx, inCluster) { // Limit the monitoring user to local connections using SCRAM. outHBAs.Mandatory = append(outHBAs.Mandatory, postgres.NewHBA().TCP().Users(MonitoringUser).Method("scram-sha-256").Network("127.0.0.0/8"), @@ -34,9 +35,11 @@ func PostgreSQLHBAs(ctx context.Context, inCluster *v1beta1.PostgresCluster, out } // PostgreSQLParameters provides additional required configuration parameters -// that Postgres needs to support monitoring +// that Postgres needs to support monitoring for both pgMonitor and OTel func PostgreSQLParameters(ctx context.Context, inCluster *v1beta1.PostgresCluster, outParameters *postgres.Parameters) { - if ExporterEnabled(ctx, inCluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { + if ExporterEnabled(ctx, inCluster) || + collector.OpenTelemetryMetricsEnabled(ctx, inCluster) { + // Exporter expects that shared_preload_libraries are installed // pg_stat_statements: https://access.crunchydata.com/documentation/pgmonitor/latest/exporter/ // pgnodemx: https://github.com/CrunchyData/pgnodemx diff --git a/internal/pgmonitor/util.go b/internal/pgmonitor/util.go index 32cf222448..72f528ffa3 100644 --- a/internal/pgmonitor/util.go +++ b/internal/pgmonitor/util.go @@ -8,7 +8,6 @@ import ( "context" "os" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/internal/logging" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -37,8 +36,5 @@ func ExporterEnabled(ctx context.Context, cluster *v1beta1.PostgresCluster) bool if cluster.Spec.Monitoring.PGMonitor.Exporter == nil { return false } - if feature.Enabled(ctx, feature.OpenTelemetryMetrics) { - return false - } return true } diff --git a/internal/pgmonitor/util_test.go b/internal/pgmonitor/util_test.go index e83bbb3730..a7758d0da4 100644 --- a/internal/pgmonitor/util_test.go +++ b/internal/pgmonitor/util_test.go @@ -10,7 +10,6 @@ import ( "gotest.tools/v3/assert" - "github.com/crunchydata/postgres-operator/internal/feature" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -27,12 +26,4 @@ func TestExporterEnabled(t *testing.T) { cluster.Spec.Monitoring.PGMonitor.Exporter = &v1beta1.ExporterSpec{} assert.Assert(t, ExporterEnabled(ctx, cluster)) - - gate := feature.NewGate() - assert.NilError(t, gate.SetFromMap(map[string]bool{ - feature.OpenTelemetryMetrics: true, - })) - ctx = feature.NewContext(ctx, gate) - cluster.Spec.Monitoring.PGMonitor.Exporter = &v1beta1.ExporterSpec{} - assert.Assert(t, !ExporterEnabled(ctx, cluster)) }