Skip to content

Commit

Permalink
Count the Collect duration towards the PeriodicReader timeout, and do…
Browse files Browse the repository at this point in the history
…cument the behavior (#4221)
  • Loading branch information
dmathieu authored Jun 26, 2023
1 parent dc187e7 commit 3b124b3
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Return an error on the creation of new instruments if their name doesn't pass regexp validation. (#4210)
- `NewManualReader` in `go.opentelemetry.io/otel/sdk/metric` returns `*ManualReader` instead of `Reader`. (#4244)
- `NewPeriodicReader` in `go.opentelemetry.io/otel/sdk/metric` returns `*PeriodicReader` instead of `Reader`. (#4244)
- Count the Collect time in the PeriodicReader timeout. (#4221)

## [1.16.0/0.39.0] 2023-05-18

Expand Down
17 changes: 9 additions & 8 deletions sdk/metric/periodic_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ func WithInterval(d time.Duration) PeriodicReaderOption {

// NewPeriodicReader returns a Reader that collects and exports metric data to
// the exporter at a defined interval. By default, the returned Reader will
// collect and export data every 60 seconds, and will cancel export attempts
// that exceed 30 seconds. The export time is not counted towards the interval
// between attempts.
// collect and export data every 60 seconds, and will cancel any attempts that
// exceed 30 seconds, collect and export combined. The collect and export time
// are not counted towards the interval between attempts.
//
// The Collect method of the returned Reader continues to gather and return
// metric data to the user. It will not automatically send that data to the
Expand Down Expand Up @@ -221,6 +221,9 @@ func (r *PeriodicReader) aggregation(kind InstrumentKind) aggregation.Aggregatio
// collectAndExport gather all metric data related to the periodicReader r from
// the SDK and exports it with r's exporter.
func (r *PeriodicReader) collectAndExport(ctx context.Context) error {
ctx, cancel := context.WithTimeout(ctx, r.timeout)
defer cancel()

// TODO (#3047): Use a sync.Pool or persistent pointer instead of allocating rm every Collect.
rm := r.rmPool.Get().(*metricdata.ResourceMetrics)
err := r.Collect(ctx, rm)
Expand All @@ -236,8 +239,8 @@ func (r *PeriodicReader) collectAndExport(ctx context.Context) error {
// data is not exported to the configured exporter, it is left to the caller to
// handle that if desired.
//
// An error is returned if this is called after Shutdown. An error is return if rm is nil.
//
// An error is returned if this is called after Shutdown, if rm is nil or if
// the duration of the collect and export exceeded the timeout.
// This method is safe to call concurrently.
func (r *PeriodicReader) Collect(ctx context.Context, rm *metricdata.ResourceMetrics) error {
if rm == nil {
Expand Down Expand Up @@ -280,9 +283,7 @@ func (r *PeriodicReader) collect(ctx context.Context, p interface{}, rm *metricd

// export exports metric data m using r's exporter.
func (r *PeriodicReader) export(ctx context.Context, m *metricdata.ResourceMetrics) error {
c, cancel := context.WithTimeout(ctx, r.timeout)
defer cancel()
return r.exporter.Export(c, m)
return r.exporter.Export(ctx, m)
}

// ForceFlush flushes pending telemetry.
Expand Down

0 comments on commit 3b124b3

Please sign in to comment.