Skip to content

OTel: Allow users to add metrics exporters. #4189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2301,6 +2301,13 @@ spec:
type: string
type: array
type: object
exporters:
description: The names of exporters that should send metrics.
items:
type: string
minItems: 1
type: array
x-kubernetes-list-type: set
perDBMetricTargets:
description: User defined databases to target for default
per-db metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12146,6 +12146,13 @@ spec:
type: string
type: array
type: object
exporters:
description: The names of exporters that should send metrics.
items:
type: string
minItems: 1
type: array
x-kubernetes-list-type: set
perDBMetricTargets:
description: User defined databases to target for default
per-db metrics
Expand Down
3 changes: 3 additions & 0 deletions internal/collector/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ func testInstrumentationSpec() *v1beta1.InstrumentationSpec {
Logs: &v1beta1.InstrumentationLogsSpec{
Exporters: []string{"googlecloud"},
},
Metrics: &v1beta1.InstrumentationMetricsSpec{
Exporters: []string{"googlecloud"},
},
}

return spec.DeepCopy()
Expand Down
2 changes: 1 addition & 1 deletion internal/collector/naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const DebugExporter = "debug"
const LogsBatchProcessor = "batch/logs"
const OneSecondBatchProcessor = "batch/1s"
const SubSecondBatchProcessor = "batch/200ms"
const Prometheus = "prometheus"
const Prometheus = "prometheus/cpk-monitoring"
const PrometheusPort = 9187
const PGBouncerMetrics = "metrics/pgbouncer"
const PostgresMetrics = "metrics/postgres"
Expand Down
10 changes: 9 additions & 1 deletion internal/collector/patroni.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,22 @@ func EnablePatroniMetrics(ctx context.Context,
},
}

// If there are exporters to be added to the metrics pipelines defined
// in the spec, add them to the pipeline.
exporters := []ComponentID{Prometheus}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's no way for a user to remove the Prometheus exporter? (Of course, if they did, we'd maybe also want to change the way we expose the port?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I'm not opposed to providing a way to remove it. I'm just not sure that we should remove it by default whenever the user provides their own exporters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep thinking about the way we deal with debug for logs as a model, but maybe that's too different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah feels pretty different to me. The debug exporter is there just so that there is some way to view logs if the user doesn't provide another solution, but the Prometheus exporter is key for integrating into our monitoring stack.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed -- we can use this PR to add other exporters and maybe another to remove Prometheus/remove the exposed port (if we want to. I can imagine someone who wants their own exporter might not want Prometheus at all).

(Though also I'm still curious if there's other ports to expose or other changes for other metrics exporters, but at least we've covered the major ones, I think.)

if inCluster.Spec.Instrumentation.Metrics != nil &&
inCluster.Spec.Instrumentation.Metrics.Exporters != nil {
exporters = append(exporters, inCluster.Spec.Instrumentation.Metrics.Exporters...)
}

// Add Metrics Pipeline
outConfig.Pipelines[PatroniMetrics] = Pipeline{
Receivers: []ComponentID{Prometheus},
Processors: []ComponentID{
SubSecondBatchProcessor,
CompactingProcessor,
},
Exporters: []ComponentID{Prometheus},
Exporters: exporters,
}
}
}
140 changes: 136 additions & 4 deletions internal/collector/patroni_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func TestEnablePatroniLogging(t *testing.T) {
t.Run("NilInstrumentationSpec", func(t *testing.T) {
t.Run("EmptyInstrumentationSpec", func(t *testing.T) {
gate := feature.NewGate()
assert.NilError(t, gate.SetFromMap(map[string]bool{
feature.OpenTelemetryLogs: true,
Expand All @@ -26,9 +26,7 @@ func TestEnablePatroniLogging(t *testing.T) {
config := NewConfig(nil)
cluster := new(v1beta1.PostgresCluster)
require.UnmarshalInto(t, &cluster.Spec, `{
instrumentation: {
logs: { retentionPeriod: 5h },
},
instrumentation: {}
}`)

EnablePatroniLogging(ctx, cluster, config)
Expand Down Expand Up @@ -216,3 +214,137 @@ service:
`)
})
}

func TestEnablePatroniMetrics(t *testing.T) {
t.Run("EmptyInstrumentationSpec", func(t *testing.T) {
gate := feature.NewGate()
assert.NilError(t, gate.SetFromMap(map[string]bool{
feature.OpenTelemetryMetrics: true,
}))
ctx := feature.NewContext(context.Background(), gate)

config := NewConfig(nil)
cluster := new(v1beta1.PostgresCluster)
require.UnmarshalInto(t, &cluster.Spec, `{
instrumentation: {}
}`)

EnablePatroniMetrics(ctx, cluster, config)

result, err := config.ToYAML()
assert.NilError(t, err)
assert.DeepEqual(t, result, `# Generated by postgres-operator. DO NOT EDIT.
# Your changes will not be saved.
exporters:
debug:
verbosity: detailed
prometheus/cpk-monitoring:
endpoint: 0.0.0.0:9187
extensions: {}
processors:
batch/1s:
timeout: 1s
batch/200ms:
timeout: 200ms
batch/logs:
send_batch_size: 8192
timeout: 200ms
groupbyattrs/compact: {}
resourcedetection:
detectors: []
override: false
timeout: 30s
receivers:
prometheus/cpk-monitoring:
config:
scrape_configs:
- job_name: patroni
scheme: https
scrape_interval: 10s
static_configs:
- targets:
- 0.0.0.0:8008
tls_config:
insecure_skip_verify: true
service:
extensions: []
pipelines:
metrics/patroni:
exporters:
- prometheus/cpk-monitoring
processors:
- batch/200ms
- groupbyattrs/compact
receivers:
- prometheus/cpk-monitoring
`)
})

t.Run("InstrumentationSpecDefined", func(t *testing.T) {
gate := feature.NewGate()
assert.NilError(t, gate.SetFromMap(map[string]bool{
feature.OpenTelemetryMetrics: true,
}))
ctx := feature.NewContext(context.Background(), gate)

cluster := new(v1beta1.PostgresCluster)
cluster.Spec.Instrumentation = testInstrumentationSpec()
config := NewConfig(cluster.Spec.Instrumentation)

EnablePatroniMetrics(ctx, cluster, config)

result, err := config.ToYAML()
assert.NilError(t, err)
assert.DeepEqual(t, result, `# Generated by postgres-operator. DO NOT EDIT.
# Your changes will not be saved.
exporters:
debug:
verbosity: detailed
googlecloud:
log:
default_log_name: opentelemetry.io/collector-exported-log
project: google-project-name
prometheus/cpk-monitoring:
endpoint: 0.0.0.0:9187
extensions: {}
processors:
batch/1s:
timeout: 1s
batch/200ms:
timeout: 200ms
batch/logs:
send_batch_size: 8192
timeout: 200ms
groupbyattrs/compact: {}
resourcedetection:
detectors: []
override: false
timeout: 30s
receivers:
prometheus/cpk-monitoring:
config:
scrape_configs:
- job_name: patroni
scheme: https
scrape_interval: 10s
static_configs:
- targets:
- 0.0.0.0:8008
tls_config:
insecure_skip_verify: true
service:
extensions: []
pipelines:
metrics/patroni:
exporters:
- prometheus/cpk-monitoring
- googlecloud
processors:
- batch/200ms
- groupbyattrs/compact
receivers:
- prometheus/cpk-monitoring
`)

})
}
6 changes: 2 additions & 4 deletions internal/collector/pgadmin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

func TestEnablePgAdminLogging(t *testing.T) {
t.Run("NilInstrumentationSpec", func(t *testing.T) {
t.Run("EmptyInstrumentationSpec", func(t *testing.T) {
gate := feature.NewGate()
assert.NilError(t, gate.SetFromMap(map[string]bool{
feature.OpenTelemetryLogs: true,
Expand All @@ -31,9 +31,7 @@ func TestEnablePgAdminLogging(t *testing.T) {
configmap := new(corev1.ConfigMap)
initialize.Map(&configmap.Data)
var instrumentation *v1beta1.InstrumentationSpec
require.UnmarshalInto(t, &instrumentation, `{
logs: { retentionPeriod: 12h },
}`)
require.UnmarshalInto(t, &instrumentation, `{}`)
err := collector.EnablePgAdminLogging(ctx, instrumentation, configmap)
assert.NilError(t, err)

Expand Down
6 changes: 2 additions & 4 deletions internal/collector/pgbackrest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func TestNewConfigForPgBackrestRepoHostPod(t *testing.T) {
t.Run("NilInstrumentationSpec", func(t *testing.T) {
t.Run("EmptyInstrumentationSpec", func(t *testing.T) {
gate := feature.NewGate()
assert.NilError(t, gate.SetFromMap(map[string]bool{
feature.OpenTelemetryLogs: true,
Expand All @@ -29,9 +29,7 @@ func TestNewConfigForPgBackrestRepoHostPod(t *testing.T) {
},
}
var instrumentation *v1beta1.InstrumentationSpec
require.UnmarshalInto(t, &instrumentation, `{
logs: { retentionPeriod: 12h },
}`)
require.UnmarshalInto(t, &instrumentation, `{}`)

config := NewConfigForPgBackrestRepoHostPod(ctx, instrumentation, repos)

Expand Down
10 changes: 9 additions & 1 deletion internal/collector/pgbouncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,22 @@ func EnablePgBouncerMetrics(ctx context.Context, inCluster *v1beta1.PostgresClus
"queries": slices.Clone(pgBouncerMetricsQueries),
}

// If there are exporters to be added to the metrics pipelines defined
// in the spec, add them to the pipeline.
exporters := []ComponentID{Prometheus}
if inCluster.Spec.Instrumentation.Metrics != nil &&
inCluster.Spec.Instrumentation.Metrics.Exporters != nil {
exporters = append(exporters, inCluster.Spec.Instrumentation.Metrics.Exporters...)
}

// Add Metrics Pipeline
config.Pipelines[PGBouncerMetrics] = Pipeline{
Receivers: []ComponentID{SqlQuery},
Processors: []ComponentID{
SubSecondBatchProcessor,
CompactingProcessor,
},
Exporters: []ComponentID{Prometheus},
Exporters: exporters,
}
}
}
Loading
Loading