Skip to content
Draft
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
89 changes: 89 additions & 0 deletions platform/view/services/storage/driver/sql/postgres/sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@
package postgres

import (
"context"
"os"
"testing"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/hyperledger-labs/fabric-smart-client/platform/view/services/storage/driver"
"github.com/hyperledger-labs/fabric-smart-client/platform/view/services/storage/driver/common"
testing2 "github.com/hyperledger-labs/fabric-smart-client/platform/view/services/storage/driver/common/testing"
common3 "github.com/hyperledger-labs/fabric-smart-client/platform/view/services/storage/driver/sql/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
_ "modernc.org/sqlite"
)

Expand Down Expand Up @@ -49,3 +55,86 @@
return p.(*KeyValueStore).KeyValueStore
})
}

func verifyExtendedLoggingInCommand(t *testing.T, config *ContainerConfig, expectExtendedLogging bool) {
cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
require.NoError(t, err)

ctx := context.Background()
Copy link
Member

Choose a reason for hiding this comment

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

you can use t.Context() as this allows the test harness to cancel the context and all probably all consumers of the context.

containers, err := cli.ContainerList(ctx, container.ListOptions{})
require.NoError(t, err)

// Find our container
var foundContainer *types.Container
for _, c := range containers {
for _, name := range c.Names {
if name == "/"+config.Container {
foundContainer = &c
break
}
}
}
require.NotNil(t, foundContainer, "Container not found")

// Inspect the container to get the command
inspect, err := cli.ContainerInspect(ctx, foundContainer.ID)
require.NoError(t, err)

// Check extended logging parameters based on expectation
cmd := inspect.Config.Cmd
if expectExtendedLogging {
assert.Contains(t, cmd, "log_destination=stderr")
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if there is a particular reason to use assert over require here. The difference is that assert does not call FailNow.

assert.Contains(t, cmd, "logging_collector=on")
assert.Contains(t, cmd, "log_min_duration_statement=0")
assert.Contains(t, cmd, "log_connections=on")
assert.Contains(t, cmd, "log_disconnections=on")
} else {
// When extended logging is disabled, these parameters should not be present
assert.NotContains(t, cmd, "log_destination=stderr")
assert.NotContains(t, cmd, "logging_collector=on")
assert.NotContains(t, cmd, "log_min_duration_statement=0")
assert.NotContains(t, cmd, "log_connections=on")
assert.NotContains(t, cmd, "log_disconnections=on")
}
}

func TestContainerStartup_ExtendedLogging(t *testing.T) {
if os.Getenv("TEST_POSTGRES") != "true" {
t.Skip("set environment variable TEST_POSTGRES to true to include postgres test")
}
if testing.Short() {
t.Skip("skipping postgres container test in short mode")
}

t.Run("Container starts successfully with extended logging enabled", func(t *testing.T) {
config := DefaultConfig("issuer-extended")
config.DbConfig.ExtendedLogging = true

Check failure on line 111 in platform/view/services/storage/driver/sql/postgres/sql_test.go

View workflow job for this annotation

GitHub Actions / lint

QF1008: could remove embedded field "DbConfig" from selector (staticcheck)

closeFunc, err := startPostgresWithLogger(*config, t, false)
assert.NoError(t, err)
assert.NotNil(t, closeFunc)

// Verify the container command includes extended logging parameters
verifyExtendedLoggingInCommand(t, config, true)

if closeFunc != nil {
closeFunc()
}
})

t.Run("Container starts successfully with extended logging disabled", func(t *testing.T) {
config := DefaultConfig("issuer-normal")
config.DbConfig.ExtendedLogging = false

Check failure on line 127 in platform/view/services/storage/driver/sql/postgres/sql_test.go

View workflow job for this annotation

GitHub Actions / lint

QF1008: could remove embedded field "DbConfig" from selector (staticcheck)

closeFunc, err := startPostgresWithLogger(*config, t, false)
assert.NoError(t, err)
assert.NotNil(t, closeFunc)

// Verify the container command does not include extended logging parameters
verifyExtendedLoggingInCommand(t, config, false)

if closeFunc != nil {
closeFunc()
}
})
}
36 changes: 29 additions & 7 deletions platform/view/services/storage/driver/sql/postgres/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,12 @@ type ContainerConfig struct {
}

type DbConfig struct {
DBName string
User string
Pass string
Host string
Port int
DBName string
User string
Pass string
Host string
Port int
ExtendedLogging bool // enables extended loggings for PostgreSQL
}

func (c *DbConfig) DataSource() string {
Expand Down Expand Up @@ -163,7 +164,7 @@ func startPostgresWithLogger(c ContainerConfig, t Logger, printLogs bool) (func(
return nil, fmt.Errorf("can't start postgres: %s", err)
}
ctx := context.Background()
resp, err := cli.ContainerCreate(ctx, &container.Config{
containerConfig := &container.Config{
Env: []string{
"POSTGRES_DB=" + c.DBName,
"POSTGRES_USER=" + c.User,
Expand All @@ -181,7 +182,28 @@ func startPostgresWithLogger(c ContainerConfig, t Logger, printLogs bool) (func(
Timeout: time.Second,
Retries: 10,
},
}, &container.HostConfig{
}

// Add extended logging configuration if enabled
if c.ExtendedLogging {
containerConfig.Cmd = []string{
Copy link
Member

Choose a reason for hiding this comment

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

The ExtendedLogging toggle probably makes sense for many cases and I can imagine that when debugging with postgres, sometimes more flexibility is needed when investigating an odd issue.

I think it would be nice to allow the Cmd be overwritten via env vars to let the user inject any postgres command / env vars via testing without touching this code all the time.

"postgres",
"-c", "log_destination=stderr",
"-c", "logging_collector=on",
"-c", "log_directory=pg_log",
"-c", "log_filename=postgresql-%Y-%m-%d_%H%M%S.log",
"-c", "log_min_duration_statement=0",
"-c", "log_line_prefix=%t [%p]: [%l-1] user=%u,db=%d,app=%a,client=%h ",
"-c", "log_checkpoints=on",
"-c", "log_connections=on",
"-c", "log_disconnections=on",
"-c", "log_lock_waits=on",
"-c", "log_temp_files=0",
"-c", "log_autovacuum_min_duration=0",
}
}

resp, err := cli.ContainerCreate(ctx, containerConfig, &container.HostConfig{
PortBindings: nat.PortMap{
nat.Port("5432/tcp"): []nat.PortBinding{
{
Expand Down
Loading