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.

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

Add Prometheus metrics for S3 and IAM requests #71

26 changes: 26 additions & 0 deletions pkg/clients/s3/s3_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ import (
"net/http"
"os"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/credentials"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/aws/smithy-go/logging"
c "github.com/scality/cosi-driver/pkg/constants"
"github.com/scality/cosi-driver/pkg/metrics"
"github.com/scality/cosi-driver/pkg/util"
)

Expand Down Expand Up @@ -65,6 +68,8 @@ var InitS3Client = func(params util.StorageClientParameters) (*S3Client, error)
}

func (client *S3Client) CreateBucket(ctx context.Context, bucketName string, params util.StorageClientParameters) error {
method := "CreateBucket"
start := time.Now()

input := &s3.CreateBucketInput{
Bucket: &bucketName,
Expand All @@ -77,12 +82,33 @@ func (client *S3Client) CreateBucket(ctx context.Context, bucketName string, par
}

_, err := client.S3Service.CreateBucket(ctx, input)

duration := time.Since(start).Seconds()

Choose a reason for hiding this comment

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

In case it helps, the prometheus module provides a helper to observe durations that can make the code a tiny bit simpler: https://pkg.go.dev/github.com/prometheus/client_golang/prometheus#Timer

status := c.StatusSuccess
if err != nil {
status = c.StatusError
}

metrics.S3RequestsTotal.WithLabelValues(method, status).Inc()

Choose a reason for hiding this comment

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

Since this metrics pattern seems to be repeated a lot, it may be worth having a wrapper around all request handlers that want metrics. I think it could be done using function closures, so the function signature is identical to the wrapper in all cases. But your call, just though it could help readability and maintenance and limit the risk of mistakes.

Choose a reason for hiding this comment

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

Could it be done with some kind of middleware around the aws sdk client: https://aws.github.io/aws-sdk-go-v2/docs/middleware/ ?

metrics.S3RequestDuration.WithLabelValues(method, status).Observe(duration)
return err
}

func (client *S3Client) DeleteBucket(ctx context.Context, bucketName string) error {
method := "DeleteBucket"
start := time.Now()

_, err := client.S3Service.DeleteBucket(ctx, &s3.DeleteBucketInput{
Bucket: &bucketName,
})
duration := time.Since(start).Seconds()

status := c.StatusSuccess
if err != nil {
status = c.StatusError
}

metrics.S3RequestsTotal.WithLabelValues(method, status).Inc()
metrics.S3RequestDuration.WithLabelValues(method, status).Observe(duration)
return err
}
10 changes: 8 additions & 2 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ const (

// Service initialization constants
const (
MetricsPath = "/metrics"
MetricsAddress = ":8080"
MetricsPath = "/metrics" // Path to expose Prometheus metrics
MetricsAddress = ":8080" // Default address to bind the metrics server
)

// Prometheus metrics status values
const (
StatusSuccess = "success" // Status value for successful requests

Choose a reason for hiding this comment

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

Since those are global constants, it can be better to prefix them like MetricsStatusSuccess

Choose a reason for hiding this comment

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

What about have a specific small pkg metricsStatus to remove any prefix and so everything is not mixed in a global constants file ?

StatusError = "error" // Status value for failed requests
)
Loading