From fc1f3baa9bb807f0f51b571ae61299feed632b92 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Wed, 4 Sep 2024 20:40:19 +0000 Subject: [PATCH] enable exemplars by default --- CHANGELOG.md | 4 ++ sdk/internal/x/x_test.go | 2 +- sdk/metric/exemplar.go | 4 -- sdk/metric/internal/x/x.go | 46 +++++++------------- sdk/metric/internal/x/x_test.go | 12 ----- sdk/metric/pipeline_test.go | 77 ++++++++++++++------------------- 6 files changed, 52 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8c720345000..6d57ef903c27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Changed + +- Enable exemplars by default in `go.opentelemetry.io/otel/sdk/metric`. Exemplars can be disabled by setting `OTEL_METRICS_EXEMPLAR_FILTER=always_off` (#5778) + ### Fixed - Fix memory leak in the global `MeterProvider` when identical instruments are repeatedly created. (#5754) diff --git a/sdk/internal/x/x_test.go b/sdk/internal/x/x_test.go index 0c03f2fdb1f2..b058c3a24052 100644 --- a/sdk/internal/x/x_test.go +++ b/sdk/internal/x/x_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestExemplars(t *testing.T) { +func TestResource(t *testing.T) { const key = "OTEL_GO_X_RESOURCE" require.Equal(t, key, Resource.Key()) diff --git a/sdk/metric/exemplar.go b/sdk/metric/exemplar.go index 82619da78ec1..8a05d881d2d1 100644 --- a/sdk/metric/exemplar.go +++ b/sdk/metric/exemplar.go @@ -9,7 +9,6 @@ import ( "slices" "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" - "go.opentelemetry.io/otel/sdk/metric/internal/x" ) // reservoirFunc returns the appropriately configured exemplar reservoir @@ -20,9 +19,6 @@ import ( // feature is enabled and the OTEL_METRICS_EXEMPLAR_FILTER environment variable // is not set to always_off. func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.FilteredReservoir[N] { - if !x.Exemplars.Enabled() { - return nil - } // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/configuration/sdk-environment-variables.md#exemplar const filterEnvKey = "OTEL_METRICS_EXEMPLAR_FILTER" diff --git a/sdk/metric/internal/x/x.go b/sdk/metric/internal/x/x.go index 8cd2f37417bb..089199370688 100644 --- a/sdk/metric/internal/x/x.go +++ b/sdk/metric/internal/x/x.go @@ -10,39 +10,23 @@ package x // import "go.opentelemetry.io/otel/sdk/metric/internal/x" import ( "os" "strconv" - "strings" ) -var ( - // Exemplars is an experimental feature flag that defines if exemplars - // should be recorded for metric data-points. - // - // To enable this feature set the OTEL_GO_X_EXEMPLAR environment variable - // to the case-insensitive string value of "true" (i.e. "True" and "TRUE" - // will also enable this). - Exemplars = newFeature("EXEMPLAR", func(v string) (string, bool) { - if strings.ToLower(v) == "true" { - return v, true - } - return "", false - }) - - // CardinalityLimit is an experimental feature flag that defines if - // cardinality limits should be applied to the recorded metric data-points. - // - // To enable this feature set the OTEL_GO_X_CARDINALITY_LIMIT environment - // variable to the integer limit value you want to use. - // - // Setting OTEL_GO_X_CARDINALITY_LIMIT to a value less than or equal to 0 - // will disable the cardinality limits. - CardinalityLimit = newFeature("CARDINALITY_LIMIT", func(v string) (int, bool) { - n, err := strconv.Atoi(v) - if err != nil { - return 0, false - } - return n, true - }) -) +// CardinalityLimit is an experimental feature flag that defines if +// cardinality limits should be applied to the recorded metric data-points. +// +// To enable this feature set the OTEL_GO_X_CARDINALITY_LIMIT environment +// variable to the integer limit value you want to use. +// +// Setting OTEL_GO_X_CARDINALITY_LIMIT to a value less than or equal to 0 +// will disable the cardinality limits. +var CardinalityLimit = newFeature("CARDINALITY_LIMIT", func(v string) (int, bool) { + n, err := strconv.Atoi(v) + if err != nil { + return 0, false + } + return n, true +}) // Feature is an experimental feature control flag. It provides a uniform way // to interact with these feature flags and parse their values. diff --git a/sdk/metric/internal/x/x_test.go b/sdk/metric/internal/x/x_test.go index 7a5dd15965df..257ca76137d9 100644 --- a/sdk/metric/internal/x/x_test.go +++ b/sdk/metric/internal/x/x_test.go @@ -10,18 +10,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestExemplars(t *testing.T) { - const key = "OTEL_GO_X_EXEMPLAR" - require.Equal(t, key, Exemplars.Key()) - - t.Run("true", run(setenv(key, "true"), assertEnabled(Exemplars, "true"))) - t.Run("True", run(setenv(key, "True"), assertEnabled(Exemplars, "True"))) - t.Run("TRUE", run(setenv(key, "TRUE"), assertEnabled(Exemplars, "TRUE"))) - t.Run("false", run(setenv(key, "false"), assertDisabled(Exemplars))) - t.Run("1", run(setenv(key, "1"), assertDisabled(Exemplars))) - t.Run("empty", run(assertDisabled(Exemplars))) -} - func TestCardinalityLimit(t *testing.T) { const key = "OTEL_GO_X_CARDINALITY_LIMIT" require.Equal(t, key, CardinalityLimit.Key()) diff --git a/sdk/metric/pipeline_test.go b/sdk/metric/pipeline_test.go index a5995b626a60..5036bd5d974d 100644 --- a/sdk/metric/pipeline_test.go +++ b/sdk/metric/pipeline_test.go @@ -448,59 +448,46 @@ func TestExemplars(t *testing.T) { }) sampled := trace.ContextWithSpanContext(context.Background(), sc) - t.Run("OTEL_GO_X_EXEMPLAR=true", func(t *testing.T) { - t.Setenv("OTEL_GO_X_EXEMPLAR", "true") - - t.Run("Default", func(t *testing.T) { - m, r := setup("default") - measure(ctx, m) - check(t, r, 0, 0, 0) - - measure(sampled, m) - check(t, r, nCPU, 1, 20) - }) - - t.Run("Invalid", func(t *testing.T) { - t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "unrecognized") - m, r := setup("default") - measure(ctx, m) - check(t, r, 0, 0, 0) - - measure(sampled, m) - check(t, r, nCPU, 1, 20) - }) - - t.Run("always_on", func(t *testing.T) { - t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "always_on") - m, r := setup("always_on") - measure(ctx, m) - check(t, r, nCPU, 1, 20) - }) + t.Run("Default", func(t *testing.T) { + m, r := setup("default") + measure(ctx, m) + check(t, r, 0, 0, 0) - t.Run("always_off", func(t *testing.T) { - t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "always_off") - m, r := setup("always_off") - measure(ctx, m) - check(t, r, 0, 0, 0) - }) + measure(sampled, m) + check(t, r, nCPU, 1, 20) + }) - t.Run("trace_based", func(t *testing.T) { - t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "trace_based") - m, r := setup("trace_based") - measure(ctx, m) - check(t, r, 0, 0, 0) + t.Run("Invalid", func(t *testing.T) { + t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "unrecognized") + m, r := setup("default") + measure(ctx, m) + check(t, r, 0, 0, 0) - measure(sampled, m) - check(t, r, nCPU, 1, 20) - }) + measure(sampled, m) + check(t, r, nCPU, 1, 20) }) - t.Run("OTEL_GO_X_EXEMPLAR=false", func(t *testing.T) { - t.Setenv("OTEL_GO_X_EXEMPLAR", "false") - + t.Run("always_on", func(t *testing.T) { t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "always_on") m, r := setup("always_on") measure(ctx, m) + check(t, r, nCPU, 1, 20) + }) + + t.Run("always_off", func(t *testing.T) { + t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "always_off") + m, r := setup("always_off") + measure(ctx, m) + check(t, r, 0, 0, 0) + }) + + t.Run("trace_based", func(t *testing.T) { + t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "trace_based") + m, r := setup("trace_based") + measure(ctx, m) check(t, r, 0, 0, 0) + + measure(sampled, m) + check(t, r, nCPU, 1, 20) }) }