Skip to content
Open
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
35 changes: 35 additions & 0 deletions database/plugin/blob/aws/commit_timestamp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2025 Blink Labs Software
//
// 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 aws

import (
"context"
"math/big"
)

const commitTimestampBlobKey = "metadata_commit_timestamp"

func (b *BlobStoreS3) GetCommitTimestamp(ctx context.Context) (int64, error) {
data, err := b.Get(ctx, commitTimestampBlobKey)
if err != nil {
return 0, err
}
return new(big.Int).SetBytes(data).Int64(), nil
}

func (b *BlobStoreS3) SetCommitTimestamp(ctx context.Context, ts int64) error {
raw := new(big.Int).SetInt64(ts).Bytes()
return b.Put(ctx, commitTimestampBlobKey, raw)
}
184 changes: 184 additions & 0 deletions database/plugin/blob/aws/database.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
// Copyright 2025 Blink Labs Software
//
// 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 aws

import (
"bytes"
"context"
"errors"
"fmt"
"io"
"log/slog"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/blinklabs-io/dingo/database/plugin"
"github.com/prometheus/client_golang/prometheus"
)

// Register plugin
func init() {
plugin.Register(
plugin.PluginEntry{
Type: plugin.PluginTypeBlob,
Name: "s3",
},
)
}
Comment on lines +33 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that the plugin registration is complete.

The plugin registration only sets Type and Name, but PluginEntry includes additional fields (NewFromOptionsFunc, Description, Options) that are not populated. Confirm whether NewFromOptionsFunc is required for the plugin system to instantiate the S3 blob store, or if the current registration is sufficient.

Run the following script to check how other blob plugins register themselves:


🏁 Script executed:

#!/bin/bash
# Description: Check plugin registration patterns in other blob implementations

# Search for plugin.Register calls in other blob plugins
rg -nP --type=go 'plugin\.Register\(' -A 8 -g 'database/plugin/blob/**'

Length of output: 1592



---

🏁 Script executed:

```shell
#!/bin/bash
# Description: Check PluginEntry type definition and plugin instantiation pattern

# Find PluginEntry type definition
rg -nP --type=go 'type\s+PluginEntry\s+struct' -A 15

# Also search for how plugins are actually instantiated/used
rg -nP --type=go 'NewFromOptionsFunc' -B 2 -A 2

Length of output: 1443



---

🏁 Script executed:

```shell
#!/bin/bash
# Description: Check if blob plugins are instantiated through the plugin system

# Search for GetPlugin calls related to blob
rg -nP --type=go 'GetPlugin|NewFromOptionsFunc' -B 3 -A 3 -g '!**/test/**'

# Search for BlobStore instantiation patterns
rg -nP --type=go 'BlobStore|blob.*New' -B 2 -A 2 | head -50

# Check if there are any New() functions in blob implementations
fd --type f -name 'database.go' -path '*/blob/*' -exec grep -l 'func New' {} \;

Length of output: 3763



---

🏁 Script executed:

```shell
#!/bin/bash
# Description: Find blob.New() implementation and understand instantiation flow

# Search for blob.New() function
rg -nP --type=go 'func New\(' -A 20 'database/plugin/blob/store.go'

# Check for all files in blob plugin directory
fd --type f --path 'database/plugin/blob/*' -name '*.go'

# Search for plugin system usage with blob
rg -nP --type=go 'GetPlugin.*PluginTypeBlob|plugin\.PluginTypeBlob' -B 3 -A 3

Length of output: 1676



---

🏁 Script executed:

```shell
#!/bin/bash
# Description: Verify if pluginName is truly unused in blob.New()

# Search complete blob.New() and surrounding context
rg -nP --type=go 'func New\(' -A 30 'database/plugin/blob/store.go'

# Search for any conditional logic or factory pattern
rg -nP --type=go 'pluginName|GetPlugin' 'database/plugin/blob/store.go'

Length of output: 275


Plugin registration is incomplete and will cause runtime failures.

The PluginEntry requires a NewFromOptionsFunc field that creates and returns a Plugin instance. Without it, calling GetPlugin(plugin.PluginTypeBlob, "s3") will invoke a nil function pointer and panic. All three blob plugin implementations (S3, Badger, GCS) have this same issue. Each blob plugin should define a factory function and register it.

Example fix needed:

func init() {
	plugin.Register(
		plugin.PluginEntry{
			Type: plugin.PluginTypeBlob,
			Name: "s3",
			NewFromOptionsFunc: func() plugin.Plugin {
				// Return properly initialized plugin instance
			},
		},
	)
}

Note: The pluginName parameter in database/plugin/blob/store.go's New() function is also unused—the implementation hardcodes badgerPlugin.New() regardless of the plugin name passed in.

🤖 Prompt for AI Agents
In database/plugin/blob/aws/database.go around lines 33-41, the plugin
registration is missing the required NewFromOptionsFunc which will cause a nil
function panic at runtime; add a NewFromOptionsFunc that returns a properly
initialized S3 blob plugin instance (accepting options as needed), and apply the
same pattern to the Badger and GCS plugin registration files so each PluginEntry
provides a factory. Also update database/plugin/blob/store.go to stop ignoring
the pluginName parameter and instead choose/instantiate the correct plugin
implementation based on pluginName (call the registered factory/new-from-options
for the requested plugin) so New() returns the requested plugin rather than
hardcoding the Badger implementation.


// BlobStoreS3 stores data in an AWS S3 bucket
type BlobStoreS3 struct {
promRegistry prometheus.Registerer
startupCtx context.Context
logger *S3Logger
client *s3.Client
bucket string
prefix string
startupCancel context.CancelFunc
}

// New creates a new S3-backed blob store and dataDir must be "s3://bucket" or "s3://bucket/prefix"
func New(
dataDir string,
logger *slog.Logger,
promRegistry prometheus.Registerer,
) (*BlobStoreS3, error) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)

if logger == nil {
logger = slog.New(slog.NewJSONHandler(io.Discard, nil))
}

const prefix = "s3://"
if !strings.HasPrefix(strings.ToLower(dataDir), prefix) {
cancel()
return nil, errors.New("s3 blob: expected dataDir='s3://<bucket>[/prefix]'")
}

path := strings.TrimPrefix(strings.ToLower(dataDir), prefix)
if path == "" {
cancel()
return nil, errors.New("s3 blob: bucket not set")
}

parts := strings.SplitN(path, "/", 2)
if len(parts) == 0 || parts[0] == "" {
cancel()
return nil, errors.New("s3 blob: invalid S3 path (missing bucket)")
}

bucket := parts[0]
keyPrefix := ""
if len(parts) > 1 && parts[1] != "" {
keyPrefix = strings.TrimSuffix(parts[1], "/")
if keyPrefix != "" {
keyPrefix += "/"
}
}

// Loads all the aws credentials.
awsCfg, err := config.LoadDefaultConfig(ctx)
if err != nil {
cancel()
return nil, fmt.Errorf("s3 blob: load default AWS config: %w", err)
}
client := s3.NewFromConfig(awsCfg)

db := &BlobStoreS3{
logger: NewS3Logger(logger),
promRegistry: promRegistry,
client: client,
bucket: bucket,
prefix: keyPrefix,
startupCtx: ctx,
startupCancel: cancel,
}
if err := db.init(); err != nil {
cancel()
return nil, err
}
return db, nil
}

func (d *BlobStoreS3) init() error {
// Configure metrics
if d.promRegistry != nil {
d.registerBlobMetrics()
}

// Close the startup context so that initialization will succeed.
if d.startupCancel != nil {
d.startupCancel()
d.startupCancel = nil
}
return nil
}

// Returns the S3 client.
func (d *BlobStoreS3) Client() *s3.Client {
return d.client
}

// Returns the bucket handle.
func (d *BlobStoreS3) Bucket() string {
return d.bucket
}

// Returns the S3 key with an optional prefix.
func (d *BlobStoreS3) fullKey(key string) string {
return d.prefix + key
}

func awsString(s string) *string {
return &s
}

// Get reads the value at key.
func (d *BlobStoreS3) Get(ctx context.Context, key string) ([]byte, error) {
out, err := d.client.GetObject(ctx, &s3.GetObjectInput{
Bucket: &d.bucket,
Key: awsString(d.fullKey(key)),
})
if err != nil {
d.logger.Errorf("s3 get %q failed: %v", key, err)
return nil, err
}
defer out.Body.Close()

data, err := io.ReadAll(out.Body)
if err != nil {
d.logger.Errorf("s3 read %q failed: %v", key, err)
return nil, err
}
d.logger.Infof("s3 get %q ok (%d bytes)", key, len(data))
return data, nil
}
Comment on lines +150 to +169
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing metrics recording in Get operation.

The BlobStoreS3 struct includes a Prometheus registry and the PR summary mentions "Added metrics and logging for all operations," but the Get method does not record any metrics. According to the AI summary, metrics.go defines counters for blob operations (ops_total, bytes_total), but they are not incremented here.

Ensure that metrics are recorded for successful and failed Get operations, including bytes read. For example:

// After successful read (before line 168):
if d.promRegistry != nil {
    // Increment ops_total counter with labels
    // Increment bytes_total counter with len(data)
}

Note: The exact implementation depends on how metrics are defined in metrics.go.

🤖 Prompt for AI Agents
In database/plugin/blob/aws/database.go around lines 150-169, the Get method
logs but does not update Prometheus metrics; update it to increment the blob
operation counters on both success and error (use the same label set you use
elsewhere: operation="get", bucket/name labels as appropriate) and on success
also increment the bytes_total counter by len(data); guard updates with a nil
check on d.promRegistry (or whatever metric handles are stored) so tests/older
builds without metrics won't panic, and place the success increments after the
ReadAll succeeds but before returning, and the error increment(s) in each error
branch before returning.


// Put writes a value to key.
func (d *BlobStoreS3) Put(ctx context.Context, key string, value []byte) error {
_, err := d.client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &d.bucket,
Key: awsString(d.fullKey(key)),
Body: bytes.NewReader(value),
})
if err != nil {
d.logger.Errorf("s3 put %q failed: %v", key, err)
return err
}
d.logger.Infof("s3 put %q ok (%d bytes)", key, len(value))
return nil
}
Comment on lines +171 to +184
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing metrics recording in Put operation.

Similar to the Get method, the Put method does not record any metrics despite the metrics infrastructure being set up. The PR summary explicitly mentions "Added metrics and logging for all operations," but metrics are not incremented here.

Ensure that metrics are recorded for successful and failed Put operations, including bytes written. For example:

// After successful put (before line 183):
if d.promRegistry != nil {
    // Increment ops_total counter with labels
    // Increment bytes_total counter with len(value)
}

Note: The exact implementation depends on how metrics are defined in metrics.go.

🤖 Prompt for AI Agents
In database/plugin/blob/aws/database.go around lines 171 to 184, the Put method
currently logs success/failure but does not update Prometheus metrics; add the
same metrics updates used in Get: check d.promRegistry (or equivalent) is
non-nil, on error increment the ops_total counter with labels (operation="put",
result="error" or similar) and on success increment ops_total with result="ok"
and increment the bytes_total (or equivalent) counter by len(value); match the
label names and metric variables defined in metrics.go so the Put operation
records both operation count and bytes written.

61 changes: 61 additions & 0 deletions database/plugin/blob/aws/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2025 Blink Labs Software
//
// 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 aws

import (
"fmt"
"io"
"log/slog"
)

// S3Logger is a thin wrapper giving our logger a consistent interface.
type S3Logger struct {
logger *slog.Logger
}

func NewS3Logger(logger *slog.Logger) *S3Logger {
if logger == nil {
logger = slog.New(slog.NewJSONHandler(io.Discard, nil))
}
return &S3Logger{logger: logger}
}

func (g *S3Logger) Infof(msg string, args ...any) {
g.logger.Info(
fmt.Sprintf(msg, args...),
"component", "database",
)
}

func (g *S3Logger) Warningf(msg string, args ...any) {
g.logger.Warn(
fmt.Sprintf(msg, args...),
"component", "database",
)
}

func (g *S3Logger) Debugf(msg string, args ...any) {
g.logger.Debug(
fmt.Sprintf(msg, args...),
"component", "database",
)
}

func (g *S3Logger) Errorf(msg string, args ...any) {
g.logger.Error(
fmt.Sprintf(msg, args...),
"component", "database",
)
}
36 changes: 36 additions & 0 deletions database/plugin/blob/aws/metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2025 Blink Labs Software
//
// 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 aws

import "github.com/prometheus/client_golang/prometheus"

const s3MetricNamePrefix = "database_blob_"

func (d *BlobStoreS3) registerBlobMetrics() {
opsTotal := prometheus.NewCounter(
prometheus.CounterOpts{
Name: s3MetricNamePrefix + "ops_total",
Help: "Total number of S3 blob operations",
},
)
bytesTotal := prometheus.NewCounter(
prometheus.CounterOpts{
Name: s3MetricNamePrefix + "bytes_total",
Help: "Total bytes read/written for S3 blob operations",
},
)

d.promRegistry.MustRegister(opsTotal, bytesTotal)
}
Comment on lines +21 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Metrics are created but never used or incremented.

The counters opsTotal and bytesTotal are created as local variables but are never stored as fields on BlobStoreS3, making it impossible for the Get and Put methods to increment them. These metrics will always report zero.

Store the counters as fields and increment them in the relevant operations:

+type BlobStoreS3 struct {
+	promRegistry  prometheus.Registerer
+	startupCtx    context.Context
+	logger        *S3Logger
+	client        *s3.Client
+	bucket        string
+	prefix        string
+	startupCancel context.CancelFunc
+	opsTotal      prometheus.Counter
+	bytesTotal    prometheus.Counter
+}

 func (d *BlobStoreS3) registerBlobMetrics() {
-	opsTotal := prometheus.NewCounter(
+	d.opsTotal = prometheus.NewCounter(
 		prometheus.CounterOpts{
 			Name: s3MetricNamePrefix + "ops_total",
 			Help: "Total number of S3 blob operations",
 		},
 	)
-	bytesTotal := prometheus.NewCounter(
+	d.bytesTotal = prometheus.NewCounter(
 		prometheus.CounterOpts{
 			Name: s3MetricNamePrefix + "bytes_total",
 			Help: "Total bytes read/written for S3 blob operations",
 		},
 	)
 
-	d.promRegistry.MustRegister(opsTotal, bytesTotal)
+	d.promRegistry.MustRegister(d.opsTotal, d.bytesTotal)
 }

Then increment in Get and Put operations (in database.go):

// In Get method after successful read:
if d.opsTotal != nil {
	d.opsTotal.Inc()
	d.bytesTotal.Add(float64(len(data)))
}

// In Put method after successful write:
if d.opsTotal != nil {
	d.opsTotal.Inc()
	d.bytesTotal.Add(float64(len(value)))
}
🤖 Prompt for AI Agents
In database/plugin/blob/aws/metrics.go around lines 21 to 36, the prometheus
counters opsTotal and bytesTotal are created as local variables and never stored
on the BlobStoreS3 struct, so they cannot be incremented by Get/Put and will
always report zero; modify the BlobStoreS3 type to add fields for opsTotal and
bytesTotal, assign the created counters to those struct fields inside
registerBlobMetrics (instead of local variables), register them with
promRegistry, and then update the Get and Put implementations (in database.go)
to increment d.opsTotal.Inc() and d.bytesTotal.Add(float64(len(data))) after
successful reads and d.opsTotal.Inc() and d.bytesTotal.Add(float64(len(value)))
after successful writes (guarding with nil checks if necessary).

Loading