Skip to content
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.

Sign up for GitHub

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

Ensure OTel directories are writable #4090

Merged
merged 5 commits into from
Feb 13, 2025
Merged

Ensure OTel directories are writable #4090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b7e0145
Add shared functions for quoting shell words
cbandy Nov 4, 2024
1aaa941
Add a function for setting permission on directories
cbandy Feb 10, 2025
dd8c320
Store pgAdmin log file positions in the logs directory
cbandy Feb 6, 2025
e78b4ef
Ensure Postgres and Patroni log directories are writable
cbandy Feb 10, 2025
7cdd56f
Ensure pgBackRest log directories are writable
cbandy Feb 11, 2025
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
1 change: 1 addition & 0 deletions internal/collector/patroni.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func EnablePatroniLogging(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/patroni_logs"] = map[string]any{
Expand Down
27 changes: 11 additions & 16 deletions internal/collector/pgadmin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,20 @@ func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec
return nil
}
otelConfig := NewConfig(spec)
otelConfig.Extensions["file_storage/pgadmin"] = map[string]any{
"directory": "/var/log/pgadmin/receiver",
"create_directory": true,
"fsync": true,
}
otelConfig.Extensions["file_storage/gunicorn"] = map[string]any{
"directory": "/var/log/gunicorn" + "/receiver",
"create_directory": true,

otelConfig.Extensions["file_storage/pgadmin_data_logs"] = map[string]any{
"directory": "/var/lib/pgadmin/logs/receiver",
"create_directory": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so we're creating the /receiver directory in pod.go

shell.MakeDirectories(0o775, dataMountPath, path.Join(LogDirectoryAbsolutePath, "receiver")),

hmmm, not sure I have a question here, just interested in the different way we're doing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've put TODOs elsewhere to create the directory using our function. This create_directory option does not do what we need in Kubernetes; opentelemetry-collector-contrib#37774

IMV, this directory is a collector concern and belongs in its (the sidecar's) shell entrypoint or other init.

"fsync": true,
}

otelConfig.Receivers["filelog/pgadmin"] = map[string]any{
"include": []string{"/var/lib/pgadmin/logs/pgadmin.log"},
"storage": "file_storage/pgadmin",
"storage": "file_storage/pgadmin_data_logs",
}
otelConfig.Receivers["filelog/gunicorn"] = map[string]any{
"include": []string{"/var/lib/pgadmin/logs/gunicorn.log"},
"storage": "file_storage/gunicorn",
"storage": "file_storage/pgadmin_data_logs",
}

otelConfig.Processors["resource/pgadmin"] = map[string]any{
Expand Down Expand Up @@ -101,7 +97,7 @@ func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec
}

otelConfig.Pipelines["logs/pgadmin"] = Pipeline{
Extensions: []ComponentID{"file_storage/pgadmin"},
Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
Receivers: []ComponentID{"filelog/pgadmin"},
Processors: []ComponentID{
"resource/pgadmin",
Expand All @@ -113,7 +109,7 @@ func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec
}

otelConfig.Pipelines["logs/gunicorn"] = Pipeline{
Extensions: []ComponentID{"file_storage/gunicorn"},
Extensions: []ComponentID{"file_storage/pgadmin_data_logs"},
Receivers: []ComponentID{"filelog/gunicorn"},
Processors: []ComponentID{
"resource/pgadmin",
Expand All @@ -125,9 +121,8 @@ func EnablePgAdminLogging(ctx context.Context, spec *v1beta1.InstrumentationSpec
}

otelYAML, err := otelConfig.ToYAML()
if err != nil {
return err
if err == nil {
configmap.Data["collector.yaml"] = otelYAML
}
configmap.Data["collector.yaml"] = otelYAML
return nil
return err
}
71 changes: 36 additions & 35 deletions internal/collector/pgadmin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
//
// SPDX-License-Identifier: Apache-2.0

package collector
package collector_test

import (
"context"
"testing"

"gotest.tools/v3/assert"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/yaml"

"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/naming"
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
)
Expand All @@ -27,10 +29,9 @@ func TestEnablePgAdminLogging(t *testing.T) {

ctx := feature.NewContext(context.Background(), gate)

pgadmin := new(v1beta1.PGAdmin)
configmap := &corev1.ConfigMap{ObjectMeta: naming.StandalonePGAdmin(pgadmin)}
configmap := new(corev1.ConfigMap)
initialize.Map(&configmap.Data)
err := EnablePgAdminLogging(ctx, pgadmin.Spec.Instrumentation, configmap)
err := collector.EnablePgAdminLogging(ctx, nil, configmap)
assert.NilError(t, err)

assert.Assert(t, cmp.MarshalMatches(configmap.Data, `
Expand All @@ -41,13 +42,9 @@ collector.yaml: |
debug:
verbosity: detailed
extensions:
file_storage/gunicorn:
create_directory: true
directory: /var/log/gunicorn/receiver
fsync: true
file_storage/pgadmin:
create_directory: true
directory: /var/log/pgadmin/receiver
file_storage/pgadmin_data_logs:
create_directory: false
directory: `+pgadmin.LogDirectoryAbsolutePath+`/receiver
fsync: true
processors:
batch/1s:
Expand Down Expand Up @@ -86,16 +83,15 @@ collector.yaml: |
receivers:
filelog/gunicorn:
include:
- /var/lib/pgadmin/logs/gunicorn.log
storage: file_storage/gunicorn
- `+pgadmin.GunicornLogFileAbsolutePath+`
storage: file_storage/pgadmin_data_logs
filelog/pgadmin:
include:
- /var/lib/pgadmin/logs/pgadmin.log
storage: file_storage/pgadmin
- `+pgadmin.LogFileAbsolutePath+`
storage: file_storage/pgadmin_data_logs
service:
extensions:
- file_storage/gunicorn
- file_storage/pgadmin
- file_storage/pgadmin_data_logs
pipelines:
logs/gunicorn:
exporters:
Expand Down Expand Up @@ -128,12 +124,22 @@ collector.yaml: |

ctx := feature.NewContext(context.Background(), gate)

pgadmin := new(v1beta1.PGAdmin)
pgadmin.Spec.Instrumentation = testInstrumentationSpec()
var spec v1beta1.InstrumentationSpec
assert.NilError(t, yaml.Unmarshal([]byte(`{
config: {
exporters: {
googlecloud: {
log: { default_log_name: opentelemetry.io/collector-exported-log },
project: google-project-name,
},
},
},
logs: { exporters: [googlecloud] },
}`), &spec))

configmap := &corev1.ConfigMap{ObjectMeta: naming.StandalonePGAdmin(pgadmin)}
configmap := new(corev1.ConfigMap)
initialize.Map(&configmap.Data)
err := EnablePgAdminLogging(ctx, pgadmin.Spec.Instrumentation, configmap)
err := collector.EnablePgAdminLogging(ctx, &spec, configmap)
assert.NilError(t, err)

assert.Assert(t, cmp.MarshalMatches(configmap.Data, `
Expand All @@ -148,13 +154,9 @@ collector.yaml: |
default_log_name: opentelemetry.io/collector-exported-log
project: google-project-name
extensions:
file_storage/gunicorn:
create_directory: true
directory: /var/log/gunicorn/receiver
fsync: true
file_storage/pgadmin:
create_directory: true
directory: /var/log/pgadmin/receiver
file_storage/pgadmin_data_logs:
create_directory: false
directory: `+pgadmin.LogDirectoryAbsolutePath+`/receiver
fsync: true
processors:
batch/1s:
Expand Down Expand Up @@ -193,16 +195,15 @@ collector.yaml: |
receivers:
filelog/gunicorn:
include:
- /var/lib/pgadmin/logs/gunicorn.log
storage: file_storage/gunicorn
- `+pgadmin.GunicornLogFileAbsolutePath+`
storage: file_storage/pgadmin_data_logs
filelog/pgadmin:
include:
- /var/lib/pgadmin/logs/pgadmin.log
storage: file_storage/pgadmin
- `+pgadmin.LogFileAbsolutePath+`
storage: file_storage/pgadmin_data_logs
service:
extensions:
- file_storage/gunicorn
- file_storage/pgadmin
- file_storage/pgadmin_data_logs
pipelines:
logs/gunicorn:
exporters:
Expand Down
1 change: 1 addition & 0 deletions internal/collector/pgbackrest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ 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,
Expand Down
1 change: 1 addition & 0 deletions internal/collector/pgbouncer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ 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{
Expand Down
2 changes: 2 additions & 0 deletions internal/collector/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func EnablePostgresLogging(

// 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/postgres_logs"] = map[string]any{
Expand Down Expand Up @@ -215,6 +216,7 @@ 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,
Expand Down
Loading