diff --git a/go.mod b/go.mod index a98896d..a195dd0 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/sirupsen/logrus v1.10.2 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.12.1 + go.uber.org/goleak v1.3.0 golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 golang.org/x/tools v0.50.0 google.golang.org/protobuf v1.36.12 diff --git a/internal/cmd/collection/testdata/duplicate_labels.yaml b/internal/cmd/collection/testdata/duplicate_labels.yaml new file mode 100644 index 0000000..019bdfc --- /dev/null +++ b/internal/cmd/collection/testdata/duplicate_labels.yaml @@ -0,0 +1,37 @@ +# Copyright 2026 Cockroach Labs Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: query_count +enabled: true +scope: node +frequency: 10 +maxresults: 100 +labels: [database,database] +metrics: + - name : exec_count + kind : counter + help : statement count per application and database. +query: + SELECT + database_name as database, + sum(count) AS exec_count + FROM + crdb_internal.node_statement_statistics + WHERE + application_name NOT LIKE '$ internal-%' + GROUP BY + database_name + ORDER BY + exec_count DESC + LIMIT $1; diff --git a/internal/cmd/collection/testdata/invalid_kind.yaml b/internal/cmd/collection/testdata/invalid_kind.yaml new file mode 100644 index 0000000..3ad1bd7 --- /dev/null +++ b/internal/cmd/collection/testdata/invalid_kind.yaml @@ -0,0 +1,38 @@ +# Copyright 2026 Cockroach Labs Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: query_count +enabled: true +scope: node +frequency: 10 +maxresults: 100 +labels: [application,database] +metrics: + - name : exec_count + kind : bogus + help : statement count per application and database. +query: + SELECT + application_name as application, + database_name as database, + sum(count) AS exec_count + FROM + crdb_internal.node_statement_statistics + WHERE + application_name NOT LIKE '$ internal-%' + GROUP BY + application_name, database_name + ORDER BY + exec_count DESC + LIMIT $1; diff --git a/internal/cmd/collection/yaml.go b/internal/cmd/collection/yaml.go index a57763b..5b27718 100644 --- a/internal/cmd/collection/yaml.go +++ b/internal/cmd/collection/yaml.go @@ -80,12 +80,24 @@ func unmarshal(data []byte) (*store.Collection, error) { } metrics := make([]store.Metric, 0) for _, m := range config.Metrics { + switch strings.ToLower(m.Kind) { + case string(store.Gauge), string(store.Counter): + default: + return nil, errors.New("invalid kind") + } metrics = append(metrics, store.Metric{ Name: m.Name, - Kind: store.Kind(m.Kind), + Kind: store.Kind(strings.ToLower(m.Kind)), Help: m.Help, }) } + seenLabels := make(map[string]struct{}, len(config.Labels)) + for _, l := range config.Labels { + if _, ok := seenLabels[l]; ok { + return nil, errors.Errorf("duplicate label %q", l) + } + seenLabels[l] = struct{}{} + } var scope store.Scope switch strings.ToLower(config.Scope) { case "node": diff --git a/internal/cmd/collection/yaml_test.go b/internal/cmd/collection/yaml_test.go index 97ce05d..9e3dd6f 100644 --- a/internal/cmd/collection/yaml_test.go +++ b/internal/cmd/collection/yaml_test.go @@ -27,6 +27,12 @@ import ( //go:embed testdata/invalid_scope.yaml var invalidScope string +//go:embed testdata/invalid_kind.yaml +var invalidKind string + +//go:embed testdata/duplicate_labels.yaml +var duplicateLabels string + //go:embed testdata/malformed.yaml var malformed string @@ -97,6 +103,16 @@ func TestMarshalRoundTrip(t *testing.T) { yaml: invalidScope, wantErr: "invalid scope", }, + { + name: "invalid kind", + yaml: invalidKind, + wantErr: "invalid kind", + }, + { + name: "duplicate labels", + yaml: duplicateLabels, + wantErr: "duplicate label", + }, { name: "unmarshal error", yaml: malformed, diff --git a/internal/cmd/initialize/admin_integration_test.go b/internal/cmd/initialize/admin_integration_test.go new file mode 100644 index 0000000..88e9726 --- /dev/null +++ b/internal/cmd/initialize/admin_integration_test.go @@ -0,0 +1,77 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build integration + +package initialize + +import ( + "context" + "io" + "os" + "testing" + "time" + + "github.com/cockroachlabs/visus/internal/testutil" + "github.com/stretchr/testify/require" +) + +// TestMain starts the shared CockroachDB cluster used by every integration +// test in this package. +func TestMain(m *testing.M) { + os.Exit(testutil.StartCluster(m)) +} + +// captureStdout runs f with os.Stdout redirected to a pipe and returns +// whatever f wrote to it. admin.go prints its result with fmt.Printf +// directly to os.Stdout rather than through the cobra command's output +// writer, so cmd.SetOut has nothing to capture. +func captureStdout(t *testing.T, f func()) string { + t.Helper() + r := require.New(t) + + pipeReader, pipeWriter, err := os.Pipe() + r.NoError(err) + orig := os.Stdout + os.Stdout = pipeWriter + defer func() { os.Stdout = orig }() + + f() + + r.NoError(pipeWriter.Close()) + out, err := io.ReadAll(pipeReader) + r.NoError(err) + return string(out) +} + +// TestInitCommandIdempotent verifies that running the "init" CLI command +// twice against the same database succeeds both times, since it's the +// upgrade path an operator runs on every restart. +func TestInitCommandIdempotent(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + r := require.New(t) + + url := testutil.PGURL().String() + + for range 2 { + cmd := Command() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--url", url}) + out := captureStdout(t, func() { + r.NoError(cmd.Execute()) + }) + r.Contains(out, "Database initialized at") + } +} diff --git a/internal/cmd/initialize/admin_test.go b/internal/cmd/initialize/admin_test.go new file mode 100644 index 0000000..11891eb --- /dev/null +++ b/internal/cmd/initialize/admin_test.go @@ -0,0 +1,28 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package initialize + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestHelp ensures that the CLI command can be constructed and +// that all flag binding works. +func TestHelp(t *testing.T) { + r := require.New(t) + r.NoError(Command().Help()) +} diff --git a/internal/cmd/scan/admin_test.go b/internal/cmd/scan/admin_test.go index 1393d15..8593244 100644 --- a/internal/cmd/scan/admin_test.go +++ b/internal/cmd/scan/admin_test.go @@ -94,7 +94,7 @@ func TestCommands(t *testing.T) { mscan1, err := marshal(scan1) r.NoError(err) scan1Cfg := string(mscan1) - scan2 := &store.Scan{Name: "scan_02"} + scan2 := &store.Scan{Name: "scan_02", Format: store.CRDBv2} mscan2, err := marshal(scan2) r.NoError(err) scan2Cfg := string(mscan2) diff --git a/internal/cmd/scan/testdata/invalid_format.yaml b/internal/cmd/scan/testdata/invalid_format.yaml new file mode 100644 index 0000000..26ef585 --- /dev/null +++ b/internal/cmd/scan/testdata/invalid_format.yaml @@ -0,0 +1,18 @@ +# Copyright 2026 Cockroach Labs Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: scan_bad_format +enabled: true +format: bogus +path: testdata/sample.log diff --git a/internal/cmd/scan/testdata/malformed.yaml b/internal/cmd/scan/testdata/malformed.yaml new file mode 100644 index 0000000..c78d5b8 --- /dev/null +++ b/internal/cmd/scan/testdata/malformed.yaml @@ -0,0 +1,18 @@ +# Copyright 2026 Cockroach Labs Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: scan_malformed +enabled: notabool +format: crdb-v2 +path: testdata/sample.log diff --git a/internal/cmd/scan/yaml.go b/internal/cmd/scan/yaml.go index 49b3c2a..44f79ec 100644 --- a/internal/cmd/scan/yaml.go +++ b/internal/cmd/scan/yaml.go @@ -15,6 +15,8 @@ package scan import ( + "strings" + "github.com/cockroachdb/errors" "github.com/cockroachlabs/visus/internal/store" "github.com/creasty/defaults" @@ -81,9 +83,18 @@ func unmarshal(data []byte) (*store.Scan, error) { if config.Name == "" { return nil, errors.New("name must be specified") } + var format store.LogFormat + switch strings.ToLower(config.Format) { + case string(store.CRDBv2): + format = store.CRDBv2 + case string(store.CRDBv2Auth): + format = store.CRDBv2Auth + default: + return nil, errors.New("invalid format") + } return &store.Scan{ Enabled: config.Enabled, - Format: store.LogFormat(config.Format), + Format: format, Path: config.Path, Name: config.Name, Patterns: patterns, diff --git a/internal/cmd/scan/yaml_test.go b/internal/cmd/scan/yaml_test.go new file mode 100644 index 0000000..5c00e4d --- /dev/null +++ b/internal/cmd/scan/yaml_test.go @@ -0,0 +1,119 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package scan + +import ( + _ "embed" + "testing" + + "github.com/cockroachlabs/visus/internal/store" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +//go:embed testdata/invalid_format.yaml +var invalidFormat string + +//go:embed testdata/malformed.yaml +var malformed string + +// TestMarshalRoundTrip verifies we can marshal/unmarshal a scan configuration. +func TestMarshalRoundTrip(t *testing.T) { + tests := []struct { + name string + scan *store.Scan + yaml string + wantErr string + }{ + { + name: "good crdb-v2", + scan: &store.Scan{ + Enabled: true, + Format: store.CRDBv2, + Name: "scan_01", + Path: "testdata/sample.log", + Patterns: []store.Pattern{ + { + Help: "description", + Name: "metric", + }, + }, + }, + }, + { + name: "good crdb-v2-auth", + scan: &store.Scan{ + Enabled: true, + Format: store.CRDBv2Auth, + Name: "scan_02", + Path: "testdata/auth.log", + Patterns: []store.Pattern{ + { + Help: "auth attempts", + Name: "auth", + }, + }, + }, + }, + { + name: "no name", + scan: &store.Scan{ + Enabled: true, + Format: store.CRDBv2, + Patterns: []store.Pattern{ + { + Help: "description", + Name: "metric", + }, + }, + }, + wantErr: "name must be specified", + }, + { + name: "invalid format", + yaml: invalidFormat, + wantErr: "invalid format", + }, + { + name: "unmarshal error", + yaml: malformed, + wantErr: "cannot unmarshal", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r := require.New(t) + a := assert.New(t) + var data []byte + var err error + switch { + case test.scan != nil: + data, err = marshal(test.scan) + r.NoError(err) + case test.yaml != "": + data = []byte(test.yaml) + default: + panic("invalid test case") + } + out, err := unmarshal(data) + if test.wantErr != "" { + a.ErrorContains(err, test.wantErr) + return + } + r.NoError(err) + a.Equal(test.scan, out) + }) + } +} diff --git a/internal/cmd/server/server_integration_test.go b/internal/cmd/server/server_integration_test.go new file mode 100644 index 0000000..b01e824 --- /dev/null +++ b/internal/cmd/server/server_integration_test.go @@ -0,0 +1,84 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build integration + +package server + +import ( + "context" + "os" + "testing" + "time" + + "github.com/cockroachlabs/visus/internal/testutil" + "github.com/stretchr/testify/require" +) + +// TestMain starts the shared CockroachDB cluster used by every integration +// test in this package. +func TestMain(m *testing.M) { + os.Exit(testutil.StartCluster(m)) +} + +// TestServerNodeLifecycle verifies that RunE wires the node +// registration/heartbeat/deregistration calls correctly: the node appears +// in the store once the server starts, and disappears once the server's +// context is canceled. The store methods themselves (RegisterNode, +// Heartbeat, DeleteNode) are already covered by internal/store/node_test.go; +// this test only exercises server.go's use of them. +func TestServerNodeLifecycle(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + r := require.New(t) + + // Command()'s RunE does not initialize the schema itself; it expects + // "visus init" to have already run. testutil.NewStore does that for us + // and also gives us a Store we can poll from outside the server. + st, _ := testutil.NewStore(ctx, t) + + runCtx, stopServer := context.WithCancel(ctx) + defer stopServer() + + cmd := Command() + cmd.SetContext(runCtx) + cmd.SetArgs([]string{ + "--insecure", + "--bind-addr", "127.0.0.1:0", + "--endpoint", "/test/node-lifecycle", + "--url", testutil.PGURL().String(), + }) + + done := make(chan error, 1) + go func() { done <- cmd.Execute() }() + + r.Eventually(func() bool { + nodes, err := st.GetNodes(ctx) + return err == nil && len(nodes) == 1 + }, 15*time.Second, 200*time.Millisecond, "server did not register its node") + + stopServer() + + r.Eventually(func() bool { + nodes, err := st.GetNodes(ctx) + return err == nil && len(nodes) == 0 + }, 15*time.Second, 200*time.Millisecond, "server did not deregister its node on shutdown") + + select { + case err := <-done: + r.NoError(err) + case <-time.After(15 * time.Second): + t.Fatal("Command().Execute() did not return after shutdown") + } +} diff --git a/internal/cmd/server/server_test.go b/internal/cmd/server/server_test.go new file mode 100644 index 0000000..fc2b9ca --- /dev/null +++ b/internal/cmd/server/server_test.go @@ -0,0 +1,61 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHelp ensures that the CLI command can be constructed and +// that all flag binding works. +func TestHelp(t *testing.T) { + r := require.New(t) + r.NoError(Command().Help()) +} + +// TestFlagValidation verifies that the two flag-consistency checks at the +// top of RunE fire before any database connection is attempted, since both +// return before dialing the URL. +func TestFlagValidation(t *testing.T) { + tests := []struct { + name string + args []string + wantErr string + }{ + { + name: "missing certs without insecure", + args: []string{}, + wantErr: "--insecure must be specfied if certificates and private key are missing", + }, + { + name: "missing url", + args: []string{"--insecure"}, + wantErr: "--url must be specified", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := assert.New(t) + cmd := Command() + cmd.SilenceUsage = true + cmd.SilenceErrors = true + cmd.SetArgs(tt.args) + a.ErrorContains(cmd.Execute(), tt.wantErr) + }) + } +} diff --git a/internal/collector/collector.go b/internal/collector/collector.go index 478a362..f77dc37 100644 --- a/internal/collector/collector.go +++ b/internal/collector/collector.go @@ -141,7 +141,7 @@ func FromCollection(coll *store.Collection, registerer prometheus.Registerer) (C case store.Counter: err = res.AddCounter(m.Name, m.Help) default: - log.Errorf("%s malformed", coll.Name) + err = errors.Errorf("%s: unsupported metric kind %q for metric %s", coll.Name, m.Kind, m.Name) } if err != nil { return nil, err diff --git a/internal/collector/collector_test.go b/internal/collector/collector_test.go index 009673f..28b80c2 100644 --- a/internal/collector/collector_test.go +++ b/internal/collector/collector_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "github.com/cockroachlabs/visus/internal/store" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" @@ -270,6 +271,24 @@ func TestCollectSkipsConcurrent(t *testing.T) { c.mu.Unlock() } +// TestFromCollectionUnsupportedKind verifies that FromCollection fails +// instead of silently dropping a metric whose Kind isn't gauge or counter. +func TestFromCollectionUnsupportedKind(t *testing.T) { + _, r := assertions(t) + coll := &store.Collection{ + Name: "bad_kind", + Metrics: []store.Metric{ + { + Name: "metric", + Kind: store.Kind("bogus"), + Help: "help", + }, + }, + } + _, err := FromCollection(coll, prometheus.NewRegistry()) + r.Error(err) +} + func TestCounterLifeCycle(t *testing.T) { a, r := assertions(t) collName := "lifecycle" diff --git a/internal/collector/lease_test.go b/internal/collector/lease_test.go index 3c288fd..3409a47 100644 --- a/internal/collector/lease_test.go +++ b/internal/collector/lease_test.go @@ -33,6 +33,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/goleak" ) // testSetup resets the shared cluster's schema and returns its pg URL. @@ -118,6 +119,25 @@ func clusterCollection(name string) *store.Collection { func TestLeaseLifecycle(t *testing.T) { r := require.New(t) a := assert.New(t) + // The baseline must be captured now, before any servers start, but the + // check itself must run after newTestServer's t.Cleanup(scheduler.Stop) + // calls below: t.Cleanup callbacks run after this function's own defers, + // so a plain "defer goleak.VerifyNone" would fire first and flag each + // scheduler's still-running executor goroutine as a leak. Registering + // our own t.Cleanup here, before newTestServer registers its own, makes + // it run last (t.Cleanup is LIFO), after both schedulers have stopped. + ignoreBaseline := goleak.IgnoreCurrent() + t.Cleanup(func() { + goleak.VerifyNone(t, + ignoreBaseline, + // database.Pool has no Close method: its pools are designed to + // live for the process lifetime, so this test cannot release + // the admin/read-only connections it opens below either. That + // is a property of Pool, not of the collector's own lease and + // scheduling goroutines this test exists to verify. + goleak.IgnoreAnyFunction("github.com/jackc/pgx/v5/pgxpool.(*Pool).backgroundHealthCheck"), + ) + }) pgURL := testSetup(t) @@ -196,6 +216,14 @@ func TestLeaseLifecycle(t *testing.T) { a.Eventually(func() bool { return hasMetric(survivorReg, "handover_databases_count") }, 10*time.Second, 500*time.Millisecond, "survivor never produced metrics after handover") + + // Stop both servers and wait for their goroutines to exit before the + // goleak check above runs, so a leaked lease-holder goroutine from a + // handover does not go unnoticed. + cancel1() + cancel2() + r.NoError(stop1.Wait()) + r.NoError(stop2.Wait()) } func hasMetric(registry *prometheus.Registry, name string) bool { diff --git a/internal/database/export_test.go b/internal/database/export_test.go new file mode 100644 index 0000000..338385e --- /dev/null +++ b/internal/database/export_test.go @@ -0,0 +1,28 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// This file exposes package-private state to tests in database_test. It +// cannot import internal/testutil itself: testutil imports database, and +// this file's package declaration (database, not database_test) would +// create an import cycle if it did. +package database + +import "github.com/jackc/pgx/v5/pgxpool" + +// PoolUnderlying returns the pgxpool.Pool a Pool currently delegates to, so +// tests can confirm Refresh actually swaps and closes it rather than just +// observing that queries keep working afterward. +func PoolUnderlying(p *Pool) *pgxpool.Pool { + return p.get() +} diff --git a/internal/database/pool_integration_test.go b/internal/database/pool_integration_test.go new file mode 100644 index 0000000..bf77c1a --- /dev/null +++ b/internal/database/pool_integration_test.go @@ -0,0 +1,52 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build integration + +package database_test + +import ( + "context" + "testing" + "time" + + "github.com/cockroachlabs/visus/internal/database" + "github.com/cockroachlabs/visus/internal/testutil" + "github.com/stretchr/testify/require" +) + +// TestPoolRefresh verifies that Refresh installs a new underlying pool and +// closes the old one, rather than merely reconnecting in place. This is the +// path exercised on SIGHUP to pick up rotated certificates. +func TestPoolRefresh(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + r := require.New(t) + + pool, err := database.New(ctx, testutil.PGURL().String()) + r.NoError(err) + + oldPgxPool := database.PoolUnderlying(pool) + r.NoError(oldPgxPool.Ping(ctx)) + + r.NoError(pool.Refresh(ctx)) + + newPgxPool := database.PoolUnderlying(pool) + r.NotSame(oldPgxPool, newPgxPool, "Refresh should install a new underlying pool") + r.Error(oldPgxPool.Ping(ctx), "Refresh should close the old pool") + + var one int + r.NoError(pool.QueryRow(ctx, "SELECT 1").Scan(&one)) + r.Equal(1, one) +} diff --git a/internal/scanner/crdbv2auth_test.go b/internal/scanner/crdbv2auth_test.go index 8be0e35..bf117f0 100644 --- a/internal/scanner/crdbv2auth_test.go +++ b/internal/scanner/crdbv2auth_test.go @@ -71,6 +71,12 @@ func TestParseAuthLine(t *testing.T) { }, "", }, + { + "truncated_json", + `I250407 20:22:58.997928 731936 4@util/log/event_log.go:39 ⋮ [T1,Vsystem,n1,client=127.0.0.1:53480,hostssl,user=‹roachprod›] 15 ={"Timestamp":1744057378997924809,"EventType":"client_authentication_ok"`, + nil, + "unexpected end of JSON input", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -78,6 +84,7 @@ func TestParseAuthLine(t *testing.T) { res, err := parseAuthLine([]byte(tt.line)) if tt.wantErr != "" { a.EqualError(err, tt.wantErr) + return } a.NoError(err) a.Equal(tt.want, res) diff --git a/internal/scanner/scanner_test.go b/internal/scanner/scanner_test.go index 61a80e3..e36e5c9 100644 --- a/internal/scanner/scanner_test.go +++ b/internal/scanner/scanner_test.go @@ -57,6 +57,9 @@ var maxExpected string //go:embed testdata/cache.txt var cacheExpected string +//go:embed testdata/nonmatching.txt +var nonmatchingExpected string + // TestScanner verifies we can produce metrics from a test file. func TestScanner(t *testing.T) { tests := []struct { @@ -166,6 +169,29 @@ func TestScanner(t *testing.T) { "crdb_auth": authExpected, }, }, + { + // Real CockroachDB logs interleave lines that do not start with + // the [FEIW]yymmdd prefix, most commonly a panic stack trace. + // This verifies those lines are silently skipped rather than + // miscounted or treated as parse errors. + "non-matching prefix lines are skipped", + &store.Scan{ + Enabled: true, + Format: store.CRDBv2, + Name: "crdb", + Path: "./testdata/nonmatching.log", + Patterns: []store.Pattern{ + { + Name: "all", + Regex: "", + Help: "all events", + }, + }, + }, + map[string]string{ + "crdb_all": nonmatchingExpected, + }, + }, } for _, tt := range tests { diff --git a/internal/scanner/server_test.go b/internal/scanner/server_test.go index f7a0818..cacc2a6 100644 --- a/internal/scanner/server_test.go +++ b/internal/scanner/server_test.go @@ -26,11 +26,22 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/goleak" ) func TestRefreshScanners(t *testing.T) { r := require.New(t) a := assert.New(t) + defer goleak.VerifyNone(t, + goleak.IgnoreCurrent(), + // nxadm/tail lazily starts a single, process-wide file-watcher + // goroutine pair the first time any file is tailed, and never + // stops them: they are reused by every Tail for the lifetime of + // the process. That is a property of the library, not a leak in + // Scanner's own Start/Stop. + goleak.IgnoreTopFunction("github.com/nxadm/tail/watch.(*InotifyTracker).run"), + goleak.IgnoreAnyFunction("github.com/fsnotify/fsnotify.(*Watcher).readEvents"), + ) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -169,6 +180,50 @@ func TestRefreshScanners(t *testing.T) { a.Contains([]string{"crdblog_cache", "crdblog_max", "pebble_sstable"}, *m.Name) } + // Stop the remaining scanner and wait for its goroutines to exit before + // the goleak check above runs, so a leaked goroutine from Start/Stop + // does not go unnoticed. + cancel() + r.NoError(stopperCtx.Wait()) +} + +// TestRefreshScannersInvalidFormat verifies that a scan with an unsupported +// Format is skipped rather than failing the whole refresh. This is the +// server-layer safety net that complements the fail-fast Format validation +// added at YAML-parse time in internal/cmd/scan/yaml.go: a Format value that +// reaches the store through some other path (e.g. a stale row from a schema +// version that supported it) should not block valid scans from starting. +func TestRefreshScannersInvalidFormat(t *testing.T) { + r := require.New(t) + a := assert.New(t) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + stopperCtx := stopper.WithContext(ctx) + mockStore := &store.Memory{} + mockStore.Init(ctx) + cfg := &server.Config{} + registry := prometheus.NewRegistry() + scanners := &scannerServer{ + config: cfg, + fromBeginning: true, + registry: registry, + store: mockStore, + } + scanners.mu.scanners = make(map[string]*Scanner) + + badScan := &store.Scan{ + Enabled: true, + Format: store.LogFormat("bogus"), + Name: "badformat", + Path: "./testdata/sample.log", + } + mockStore.PutScan(ctx, badScan) + + r.NoError(scanners.Refresh(stopperCtx)) + + _, ok := scanners.get(badScan.Name) + a.False(ok, "scan with an unsupported format should not have been registered") } func waitForMetrics(ctx context.Context, registry *prometheus.Registry, expected int) error { diff --git a/internal/scanner/testdata/nonmatching.log b/internal/scanner/testdata/nonmatching.log new file mode 100644 index 0000000..a00ebd4 --- /dev/null +++ b/internal/scanner/testdata/nonmatching.log @@ -0,0 +1,6 @@ +I240418 15:31:56.563605 1 1@cli/start.go:1316 ⋮ [T1,n?] 8 using local environment variables: +panic: something went wrong +goroutine 1 [running]: +main.main() + /app/main.go:42 +0x1a +I240418 15:31:56.801319 97 server/config.go:860 ⋮ [T1,n?] 12 1 storage engine initialized diff --git a/internal/scanner/testdata/nonmatching.txt b/internal/scanner/testdata/nonmatching.txt new file mode 100644 index 0000000..d7d8db8 --- /dev/null +++ b/internal/scanner/testdata/nonmatching.txt @@ -0,0 +1,4 @@ +# HELP crdb_all all events +# TYPE crdb_all counter +crdb_all{level="I",source="cli/start.go"} 1 +crdb_all{level="I",source="server/config.go"} 1 diff --git a/internal/store/init_test.go b/internal/store/init_test.go new file mode 100644 index 0000000..3b1b507 --- /dev/null +++ b/internal/store/init_test.go @@ -0,0 +1,39 @@ +// Copyright 2026 Cockroach Labs Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build integration + +package store_test + +import ( + "context" + "testing" + "time" + + "github.com/cockroachlabs/visus/internal/testutil" + "github.com/stretchr/testify/require" +) + +// TestInitIdempotent verifies that Init can be called repeatedly against a +// schema it already created, since the schema-upgrade path relies on +// re-running the same DDL on every startup. +func TestInitIdempotent(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + r := require.New(t) + st, _ := testutil.NewStore(ctx, t) // Init already ran once inside NewStore + + r.NoError(st.Init(ctx)) + r.NoError(st.Init(ctx)) +}