From b1c1e781e6df6ac8a280da21766a00d16a45a9ed Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 16 Mar 2022 14:33:59 -0700 Subject: [PATCH] Support general attribute limits for spans (#2677) * Support general attribute limits for spans When model specific limits are not set fallback to the general ones defined by environment variables before defaults for attribute length and count limits. This is in compliance with the specification. * Update env pkg unit tests Test all environment variable helper functions. * Add changes to changelog * Update firstInt doc --- CHANGELOG.md | 1 + sdk/internal/env/env.go | 54 ++++++++++++++---- sdk/internal/env/env_test.go | 103 ++++++++++++++++++++++++++++------- 3 files changed, 126 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da2760a0e..53543ace6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Code instrumented with the `go.opentelemetry.io/otel/metric` will need to be mod - Remove the OTLP trace exporter limit of SpanEvents when exporting. (#2616) - Default to port `4318` instead of `4317` for the `otlpmetrichttp` and `otlptracehttp` client. (#2614, #2625) - Unlimited span limits are now supported (negative values). (#2636, #2637) +- Fallback to general attribute limits when span specific ones are not set in the environment. (#2675, #2677) ### Deprecated diff --git a/sdk/internal/env/env.go b/sdk/internal/env/env.go index dbc8f5124..1a03bf7af 100644 --- a/sdk/internal/env/env.go +++ b/sdk/internal/env/env.go @@ -41,16 +41,24 @@ const ( // i.e. 512 BatchSpanProcessorMaxExportBatchSizeKey = "OTEL_BSP_MAX_EXPORT_BATCH_SIZE" - // SpanAttributeValueLengthKey + // AttributeValueLengthKey // Maximum allowed attribute value size. + AttributeValueLengthKey = "OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT" + + // AttributeCountKey + // Maximum allowed span attribute count + AttributeCountKey = "OTEL_ATTRIBUTE_COUNT_LIMIT" + + // SpanAttributeValueLengthKey + // Maximum allowed attribute value size for a span. SpanAttributeValueLengthKey = "OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT" // SpanAttributeCountKey - // Maximum allowed span attribute count + // Maximum allowed span attribute count for a span. SpanAttributeCountKey = "OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT" // SpanEventCountKey - // Maximum allowed span event count + // Maximum allowed span event count. SpanEventCountKey = "OTEL_SPAN_EVENT_COUNT_LIMIT" // SpanEventAttributeCountKey @@ -58,14 +66,36 @@ const ( SpanEventAttributeCountKey = "OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT" // SpanLinkCountKey - // Maximum allowed span link count + // Maximum allowed span link count. SpanLinkCountKey = "OTEL_SPAN_LINK_COUNT_LIMIT" // SpanLinkAttributeCountKey - // Maximum allowed attribute per span link count + // Maximum allowed attribute per span link count. SpanLinkAttributeCountKey = "OTEL_LINK_ATTRIBUTE_COUNT_LIMIT" ) +// firstInt returns the value of the first matching environment variable from +// keys. If the value is not an integer or no match is found, defaultValue is +// returned. +func firstInt(defaultValue int, keys ...string) int { + for _, key := range keys { + value, ok := os.LookupEnv(key) + if !ok { + continue + } + + intValue, err := strconv.Atoi(value) + if err != nil { + global.Info("Got invalid value, number value expected.", key, value) + return defaultValue + } + + return intValue + } + + return defaultValue +} + // IntEnvOr returns the int value of the environment variable with name key if // it exists and the value is an int. Otherwise, defaultValue is returned. func IntEnvOr(key string, defaultValue int) int { @@ -112,17 +142,19 @@ func BatchSpanProcessorMaxExportBatchSize(defaultValue int) int { } // SpanAttributeValueLength returns the environment variable value for the -// OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT key if it exists, otherwise -// defaultValue is returned. +// OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT key if it exists. Otherwise, the +// environment variable value for OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT is +// returned or defaultValue if that is not set. func SpanAttributeValueLength(defaultValue int) int { - return IntEnvOr(SpanAttributeValueLengthKey, defaultValue) + return firstInt(defaultValue, SpanAttributeValueLengthKey, AttributeValueLengthKey) } // SpanAttributeCount returns the environment variable value for the -// OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT key if it exists, otherwise defaultValue is -// returned. +// OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT key if it exists. Otherwise, the +// environment variable value for OTEL_ATTRIBUTE_COUNT_LIMIT is returned or +// defaultValue if that is not set. func SpanAttributeCount(defaultValue int) int { - return IntEnvOr(SpanAttributeCountKey, defaultValue) + return firstInt(defaultValue, SpanAttributeCountKey, AttributeCountKey) } // SpanEventCount returns the environment variable value for the diff --git a/sdk/internal/env/env_test.go b/sdk/internal/env/env_test.go index 8c293f654..e150f108c 100644 --- a/sdk/internal/env/env_test.go +++ b/sdk/internal/env/env_test.go @@ -24,37 +24,98 @@ import ( ottest "go.opentelemetry.io/otel/internal/internaltest" ) -func TestIntEnvOr(t *testing.T) { +func TestEnvParse(t *testing.T) { testCases := []struct { - name string - envValue string - defaultValue int - expectedValue int + name string + keys []string + f func(int) int }{ { - name: "IntEnvOrTest - Basic", - envValue: "2500", - defaultValue: 500, - expectedValue: 2500, + name: "BatchSpanProcessorScheduleDelay", + keys: []string{BatchSpanProcessorScheduleDelayKey}, + f: BatchSpanProcessorScheduleDelay, }, + { - name: "IntEnvOrTest - Invalid Number", - envValue: "localhost", - defaultValue: 500, - expectedValue: 500, + name: "BatchSpanProcessorExportTimeout", + keys: []string{BatchSpanProcessorExportTimeoutKey}, + f: BatchSpanProcessorExportTimeout, + }, + + { + name: "BatchSpanProcessorMaxQueueSize", + keys: []string{BatchSpanProcessorMaxQueueSizeKey}, + f: BatchSpanProcessorMaxQueueSize, + }, + + { + name: "BatchSpanProcessorMaxExportBatchSize", + keys: []string{BatchSpanProcessorMaxExportBatchSizeKey}, + f: BatchSpanProcessorMaxExportBatchSize, + }, + + { + name: "SpanAttributeValueLength", + keys: []string{SpanAttributeValueLengthKey, AttributeValueLengthKey}, + f: SpanAttributeValueLength, + }, + + { + name: "SpanAttributeCount", + keys: []string{SpanAttributeCountKey, AttributeCountKey}, + f: SpanAttributeCount, + }, + + { + name: "SpanEventCount", + keys: []string{SpanEventCountKey}, + f: SpanEventCount, + }, + + { + name: "SpanEventAttributeCount", + keys: []string{SpanEventAttributeCountKey}, + f: SpanEventAttributeCount, + }, + + { + name: "SpanLinkCount", + keys: []string{SpanLinkCountKey}, + f: SpanLinkCount, + }, + + { + name: "SpanLinkAttributeCount", + keys: []string{SpanLinkAttributeCountKey}, + f: SpanLinkAttributeCount, }, } - envStore := ottest.NewEnvStore() - envStore.Record(BatchSpanProcessorMaxQueueSizeKey) - defer func() { - require.NoError(t, envStore.Restore()) - }() + const ( + defVal = 500 + envVal = 2500 + envValStr = "2500" + invalid = "localhost" + ) + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - require.NoError(t, os.Setenv(BatchSpanProcessorMaxQueueSizeKey, tc.envValue)) - actualValue := BatchSpanProcessorMaxQueueSize(tc.defaultValue) - assert.Equal(t, tc.expectedValue, actualValue) + for _, key := range tc.keys { + t.Run(key, func(t *testing.T) { + envStore := ottest.NewEnvStore() + t.Cleanup(func() { require.NoError(t, envStore.Restore()) }) + envStore.Record(key) + + assert.Equal(t, defVal, tc.f(defVal), "environment variable unset") + + require.NoError(t, os.Setenv(key, envValStr)) + assert.Equal(t, envVal, tc.f(defVal), "environment variable set/valid") + + require.NoError(t, os.Setenv(key, invalid)) + assert.Equal(t, defVal, tc.f(defVal), "invalid value") + }) + + } }) } }