Skip to content

Commit d211384

Browse files
benjaminjbbenjaminjb
committed
PR feedback
1 parent 447360b commit d211384

File tree

4 files changed

+120
-127
lines changed

4 files changed

+120
-127
lines changed

‎internal/collector/pgadmin.go‎

Lines changed: 105 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -17,113 +17,114 @@ import (
1717
func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec,
1818
configmap *corev1.ConfigMap,
1919
) error {
20-
var err error
21-
if OpenTelemetryLogsEnabled(ctx, spec) {
22-
23-
otelConfig := NewConfig(spec)
24-
25-
otelConfig.Extensions["file_storage/pgadmin_data_logs"] = map[string]any{
26-
"directory": "/var/lib/pgadmin/logs/receiver",
27-
"create_directory": false,
28-
"fsync": true,
29-
}
30-
31-
otelConfig.Receivers["filelog/pgadmin"] = map[string]any{
32-
"include": []string{"/var/lib/pgadmin/logs/pgadmin.log"},
33-
"storage": "file_storage/pgadmin_data_logs",
34-
}
35-
otelConfig.Receivers["filelog/gunicorn"] = map[string]any{
36-
"include": []string{"/var/lib/pgadmin/logs/gunicorn.log"},
37-
"storage": "file_storage/pgadmin_data_logs",
38-
}
39-
40-
otelConfig.Processors["resource/pgadmin"] = map[string]any{
41-
"attributes": []map[string]any{
42-
// Container and Namespace names need no escaping because they are DNS labels.
43-
// Pod names need no escaping because they are DNS subdomains.
44-
//
45-
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names
46-
// https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/resource/k8s.md
47-
// https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/general/logs.md
48-
{"action": "insert", "key": "k8s.container.name", "value": naming.ContainerPGAdmin},
49-
{"action": "insert", "key": "k8s.namespace.name", "value": "${env:K8S_POD_NAMESPACE}"},
50-
{"action": "insert", "key": "k8s.pod.name", "value": "${env:K8S_POD_NAME}"},
51-
},
52-
}
53-
54-
otelConfig.Processors["transform/pgadmin_log"] = map[string]any{
55-
"log_statements": []map[string]any{
56-
{
57-
"context": "log",
58-
"statements": []string{
59-
// Keep the unparsed log record in a standard attribute, and replace
60-
// the log record body with the message field.
61-
//
62-
// https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/general/logs.md
63-
`set(attributes["log.record.original"], body)`,
64-
`set(cache, ParseJSON(body))`,
65-
`merge_maps(attributes, ExtractPatterns(cache["message"], "(?P<webrequest>[A-Z]{3}.*?[\\d]{3})"), "insert")`,
66-
`set(body, cache["message"])`,
67-
68-
// Set instrumentation scope to the "name" from each log record.
69-
`set(instrumentation_scope.name, cache["name"])`,
70-
71-
// https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitytext
72-
`set(severity_text, cache["level"])`,
73-
`set(time_unix_nano, Int(cache["time"]*1000000000))`,
74-
75-
// Map pgAdmin "logging levels" to OpenTelemetry severity levels.
76-
//
77-
// https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitynumber
78-
// https://opentelemetry.io/docs/specs/otel/logs/data-model-appendix/#appendix-b-severitynumber-example-mappings
79-
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/pkg/ottl/contexts/ottllog#enums
80-
`set(severity_number, SEVERITY_NUMBER_DEBUG) where severity_text == "DEBUG"`,
81-
`set(severity_number, SEVERITY_NUMBER_INFO) where severity_text == "INFO"`,
82-
`set(severity_number, SEVERITY_NUMBER_WARN) where severity_text == "WARNING"`,
83-
`set(severity_number, SEVERITY_NUMBER_ERROR) where severity_text == "ERROR"`,
84-
`set(severity_number, SEVERITY_NUMBER_FATAL) where severity_text == "CRITICAL"`,
85-
},
20+
if !OpenTelemetryLogsEnabled(ctx, spec) {
21+
return nil
22+
}
23+
24+
otelConfig := NewConfig(spec)
25+
26+
otelConfig.Extensions["file_storage/pgadmin_data_logs"] = map[string]any{
27+
"directory": "/var/lib/pgadmin/logs/receiver",
28+
"create_directory": false,
29+
"fsync": true,
30+
}
31+
32+
otelConfig.Receivers["filelog/pgadmin"] = map[string]any{
33+
"include": []string{"/var/lib/pgadmin/logs/pgadmin.log"},
34+
"storage": "file_storage/pgadmin_data_logs",
35+
}
36+
otelConfig.Receivers["filelog/gunicorn"] = map[string]any{
37+
"include": []string{"/var/lib/pgadmin/logs/gunicorn.log"},
38+
"storage": "file_storage/pgadmin_data_logs",
39+
}
40+
41+
otelConfig.Processors["resource/pgadmin"] = map[string]any{
42+
"attributes": []map[string]any{
43+
// Container and Namespace names need no escaping because they are DNS labels.
44+
// Pod names need no escaping because they are DNS subdomains.
45+
//
46+
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names
47+
// https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/resource/k8s.md
48+
// https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/general/logs.md
49+
{"action": "insert", "key": "k8s.container.name", "value": naming.ContainerPGAdmin},
50+
{"action": "insert", "key": "k8s.namespace.name", "value": "${env:K8S_POD_NAMESPACE}"},
51+
{"action": "insert", "key": "k8s.pod.name", "value": "${env:K8S_POD_NAME}"},
52+
},
53+
}
54+
55+
otelConfig.Processors["transform/pgadmin_log"] = map[string]any{
56+
"log_statements": []map[string]any{
57+
{
58+
"context": "log",
59+
"statements": []string{
60+
// Keep the unparsed log record in a standard attribute, and replace
61+
// the log record body with the message field.
62+
//
63+
// https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/general/logs.md
64+
`set(attributes["log.record.original"], body)`,
65+
`set(cache, ParseJSON(body))`,
66+
`merge_maps(attributes, ExtractPatterns(cache["message"], "(?P<webrequest>[A-Z]{3}.*?[\\d]{3})"), "insert")`,
67+
`set(body, cache["message"])`,
68+
69+
// Set instrumentation scope to the "name" from each log record.
70+
`set(instrumentation_scope.name, cache["name"])`,
71+
72+
// https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitytext
73+
`set(severity_text, cache["level"])`,
74+
`set(time_unix_nano, Int(cache["time"]*1000000000))`,
75+
76+
// Map pgAdmin "logging levels" to OpenTelemetry severity levels.
77+
//
78+
// https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitynumber
79+
// https://opentelemetry.io/docs/specs/otel/logs/data-model-appendix/#appendix-b-severitynumber-example-mappings
80+
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/-/pkg/ottl/contexts/ottllog#enums
81+
`set(severity_number, SEVERITY_NUMBER_DEBUG) where severity_text == "DEBUG"`,
82+
`set(severity_number, SEVERITY_NUMBER_INFO) where severity_text == "INFO"`,
83+
`set(severity_number, SEVERITY_NUMBER_WARN) where severity_text == "WARNING"`,
84+
`set(severity_number, SEVERITY_NUMBER_ERROR) where severity_text == "ERROR"`,
85+
`set(severity_number, SEVERITY_NUMBER_FATAL) where severity_text == "CRITICAL"`,
8686
},
8787
},
88-
}
89-
90-
// If there are exporters to be added to the logs pipelines defined in
91-
// the spec, add them to the pipeline. Otherwise, add the DebugExporter.
92-
exporters := []ComponentID{DebugExporter}
93-
if spec != nil && spec.Logs != nil && spec.Logs.Exporters != nil {
94-
exporters = slices.Clone(spec.Logs.Exporters)
95-
}
96-
97-
otelConfig.Pipelines["logs/pgadmin"] = Pipeline{
98-
Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
99-
Receivers: []ComponentID{"filelog/pgadmin"},
100-
Processors: []ComponentID{
101-
"resource/pgadmin",
102-
"transform/pgadmin_log",
103-
ResourceDetectionProcessor,
104-
LogsBatchProcessor,
105-
CompactingProcessor,
106-
},
107-
Exporters: exporters,
108-
}
109-
110-
otelConfig.Pipelines["logs/gunicorn"] = Pipeline{
111-
Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
112-
Receivers: []ComponentID{"filelog/gunicorn"},
113-
Processors: []ComponentID{
114-
"resource/pgadmin",
115-
"transform/pgadmin_log",
116-
ResourceDetectionProcessor,
117-
LogsBatchProcessor,
118-
CompactingProcessor,
119-
},
120-
Exporters: exporters,
121-
}
88+
},
89+
}
90+
91+
// If there are exporters to be added to the logs pipelines defined in
92+
// the spec, add them to the pipeline. Otherwise, add the DebugExporter.
93+
exporters := []ComponentID{DebugExporter}
94+
if spec != nil && spec.Logs != nil && spec.Logs.Exporters != nil {
95+
exporters = slices.Clone(spec.Logs.Exporters)
96+
}
12297

123-
otelYAML, err := otelConfig.ToYAML()
124-
if err == nil {
125-
configmap.Data["collector.yaml"] = otelYAML
126-
}
98+
otelConfig.Pipelines["logs/pgadmin"] = Pipeline{
99+
Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
100+
Receivers: []ComponentID{"filelog/pgadmin"},
101+
Processors: []ComponentID{
102+
"resource/pgadmin",
103+
"transform/pgadmin_log",
104+
ResourceDetectionProcessor,
105+
LogsBatchProcessor,
106+
CompactingProcessor,
107+
},
108+
Exporters: exporters,
127109
}
110+
111+
otelConfig.Pipelines["logs/gunicorn"] = Pipeline{
112+
Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
113+
Receivers: []ComponentID{"filelog/gunicorn"},
114+
Processors: []ComponentID{
115+
"resource/pgadmin",
116+
"transform/pgadmin_log",
117+
ResourceDetectionProcessor,
118+
LogsBatchProcessor,
119+
CompactingProcessor,
120+
},
121+
Exporters: exporters,
122+
}
123+
124+
otelYAML, err := otelConfig.ToYAML()
125+
if err == nil {
126+
configmap.Data["collector.yaml"] = otelYAML
127+
}
128+
128129
return err
129130
}

‎internal/collector/pgadmin_test.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,11 @@ func TestEnablePgAdminLogging(t *testing.T) {
3030

3131
configmap := new(corev1.ConfigMap)
3232
initialize.Map(&configmap.Data)
33-
retentionPeriod, err := v1beta1.NewDuration("12 hours")
34-
assert.NilError(t, err)
35-
instrumentation := v1beta1.InstrumentationSpec{
36-
Logs: &v1beta1.InstrumentationLogsSpec{
37-
RetentionPeriod: retentionPeriod,
38-
},
39-
}
40-
err = collector.EnablePgAdminLogging(ctx, &instrumentation, configmap)
33+
var instrumentation *v1beta1.InstrumentationSpec
34+
require.UnmarshalInto(t, &instrumentation, `{
35+
logs: { retentionPeriod: 12h },
36+
}`)
37+
err := collector.EnablePgAdminLogging(ctx, instrumentation, configmap)
4138
assert.NilError(t, err)
4239

4340
assert.Assert(t, cmp.MarshalMatches(configmap.Data, `

‎internal/collector/pgbackrest_test.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"gotest.tools/v3/assert"
1212

1313
"github.com/crunchydata/postgres-operator/internal/feature"
14+
"github.com/crunchydata/postgres-operator/internal/testing/require"
1415
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1516
)
1617

@@ -27,15 +28,12 @@ func TestNewConfigForPgBackrestRepoHostPod(t *testing.T) {
2728
Volume: new(v1beta1.RepoPVC),
2829
},
2930
}
30-
retentionPeriod, err := v1beta1.NewDuration("12 hours")
31-
assert.NilError(t, err)
32-
instrumentation := v1beta1.InstrumentationSpec{
33-
Logs: &v1beta1.InstrumentationLogsSpec{
34-
RetentionPeriod: retentionPeriod,
35-
},
36-
}
31+
var instrumentation *v1beta1.InstrumentationSpec
32+
require.UnmarshalInto(t, &instrumentation, `{
33+
logs: { retentionPeriod: 12h },
34+
}`)
3735

38-
config := NewConfigForPgBackrestRepoHostPod(ctx, &instrumentation, repos)
36+
config := NewConfigForPgBackrestRepoHostPod(ctx, instrumentation, repos)
3937

4038
result, err := config.ToYAML()
4139
assert.NilError(t, err)

‎internal/controller/standalone_pgadmin/pod_test.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/crunchydata/postgres-operator/internal/initialize"
1717
"github.com/crunchydata/postgres-operator/internal/kubernetes"
1818
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
19+
"github.com/crunchydata/postgres-operator/internal/testing/require"
1920
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2021
)
2122

@@ -211,13 +212,9 @@ volumes:
211212
pgadmin.Spec.Resources.Requests = corev1.ResourceList{
212213
corev1.ResourceCPU: resource.MustParse("100m"),
213214
}
214-
retentionPeriod, err := v1beta1.NewDuration("12 hours")
215-
assert.NilError(t, err)
216-
pgadmin.Spec.Instrumentation = &v1beta1.InstrumentationSpec{
217-
Logs: &v1beta1.InstrumentationLogsSpec{
218-
RetentionPeriod: retentionPeriod,
219-
},
220-
}
215+
require.UnmarshalInto(t, &pgadmin.Spec.Instrumentation, `{
216+
logs: { retentionPeriod: 12h },
217+
}`)
221218

222219
call()
223220

0 commit comments

Comments
 (0)