Allow proxy service work without metrics DB to allow db drop (#97)

feat: allow proxy service work without metrics DB to allow db drop; includes improves for ci and database disabling via env.

.github/workflows/ci-e2e-no-metrics-tests.yml

@@ -0,0 +1,32 @@
+name: Continuous Integration (E2E Testing Checks without metrics database)
+
+on:
+  workflow_call:
+jobs:
+  e2e-no-metrics-test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: checkout repo from current commit
+        uses: actions/checkout@v3
+      - name: set up Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: "1.21"
+          check-latest: true
+          cache: false
+      - name: pull pre-built images
+        run: sudo docker compose -f ci.docker-compose.yml pull
+      - name: build and start proxy service and it's dependencies
+        # We need to provide additional env file to override the METRIC_DATABASE_ENABLED variable, not via env variable.
+        # Mentioned here: https://github.com/docker/compose/issues/9737
+        run: sudo docker compose -f ci.docker-compose.yml --env-file .env --env-file no_metric.env up -d --build proxy redis
+      - name: wait for proxy service to be running
+        run: bash ${GITHUB_WORKSPACE}/scripts/wait-for-proxy-service-running.sh
+        env:
+            PROXY_CONTAINER_PORT: 7777
+      - name: run e2e tests
+        run: SKIP_METRICS=true make e2e-test
+      - name: print proxy service logs
+        run: sudo docker compose -f ci.docker-compose.yml logs proxy
+        # because we especially want the logs if the test(s) fail 😅
+        if: always()

.github/workflows/ci-main.yml

@@ -11,10 +11,14 @@ jobs:
   # run default ci checks against main branch
   default-checks:
     uses: ./.github/workflows/ci-default.yml
-  # run e2e testing ci checks against main branch
+  # run e2e testing ci for internal testnet checks against main branch
   e2e-tests:
     needs: [lint-checks, default-checks]
     uses: ./.github/workflows/ci-e2e-tests.yml
+  # run e2e testing without metrics db ci for internal testnet checks against main branch
+  e2e-no-metrics-tests:
+    needs: [lint-checks, default-checks]
+    uses: ./.github/workflows/ci-e2e-no-metrics-tests.yml
   # build, tag and publish new service docker images
   release-docker-images:
       needs: [e2e-tests]

.github/workflows/ci-pr.yml

@@ -11,3 +11,5 @@ jobs:
     uses: ./.github/workflows/ci-default.yml
   e2e-tests:
     uses: ./.github/workflows/ci-e2e-tests.yml
+  e2e-no-metrics-tests:
+    uses: ./.github/workflows/ci-e2e-no-metrics-tests.yml

DEVELOPMENT.md

@@ -131,7 +131,7 @@ make it p=".*Eth_getBlockByNumberRequest"
 
 ## Migrations
 
-On startup the proxy service will run any SQL based migration in the [migrations folder](./clients/database/migrations) that haven't already been run against the database being used.
+On startup the proxy service will run any SQL based migration in the [migrations folder](clients/database/postgres/migrations) that haven't already been run against the database being used.
 
 For lower level details on how the migration process works consult [these docs](https://bun.uptrace.dev/guide/migrations.html).
 
@@ -144,7 +144,7 @@ $ date '+%Y%m%d%H%M%S'
 > 20230306182227
 ```
 
-Add new SQL file with commands to run in the new migration (add/delete/modify tables and or indices) in the in the [migrations folder](./clients/database/migrations)
+Add new SQL file with commands to run in the new migration (add/delete/modify tables and or indices) in the in the [migrations folder](clients/database/postgres/migrations)
 
 ### Running migrations
 

Makefile

@@ -44,6 +44,11 @@ unit-test:
 e2e-test:
 	go test -count=1 -v -cover -coverprofile cover.out --race ./... -run "^TestE2ETest*"
 
+.PHONY: e2e-test-no-metrics
+# run tests that execute against a local or remote instance of the API without database for metrics
+e2e-test-no-metrics:
+	SKIP_METRICS=true go test -count=1 -v -cover -coverprofile cover.out --race ./... -run "^TestE2ETest*"
+
 .PHONY: ci-setup
 # set up your local environment such that running `make e2e-test` runs against testnet (like in CI)
 ci-setup:

architecture/ARCHITECTURE.md

@@ -20,7 +20,7 @@ The proxy functionality provides the foundation for all other proxy service feat
 
 ![API Observability Worfklow Conceptual Overview](./images/observability_workflow_conceptual.jpg)
 
-For every request that is proxied by the proxy service, a [request metric](../decode/evm_rpc.go) is created and stored in a [postgres table](../clients/database/migrations/20230306182203_add_proxied_request_metrics_table.up.sql) that can be aggregated with other request metrics over a time range to answer ad hoc questions such as:
+For every request that is proxied by the proxy service, a [request metric](../decode/evm_rpc.go) is created and stored in a [postgres table](../clients/database/postgres/migrations/20230306182203_add_proxied_request_metrics_table.up.sql) that can be aggregated with other request metrics over a time range to answer ad hoc questions such as:
 
 - what methods take the longest time?
 - what methods are called the most frequently?

architecture/MIGRATIONS.MD

@@ -14,7 +14,7 @@ Setting an environment variable named `RUN_DATABASE_MIGRATIONS` to true will cau
 
 ### Migration Format
 
-New migration files must be placed in the [migrations directory](../clients/database/migrations/), have a unique name, and start with a timestamp in the below format:
+New migration files must be placed in the [migrations directory](../clients/database/postgres/migrations/), have a unique name, and start with a timestamp in the below format:
 
 ```bash
 $ date '+%Y%m%d%H%M%S'

ci.docker-compose.yml

@@ -2,7 +2,7 @@
 services:
   # run postgres for proxy service to store observability metrics
   postgres:
-    image: postgres:15
+    image: postgres:13.12
     env_file: .env
     ports:
       - "${POSTGRES_HOST_PORT}:${POSTGRES_CONTAINER_PORT}"
@@ -32,6 +32,9 @@ services:
       # fake the shards by defining shards with existing backends
       PROXY_SHARD_BACKEND_HOST_URL_MAP: localhost:7777>10|https://evmrpcdata.internal.testnet.proxy.kava.io|20|https://evmrpcdata.internal.testnet.proxy.kava.io
       EVM_QUERY_SERVICE_URL: https://evmrpc.internal.testnet.proxy.kava.io
+      # we need the metric to be used from no_metric.env or by default set up as true, so to test the metrics collection.
+      # doesn't work with the env variable, so need env file. Mentioned here: https://github.com/docker/compose/issues/9737
+      METRIC_DATABASE_ENABLED: "${METRIC_DATABASE_ENABLED}"
     ports:
       - "${PROXY_HOST_PORT}:${PROXY_CONTAINER_PORT}"
       - "${TEST_UNCONFIGURED_PROXY_PORT}:${PROXY_CONTAINER_PORT}"

clients/database/interface.go

@@ -0,0 +1,35 @@
+package database
+
+import (
+	"context"
+	"time"
+)
+
+// MetricsDatabase is an interface for interacting with the database
+type MetricsDatabase interface {
+	SaveProxiedRequestMetric(ctx context.Context, metric *ProxiedRequestMetric) error
+	ListProxiedRequestMetricsWithPagination(ctx context.Context, cursor int64, limit int) ([]*ProxiedRequestMetric, int64, error)
+	CountAttachedProxiedRequestMetricPartitions(ctx context.Context) (int64, error)
+	GetLastCreatedAttachedProxiedRequestMetricsPartitionName(ctx context.Context) (string, error)
+	DeleteProxiedRequestMetricsOlderThanNDays(ctx context.Context, n int64) error
+
+	HealthCheck() error
+	Partition(prefillPeriodDays int) error
+}
+
+type ProxiedRequestMetric struct {
+	ID                          int64
+	MethodName                  string
+	BlockNumber                 *int64
+	ResponseLatencyMilliseconds int64
+	Hostname                    string
+	RequestIP                   string
+	RequestTime                 time.Time
+	UserAgent                   *string
+	Referer                     *string
+	Origin                      *string
+	ResponseBackend             string
+	ResponseBackendRoute        string
+	CacheHit                    bool
+	PartOfBatch                 bool
+}

clients/database/noop/database.go

@@ -0,0 +1,41 @@
+package noop
+
+import (
+	"context"
+	"github.com/kava-labs/kava-proxy-service/clients/database"
+)
+
+// Noop is a database client that does nothing
+type Noop struct{}
+
+func New() *Noop {
+	return &Noop{}
+}
+
+func (e *Noop) SaveProxiedRequestMetric(ctx context.Context, metric *database.ProxiedRequestMetric) error {
+	return nil
+}
+
+func (e *Noop) ListProxiedRequestMetricsWithPagination(ctx context.Context, cursor int64, limit int) ([]*database.ProxiedRequestMetric, int64, error) {
+	return []*database.ProxiedRequestMetric{}, 0, nil
+}
+
+func (e *Noop) CountAttachedProxiedRequestMetricPartitions(ctx context.Context) (int64, error) {
+	return 0, nil
+}
+
+func (e *Noop) GetLastCreatedAttachedProxiedRequestMetricsPartitionName(ctx context.Context) (string, error) {
+	return "", nil
+}
+
+func (e *Noop) DeleteProxiedRequestMetricsOlderThanNDays(ctx context.Context, n int64) error {
+	return nil
+}
+
+func (e *Noop) HealthCheck() error {
+	return nil
+}
+
+func (e *Noop) Partition(prefillPeriodDays int) error {
+	return nil
+}

clients/database/postgres.go → clients/database/postgres/database.go

@@ -1,4 +1,4 @@
-package database
+package postgres
 
 import (
 	"crypto/tls"
@@ -13,15 +13,15 @@ import (
 	"github.com/uptrace/bun/extra/bundebug"
 )
 
-// PostgresDatabaseConfig contains values for creating a
+// DatabaseConfig contains values for creating a
 // new connection to a postgres database
-type PostgresDatabaseConfig struct {
+type DatabaseConfig struct {
 	DatabaseName                     string
 	DatabaseEndpointURL              string
 	DatabaseUsername                 string
 	DatabasePassword                 string
 	ReadTimeoutSeconds               int64
-	WriteTimeousSeconds              int64
+	WriteTimeoutSeconds              int64
 	DatabaseMaxIdleConnections       int64
 	DatabaseConnectionMaxIdleSeconds int64
 	DatabaseMaxOpenConnections       int64
@@ -31,14 +31,15 @@ type PostgresDatabaseConfig struct {
 	Logger                           *logging.ServiceLogger
 }
 
-// PostgresClient wraps a connection to a postgres database
-type PostgresClient struct {
-	*bun.DB
+// Client wraps a connection to a postgres database
+type Client struct {
+	db     *bun.DB
+	logger *logging.ServiceLogger
 }
 
-// NewPostgresClient returns a new connection to the specified
+// NewClient returns a new connection to the specified
 // postgres data and error (if any)
-func NewPostgresClient(config PostgresDatabaseConfig) (PostgresClient, error) {
+func NewClient(config DatabaseConfig) (Client, error) {
 	// configure postgres database connection options
 	var pgOptions *pgdriver.Connector
 
@@ -54,7 +55,7 @@ func NewPostgresClient(config PostgresDatabaseConfig) (PostgresClient, error) {
 				pgdriver.WithPassword(config.DatabasePassword),
 				pgdriver.WithDatabase(config.DatabaseName),
 				pgdriver.WithReadTimeout(time.Second*time.Duration(config.ReadTimeoutSeconds)),
-				pgdriver.WithWriteTimeout(time.Second*time.Duration(config.WriteTimeousSeconds)),
+				pgdriver.WithWriteTimeout(time.Second*time.Duration(config.WriteTimeoutSeconds)),
 			)
 	} else {
 		pgOptions = pgdriver.NewConnector(
@@ -64,7 +65,7 @@ func NewPostgresClient(config PostgresDatabaseConfig) (PostgresClient, error) {
 			pgdriver.WithPassword(config.DatabasePassword),
 			pgdriver.WithDatabase(config.DatabaseName),
 			pgdriver.WithReadTimeout(time.Second*time.Duration(config.ReadTimeoutSeconds)),
-			pgdriver.WithWriteTimeout(time.Second*time.Duration(config.WriteTimeousSeconds)),
+			pgdriver.WithWriteTimeout(time.Second*time.Duration(config.WriteTimeoutSeconds)),
 		)
 	}
 
@@ -86,13 +87,14 @@ func NewPostgresClient(config PostgresDatabaseConfig) (PostgresClient, error) {
 		db.AddQueryHook(bundebug.NewQueryHook(bundebug.WithVerbose(true)))
 	}
 
-	return PostgresClient{
-		DB: db,
+	return Client{
+		db:     db,
+		logger: config.Logger,
 	}, nil
 }
 
 // HealthCheck returns an error if the database can not
 // be connected to and queried, nil otherwise
-func (pg *PostgresClient) HealthCheck() error {
-	return pg.Ping()
+func (c *Client) HealthCheck() error {
+	return c.db.Ping()
 }

clients/database/postgres/database_test.go

@@ -0,0 +1,18 @@
+package postgres
+
+import (
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestDisabledDBCreation(t *testing.T) {
+	config := DatabaseConfig{}
+	_, err := NewClient(config)
+	require.Error(t, err)
+}
+
+func TestHealthcheckNoDatabase(t *testing.T) {
+	config := DatabaseConfig{}
+	_, err := NewClient(config)
+	require.Error(t, err)
+}

clients/database/database.go → clients/database/postgres/migrate.go

@@ -1,22 +1,21 @@
-package database
+package postgres
 
 import (
 	"context"
 	"fmt"
 	"time"
 
 	"github.com/kava-labs/kava-proxy-service/logging"
-	"github.com/uptrace/bun"
 	"github.com/uptrace/bun/migrate"
 )
 
 // Migrate sets up and runs all migrations in the migrations model
 // that haven't been run on the database being used by the proxy service
 // returning error (if any) and a list of migrations that have been
 // run and any that were not
-func Migrate(ctx context.Context, db *bun.DB, migrations migrate.Migrations, logger *logging.ServiceLogger) (*migrate.MigrationSlice, error) {
+func (c *Client) Migrate(ctx context.Context, migrations migrate.Migrations, logger *logging.ServiceLogger) (*migrate.MigrationSlice, error) {
 	// set up migration config
-	migrator := migrate.NewMigrator(db, &migrations)
+	migrator := migrate.NewMigrator(c.db, &migrations)
 
 	// create / verify tables used to tack migrations
 	err := migrator.Init(ctx)

clients/database/postgres/migrate_test.go

@@ -0,0 +1,15 @@
+package postgres
+
+import (
+	"context"
+	"github.com/stretchr/testify/require"
+	"github.com/uptrace/bun/migrate"
+	"testing"
+)
+
+func TestMigrateNoDatabase(t *testing.T) {
+	db := &Client{}
+
+	_, err := db.Migrate(context.Background(), migrate.Migrations{}, nil)
+	require.Error(t, err)
+}