Replies: 7 comments 1 reply
-
|
To add more context, if I were writing an adapter for the Go Prometheus Client I would need to write this caching myself inside the adapter to avoid duplicate registration of metrics. I don't know how heavy-weight the creation of instruments are in other client-SDKs for metrics, so that could also be a consideration. |
Beta Was this translation helpful? Give feedback.
-
|
I'm not familiar with Prometheus - does that implementation not cache instruments/meters by default? The OTEL SDK for Go explicitly states that it does, which we released an adapter for out-of-the-box. So, since we need to generally support functional options that modify config on a per-request basis (e.g. setting a different one for a specific operation) we took advantage of this and made the operation workflow invoke Since our clients support functional options, I'm leaning towards taking a hard stance that Meter() and Instrument() calls through the smithy-go APIs should be idempotent on spec. |
Beta Was this translation helpful? Give feedback.
-
|
Using the Prometheus Go SDK, the flow is:
It's an error to register two metrics with the same unique-identifier (or rather, it's an error to register two metrics which would result in the same time-series, if that makes sense), and there is no out-of-the-box mechanism to look up previously registered metrics to avoid creating new ones - it is up to the application to keep a reference to the metric. If we knew that for a given scope + name we would never get different configs (which sounds like what you're suggesting), a hypothetical Prometheus meter-provider-adapter could keep these metrics persistent in-between calls. |
Beta Was this translation helpful? Give feedback.
-
|
It also looks like the OTEL SDK has an adapter to register itself with a Prometheus registry - that might be the smoothest option if you're primarily targeting and testing against the OTEL SDKs. |
Beta Was this translation helpful? Give feedback.
-
|
I'm okay closing this if your primary target is OTEL metrics - the OTEL project has a Prometheus adapter which seems to work okay. For reference, a sketch of what that looks like is below. That assumes the caller is okay hauling around the OTEL SDK for these purposes. A prometheus adapter plugged directly into the AWS SDK which caches instruments wouldn't be too hard, and might avoid some allocations per SDK-call. I'm not sure about what I'm doing with histogram-buckets - that might not actually be necessary. Click here!package main
import (
"context"
"fmt"
"io"
"net/http/httptest"
"os"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/smithy-go/metrics/smithyotelmetrics"
"go.opentelemetry.io/otel"
otelprom "go.opentelemetry.io/otel/exporters/prometheus"
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
)
func main() {
if err := mainErr(); err != nil {
fmt.Fprintf(os.Stderr, "error: %s\n", err)
os.Exit(1)
}
}
func mainErr() error {
// set up our metric-exporter
setupOTELExporter()
// for demo purposes, scrape all prom metrics and dump to stdout
defer scrapePromMetrics()
ctx := context.Background()
cfg, err := config.LoadDefaultConfig(ctx)
if err != nil {
return fmt.Errorf("loading aws config: %s", err)
}
s3c := s3.NewFromConfig(cfg, func(o *s3.Options) {
o.MeterProvider = smithyotelmetrics.Adapt(otel.GetMeterProvider())
})
// make a few API calls
_, err = s3c.ListBuckets(ctx, &s3.ListBucketsInput{})
if err != nil {
return fmt.Errorf("list buckets: %s", err)
}
_, err = s3c.ListBuckets(ctx, &s3.ListBucketsInput{})
if err != nil {
return fmt.Errorf("list buckets: %s", err)
}
_, err = s3c.ListBuckets(ctx, &s3.ListBucketsInput{})
if err != nil {
return fmt.Errorf("list buckets: %s", err)
}
return nil
}
func setupOTELExporter() {
// create an otel metric-exporter associated with the
// default prometheus registry
metricExporter, err := otelprom.New(
otelprom.WithNamespace("aws"),
otelprom.WithoutScopeInfo(),
// OTEL default buckets assume you're using milliseconds. Substitute defaults
// appropriate for units of seconds.
//
// https://github.com/open-telemetry/opentelemetry-go/issues/5821
otelprom.WithAggregationSelector(func(ik sdkmetric.InstrumentKind) sdkmetric.Aggregation {
switch ik {
case sdkmetric.InstrumentKindHistogram:
return sdkmetric.AggregationExplicitBucketHistogram{
Boundaries: prometheus.DefBuckets,
NoMinMax: false,
}
default:
return sdkmetric.DefaultAggregationSelector(ik)
}
}),
)
if err != nil {
panic(err)
}
// create a meter-provider associated with the exporter
meterProvider := sdkmetric.NewMeterProvider(
sdkmetric.WithReader(metricExporter),
)
otel.SetMeterProvider(meterProvider)
}
// for demo purposes, dump all prom metrics to stdout
func scrapePromMetrics() {
req := httptest.NewRequest("GET", "/", nil)
rw := httptest.NewRecorder()
promhttp.Handler().ServeHTTP(rw, req)
io.Copy(os.Stdout, rw.Result().Body)
} |
Beta Was this translation helpful? Give feedback.
-
|
Another difference between OTEL and Prometheus is that Prometheus requires that the time-series labels be defined during instrument-creation, but it looks like the APIs provided by smithy-metrics defer that to observation, which makes the direct Prometheus adapter more awkward. Anyway, I have a basic path forward. If I'm okay with the OTEL SDK as a dependency the code is pretty straightforward. If I want a native Prometheus-client integration, implementing my own middleware is likely going to be less work that fitting the smithy-metrics interfaces. |
Beta Was this translation helpful? Give feedback.
-
|
Is there a reason you would choose to adapt directly to Prometheus instead of instrumenting w/ OTEL and exporting to Prometheus from there? |
Beta Was this translation helpful? Give feedback.
-
|
Mostly the above is me stumbling through trying to understand the integration being offered. If the OTEL Go SDK is the way you're imagining AWS-SDK metrics are exposed then that's the way to go. It wasn't obvious from my initial reading of the announcement that that was the exclusive approach. My team simply has never even looked at the OTEL libraries for metrics because the Prometheus client-SDKs are mature and well supported by the rest of our observability stack. I'm not against the sketch I have above of using the OTEL prometheus exporter - we've just never had the occasion of needing this sort of integration so we haven't evaluated it. |
Beta Was this translation helpful? Give feedback.
-
Acknowledgements
go get -u github.com/aws/aws-sdk-go-v2/...)Describe the bug
When supplying a MeterProvider to the s3 client, the underlying retryer seems to be creating a metrics-instrument on every API call.
Following the OTEL model (and other similar metrics-libraries) I would have expected instruments to be creating once, cached, and then getting new observations on existing instruments on each API call.
For example, from the OTEL documentation on instrumenting a Go application: https://opentelemetry.io/docs/languages/go/instrumentation/#using-counters
Click here!
Excerpt of my test code:
Full code sample:
Click here!
Regression Issue
Expected Behavior
I expect my demo-meter-provider to log each instrument creation only once.
Current Behavior
The instruments were creating three times - once per API call I made to the S3 service:
Click here!
Reproduction Steps
Possible Solution
No response
Additional Information/Context
No response
AWS Go SDK V2 Module Versions Used
Compiler and Version used
go version go1.23.1 linux/amd64
Operating System and version
Arch Linux x86_64, Linux 6.10.10-arch1-1
Beta Was this translation helpful? Give feedback.
All reactions