Skip to content

Commit 061d918

Browse files
dsessler7dsessler7
committed
Handle errors in enabling postgres metrics.
Clean up some code and add some comments.
1 parent 9949681 commit 061d918

File tree

3 files changed

+48
-25
lines changed

3 files changed

+48
-25
lines changed

‎internal/collector/instance.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ while read -r -t 5 -u "${fd}" ||:; do
210210
done
211211
`, mkdirScript, configDirectory, logrotateCommand)
212212

213-
wrapper := `monitor() {` + startScript + `}; export directory="$1"; export -f monitor; exec -a "$0" bash -ceu monitor`
213+
wrapper := `monitor() {` + startScript +
214+
`}; export directory="$1"; export -f monitor; exec -a "$0" bash -ceu monitor`
214215

215216
return []string{"bash", "-ceu", "--", wrapper, "collector", configDirectory}
216217
}

‎internal/collector/pgbouncer.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ func EnablePgBouncerMetrics(ctx context.Context, config *Config, sqlQueryUsernam
181181
// Add SqlQuery Receiver
182182
config.Receivers[SqlQuery] = map[string]any{
183183
"driver": "postgres",
184-
"datasource": fmt.Sprintf(`host=localhost dbname=pgbouncer port=5432 user=%s password=${env:PGPASSWORD}`,
184+
"datasource": fmt.Sprintf(
185+
`host=localhost dbname=pgbouncer port=5432 user=%s password=${env:PGPASSWORD}`,
185186
sqlQueryUsername),
186187
"queries": slices.Clone(pgBouncerMetricsQueries),
187188
}

‎internal/collector/postgres_metrics.go‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"strconv"
1414

1515
"github.com/crunchydata/postgres-operator/internal/feature"
16+
"github.com/crunchydata/postgres-operator/internal/logging"
1617
"github.com/crunchydata/postgres-operator/internal/pgmonitor"
1718
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1819
)
@@ -39,60 +40,70 @@ var ltPG16 json.RawMessage
3940

4041
func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresCluster, config *Config) {
4142
if feature.Enabled(ctx, feature.OpenTelemetryMetrics) {
43+
log := logging.FromContext(ctx)
44+
var err error
45+
4246
// We must create a copy of the fiveSecondMetrics variable, otherwise we
4347
// will continually append to it and blow up our ConfigMap
4448
fiveSecondMetricsClone := slices.Clone(fiveSecondMetrics)
4549
fiveMinuteMetricsClone := slices.Clone(fiveMinuteMetrics)
4650

4751
if inCluster.Spec.PostgresVersion >= 17 {
48-
fiveSecondMetricsClone, _ = appendToJSONArray(fiveSecondMetricsClone, gtePG17)
52+
fiveSecondMetricsClone, err = appendToJSONArray(fiveSecondMetricsClone, gtePG17)
4953
} else {
50-
fiveSecondMetricsClone, _ = appendToJSONArray(fiveSecondMetricsClone, ltPG17)
54+
fiveSecondMetricsClone, err = appendToJSONArray(fiveSecondMetricsClone, ltPG17)
55+
}
56+
if err != nil {
57+
log.Error(err, "error compiling postgres metrics")
5158
}
5259

5360
if inCluster.Spec.PostgresVersion >= 16 {
54-
fiveSecondMetricsClone, _ = appendToJSONArray(fiveSecondMetricsClone, gtePG16)
61+
fiveSecondMetricsClone, err = appendToJSONArray(fiveSecondMetricsClone, gtePG16)
5562
} else {
56-
fiveSecondMetricsClone, _ = appendToJSONArray(fiveSecondMetricsClone, ltPG16)
63+
fiveSecondMetricsClone, err = appendToJSONArray(fiveSecondMetricsClone, ltPG16)
64+
}
65+
if err != nil {
66+
log.Error(err, "error compiling postgres metrics")
5767
}
5868

5969
// Remove any queries that user has specified in the spec
6070
if inCluster.Spec.Instrumentation != nil &&
6171
inCluster.Spec.Instrumentation.Metrics != nil &&
6272
inCluster.Spec.Instrumentation.Metrics.CustomQueries != nil &&
6373
inCluster.Spec.Instrumentation.Metrics.CustomQueries.Remove != nil {
74+
6475
// Convert json to array of maps
6576
var fiveSecondMetricsArr []map[string]any
6677
err := json.Unmarshal(fiveSecondMetricsClone, &fiveSecondMetricsArr)
6778
if err != nil {
68-
// TODO: handle this error better
69-
panic(err)
79+
log.Error(err, "error compiling postgres metrics")
7080
}
81+
7182
// Remove any specified metrics from the five second metrics
72-
fiveSecondMetricsArr = removeMetricsFromQueries(inCluster.Spec.Instrumentation.Metrics.CustomQueries.Remove, fiveSecondMetricsArr)
83+
fiveSecondMetricsArr = removeMetricsFromQueries(
84+
inCluster.Spec.Instrumentation.Metrics.CustomQueries.Remove, fiveSecondMetricsArr)
7385

7486
// Convert json to array of maps
7587
var fiveMinuteMetricsArr []map[string]any
7688
err = json.Unmarshal(fiveMinuteMetricsClone, &fiveMinuteMetricsArr)
7789
if err != nil {
78-
// TODO: handle this error better
79-
panic(err)
90+
log.Error(err, "error compiling postgres metrics")
8091
}
92+
8193
// Remove any specified metrics from the five minute metrics
82-
fiveMinuteMetricsArr = removeMetricsFromQueries(inCluster.Spec.Instrumentation.Metrics.CustomQueries.Remove, fiveMinuteMetricsArr)
94+
fiveMinuteMetricsArr = removeMetricsFromQueries(
95+
inCluster.Spec.Instrumentation.Metrics.CustomQueries.Remove, fiveMinuteMetricsArr)
8396

8497
// Convert back to json data
8598
fiveSecondMetricsClone, err = json.Marshal(fiveSecondMetricsArr)
8699
if err != nil {
87-
// TODO: handle this error better
88-
panic(err)
100+
log.Error(err, "error compiling postgres metrics")
89101
}
90102

91103
// Convert back to json data
92104
fiveMinuteMetricsClone, err = json.Marshal(fiveMinuteMetricsArr)
93105
if err != nil {
94-
// TODO: handle this error better
95-
panic(err)
106+
log.Error(err, "error compiling postgres metrics")
96107
}
97108
}
98109

@@ -102,17 +113,21 @@ func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresClust
102113
}
103114

104115
config.Receivers[FiveSecondSqlQuery] = map[string]any{
105-
"driver": "postgres",
106-
"datasource": fmt.Sprintf(`host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`, pgmonitor.MonitoringUser),
116+
"driver": "postgres",
117+
"datasource": fmt.Sprintf(
118+
`host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`,
119+
pgmonitor.MonitoringUser),
107120
"collection_interval": "5s",
108121
// Give Postgres time to finish setup.
109122
"initial_delay": "10s",
110123
"queries": slices.Clone(fiveSecondMetricsClone),
111124
}
112125

113126
config.Receivers[FiveMinuteSqlQuery] = map[string]any{
114-
"driver": "postgres",
115-
"datasource": fmt.Sprintf(`host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`, pgmonitor.MonitoringUser),
127+
"driver": "postgres",
128+
"datasource": fmt.Sprintf(
129+
`host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`,
130+
pgmonitor.MonitoringUser),
116131
"collection_interval": "300s",
117132
// Give Postgres time to finish setup.
118133
"initial_delay": "10s",
@@ -134,16 +149,20 @@ func EnablePostgresMetrics(ctx context.Context, inCluster *v1beta1.PostgresClust
134149
inCluster.Spec.Instrumentation.Metrics != nil &&
135150
inCluster.Spec.Instrumentation.Metrics.CustomQueries != nil &&
136151
inCluster.Spec.Instrumentation.Metrics.CustomQueries.Add != nil {
152+
137153
for _, querySet := range inCluster.Spec.Instrumentation.Metrics.CustomQueries.Add {
138154
// Create a receiver for the query set
139155
receiverName := "sqlquery/" + querySet.Name
140156
config.Receivers[receiverName] = map[string]any{
141-
"driver": "postgres",
142-
"datasource": fmt.Sprintf(`host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`, pgmonitor.MonitoringUser),
157+
"driver": "postgres",
158+
"datasource": fmt.Sprintf(
159+
`host=localhost dbname=postgres port=5432 user=%s password=${env:PGPASSWORD}`,
160+
pgmonitor.MonitoringUser),
143161
"collection_interval": querySet.CollectionInterval,
144162
// Give Postgres time to finish setup.
145163
"initial_delay": "10s",
146-
"queries": "${file:/etc/otel-collector/" + querySet.Name + "/" + querySet.Queries.Key + "}",
164+
"queries": "${file:/etc/otel-collector/" +
165+
querySet.Name + "/" + querySet.Queries.Key + "}",
147166
}
148167

149168
// Add the receiver to the pipeline
@@ -214,11 +233,13 @@ Outer:
214233
continue Outer
215234
}
216235
} else {
217-
fmt.Println("data is not a map[string]any")
236+
// TODO: What should we do if the assertion fails?
237+
fmt.Printf("%s is not a map[string]any", metric)
218238
}
219239
}
220240
} else {
221-
fmt.Println("data is not a []any")
241+
// TODO: What should we do if the assertion fails?
242+
fmt.Printf("%s is not a []any", queryAndMetrics["metrics"])
222243
}
223244
}
224245
}

0 commit comments

Comments
 (0)