From 1d6704c8fc57b78060405422302c1f00a4a02e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 24 Feb 2023 16:26:47 +0100 Subject: [PATCH] Support OTEL_METRIC_EXPORT_INTERVAL and OTEL_METRIC_EXPORT_TIMEOUT (#3763) * Support OTEL_METRIC_EXPORT_INTERVAL and OTEL_METRIC_EXPORT_TIMEOUT * Fix non-positive duration error log Co-authored-by: Tyler Yahn --- CHANGELOG.md | 3 + sdk/metric/env.go | 50 ++++++++++++++++ sdk/metric/periodic_reader.go | 10 +++- sdk/metric/periodic_reader_test.go | 96 ++++++++++++++++++++++++++++++ sdk/metric/reader.go | 4 ++ 5 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 sdk/metric/env.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c58eb5fc67f..39078e56970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `bridgetSpanContext.IsSampled` to `go.opentelemetry.io/otel/bridget/opentracing` to expose whether span is sampled or not. (#3570) - The `WithInstrumentationAttributes` option to `go.opentelemetry.io/otel/metric`. (#3738) - The `WithInstrumentationAttributes` option to `go.opentelemetry.io/otel/trace`. (#3739) +- The following environment variables are supported by the `Reader`s in `go.opentelemetry.io/otel/sdk/metric`. (#3763) + - `OTEL_METRIC_EXPORT_INTERVAL` + - `OTEL_METRIC_EXPORT_TIMEOUT` ### Changed diff --git a/sdk/metric/env.go b/sdk/metric/env.go new file mode 100644 index 00000000000..940ba815942 --- /dev/null +++ b/sdk/metric/env.go @@ -0,0 +1,50 @@ +// Copyright The OpenTelemetry Authors +// +// 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 metric // import "go.opentelemetry.io/otel/sdk/metric" + +import ( + "os" + "strconv" + "time" + + "go.opentelemetry.io/otel/internal/global" +) + +// Environment variable names. +const ( + // The time interval (in milliseconds) between the start of two export attempts. + envInterval = "OTEL_METRIC_EXPORT_INTERVAL" + // Maximum allowed time (in milliseconds) to export data. + envTimeout = "OTEL_METRIC_EXPORT_TIMEOUT" +) + +// envDuration returns an environment variable's value as duration in milliseconds if it is exists, +// or the defaultValue if the environment variable is not defined or the value is not valid. +func envDuration(key string, defaultValue time.Duration) time.Duration { + v := os.Getenv(key) + if v == "" { + return defaultValue + } + d, err := strconv.Atoi(v) + if err != nil { + global.Error(err, "parse duration", "environment variable", key, "value", v) + return defaultValue + } + if d <= 0 { + global.Error(errNonPositiveDuration, "non-positive duration", "environment variable", key, "value", v) + return defaultValue + } + return time.Duration(d) * time.Millisecond +} diff --git a/sdk/metric/periodic_reader.go b/sdk/metric/periodic_reader.go index 2c1d00bed93..3ba93293bf7 100644 --- a/sdk/metric/periodic_reader.go +++ b/sdk/metric/periodic_reader.go @@ -44,8 +44,8 @@ type periodicReaderConfig struct { // options. func newPeriodicReaderConfig(options []PeriodicReaderOption) periodicReaderConfig { c := periodicReaderConfig{ - interval: defaultInterval, - timeout: defaultTimeout, + interval: envDuration(envInterval, defaultInterval), + timeout: envDuration(envTimeout, defaultTimeout), } for _, o := range options { c = o.applyPeriodic(c) @@ -69,6 +69,9 @@ func (o periodicReaderOptionFunc) applyPeriodic(conf periodicReaderConfig) perio // WithTimeout configures the time a PeriodicReader waits for an export to // complete before canceling it. // +// This option overrides any value set for the +// OTEL_METRIC_EXPORT_TIMEOUT environment variable. +// // If this option is not used or d is less than or equal to zero, 30 seconds // is used as the default. func WithTimeout(d time.Duration) PeriodicReaderOption { @@ -84,6 +87,9 @@ func WithTimeout(d time.Duration) PeriodicReaderOption { // WithInterval configures the intervening time between exports for a // PeriodicReader. // +// This option overrides any value set for the +// OTEL_METRIC_EXPORT_INTERVAL environment variable. +// // If this option is not used or d is less than or equal to zero, 60 seconds // is used as the default. func WithInterval(d time.Duration) PeriodicReaderOption { diff --git a/sdk/metric/periodic_reader_test.go b/sdk/metric/periodic_reader_test.go index 138aae48944..a6b319e8abc 100644 --- a/sdk/metric/periodic_reader_test.go +++ b/sdk/metric/periodic_reader_test.go @@ -41,6 +41,54 @@ func TestWithTimeout(t *testing.T) { assert.Equal(t, defaultTimeout, test(time.Duration(-1)), "invalid timeout should use default") } +func TestTimeoutEnvVar(t *testing.T) { + testCases := []struct { + v string + want time.Duration + }{ + { + // empty value + "", + defaultTimeout, + }, + { + // positive value + "1", + time.Millisecond, + }, + { + // non-positive value + "0", + defaultTimeout, + }, + { + // value with unit (not supported) + "1ms", + defaultTimeout, + }, + { + // NaN + "abc", + defaultTimeout, + }, + } + for _, tc := range testCases { + t.Run(tc.v, func(t *testing.T) { + t.Setenv(envTimeout, tc.v) + got := newPeriodicReaderConfig(nil).timeout + assert.Equal(t, tc.want, got) + }) + } +} + +func TestTimeoutEnvAndOption(t *testing.T) { + want := 5 * time.Millisecond + t.Setenv(envTimeout, "999") + opts := []PeriodicReaderOption{WithTimeout(want)} + got := newPeriodicReaderConfig(opts).timeout + assert.Equal(t, want, got, "option should have precedence over env var") +} + func TestWithInterval(t *testing.T) { test := func(d time.Duration) time.Duration { opts := []PeriodicReaderOption{WithInterval(d)} @@ -53,6 +101,54 @@ func TestWithInterval(t *testing.T) { assert.Equal(t, defaultInterval, test(time.Duration(-1)), "invalid interval should use default") } +func TestIntervalEnvVar(t *testing.T) { + testCases := []struct { + v string + want time.Duration + }{ + { + // empty value + "", + defaultInterval, + }, + { + // positive value + "1", + time.Millisecond, + }, + { + // non-positive value + "0", + defaultInterval, + }, + { + // value with unit (not supported) + "1ms", + defaultInterval, + }, + { + // NaN + "abc", + defaultInterval, + }, + } + for _, tc := range testCases { + t.Run(tc.v, func(t *testing.T) { + t.Setenv(envInterval, tc.v) + got := newPeriodicReaderConfig(nil).interval + assert.Equal(t, tc.want, got) + }) + } +} + +func TestIntervalEnvAndOption(t *testing.T) { + want := 5 * time.Millisecond + t.Setenv(envInterval, "999") + opts := []PeriodicReaderOption{WithInterval(want)} + got := newPeriodicReaderConfig(opts).interval + assert.Equal(t, want, got, "option should have precedence over env var") +} + type fnExporter struct { temporalityFunc TemporalitySelector aggregationFunc AggregationSelector diff --git a/sdk/metric/reader.go b/sdk/metric/reader.go index 9ba6c867d5b..9d6972b53d8 100644 --- a/sdk/metric/reader.go +++ b/sdk/metric/reader.go @@ -34,6 +34,10 @@ var ErrReaderNotRegistered = fmt.Errorf("reader is not registered") // reader has been Shutdown once. var ErrReaderShutdown = fmt.Errorf("reader is shutdown") +// errNonPositiveDuration is logged when an environmental variable +// has non-positive value. +var errNonPositiveDuration = fmt.Errorf("non-positive duration") + // Reader is the interface used between the SDK and an // exporter. Control flow is bi-directional through the // Reader, since the SDK initiates ForceFlush and Shutdown