-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(storage): add grpc metrics experimental options #10984
Changes from 12 commits
1e00c56
56a21af
45ada59
797cb7d
dde7cbd
f901a2e
aee0bf1
475f37a
28d675f
22d23ee
a898518
2210b54
2bf7e35
243b0a6
154fa54
fa56a51
31dc86b
596199d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,9 +25,27 @@ import ( | |
"time" | ||
|
||
"cloud.google.com/go/storage/internal" | ||
"go.opentelemetry.io/otel/sdk/metric" | ||
"google.golang.org/api/option" | ||
) | ||
|
||
// WithMetricInterval provides a [ClientOption] that may be passed to [storage.NewGrpcClient]. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: what does ClientOption link to? |
||
// It sets how often to emit metrics when using NewPeriodicReader and must be | ||
// greater than 1 minute. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this the minimum? Not sure why this requires validation, users might have their own systems with higher rate limits. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point; could pass along a value if it's non-zero. |
||
// https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric#NewPeriodicReader | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leave an empty line (with a |
||
// https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric#WithInterval | ||
func WithMetricInterval(metricInterval time.Duration) option.ClientOption { | ||
return internal.WithMetricInterval.(func(time.Duration) option.ClientOption)(metricInterval) | ||
} | ||
|
||
// WithMetricExporter provides a [ClientOption] that may be passed to [storage.NewGrpcClient]. | ||
// Set an alternate client-side metric Exporter to emit metrics through. | ||
// Must implement interface metric.Exporter: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just do [metric.Exporter] and the godoc should autolink. |
||
// https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric#Exporter | ||
func WithMetricExporter(ex *metric.Exporter) option.ClientOption { | ||
return internal.WithMetricExporter.(func(*metric.Exporter) option.ClientOption)(ex) | ||
} | ||
|
||
// WithReadStallTimeout provides a [ClientOption] that may be passed to [storage.NewClient]. | ||
// It enables the client to retry stalled requests when starting a download from | ||
// Cloud Storage. If the timeout elapses with no response from the server, the request | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,12 +15,14 @@ | |
package storage | ||
|
||
import ( | ||
"time" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why this got moved but it will probably break formatting presubmits |
||
|
||
"os" | ||
"strconv" | ||
"time" | ||
|
||
"cloud.google.com/go/storage/experimental" | ||
storageinternal "cloud.google.com/go/storage/internal" | ||
"go.opentelemetry.io/otel/sdk/metric" | ||
"google.golang.org/api/option" | ||
"google.golang.org/api/option/internaloption" | ||
) | ||
|
@@ -35,7 +37,9 @@ const ( | |
) | ||
|
||
func init() { | ||
// initialize experimental option. | ||
// initialize experimental options | ||
storageinternal.WithMetricExporter = withMetricExporter | ||
storageinternal.WithMetricInterval = withMetricInterval | ||
storageinternal.WithReadStallTimeout = withReadStallTimeout | ||
} | ||
|
||
|
@@ -69,12 +73,13 @@ func getDynamicReadReqInitialTimeoutSecFromEnv(defaultVal time.Duration) time.Du | |
return val | ||
} | ||
|
||
// storageConfig contains the Storage client option configuration that can be | ||
// set through storageClientOptions. | ||
type storageConfig struct { | ||
useJSONforReads bool | ||
readAPIWasSet bool | ||
disableClientMetrics bool | ||
metricExporter *metric.Exporter | ||
metricInterval time.Duration | ||
readStallTimeoutConfig *experimental.ReadStallTimeoutConfig | ||
} | ||
|
||
|
@@ -160,6 +165,34 @@ func (w *withDisabledClientMetrics) ApplyStorageOpt(c *storageConfig) { | |
c.disableClientMetrics = w.disabledClientMetrics | ||
} | ||
|
||
type withMeterOptions struct { | ||
internaloption.EmbeddableAdapter | ||
// set sampling interval | ||
interval time.Duration | ||
} | ||
|
||
func withMetricInterval(interval time.Duration) option.ClientOption { | ||
return &withMeterOptions{interval: interval} | ||
} | ||
|
||
func (w *withMeterOptions) ApplyStorageOpt(c *storageConfig) { | ||
c.metricInterval = w.interval | ||
} | ||
|
||
type withMetricExporterConfig struct { | ||
internaloption.EmbeddableAdapter | ||
// exporter override | ||
metricExporter *metric.Exporter | ||
} | ||
|
||
func withMetricExporter(ex *metric.Exporter) option.ClientOption { | ||
return &withMetricExporterConfig{metricExporter: ex} | ||
} | ||
|
||
func (w *withMetricExporterConfig) ApplyStorageOpt(c *storageConfig) { | ||
c.metricExporter = w.metricExporter | ||
} | ||
|
||
// WithReadStallTimeout is an option that may be passed to [NewClient]. | ||
// It enables the client to retry the stalled read request, happens as part of | ||
// storage.Reader creation. As the name suggest, timeout is adjusted dynamically | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: should be storage.NewGRPCClient (here and below)