Skip to content

fix: error raised for env checks in LogLimits and SpanLimits #4458

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

Merged
merged 3 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#4448](https://github.com/open-telemetry/opentelemetry-python/pull/4448))
- Make `trace_api.use_span()` record `BaseException` as well as `Exception`
([#4406](https://github.com/open-telemetry/opentelemetry-python/pull/4406))
- Fix env var error message for TraceLimits/SpanLimits
([#4458](https://github.com/open-telemetry/opentelemetry-python/pull/4458))

## Version 1.30.0/0.51b0 (2025-02-03)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def _from_env_if_absent(
if value == cls.UNSET:
return None

err_msg = "{0} must be a non-negative integer but got {}"
err_msg = "{} must be a non-negative integer but got {}"

# if no value is provided for the limit, try to load it from env
if value is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ def _from_env_if_absent(
if value == cls.UNSET:
return None

err_msg = "{0} must be a non-negative integer but got {}"
err_msg = "{} must be a non-negative integer but got {}"

# if no value is provided for the limit, try to load it from env
if value is None:
Expand Down
32 changes: 32 additions & 0 deletions opentelemetry-sdk/tests/logs/test_log_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@
# limitations under the License.

import unittest
from unittest.mock import patch

from opentelemetry.sdk._logs import LogLimits
from opentelemetry.sdk._logs._internal import (
_DEFAULT_OTEL_ATTRIBUTE_COUNT_LIMIT,
)
from opentelemetry.sdk.environment_variables import (
OTEL_ATTRIBUTE_COUNT_LIMIT,
OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
)


class TestLogLimits(unittest.TestCase):
Expand All @@ -38,3 +43,30 @@ def test_log_limits_max_attribute_length(self):
limits = LogLimits(max_attribute_length=1)

self.assertEqual(expected, limits.max_attribute_length)

def test_invalid_env_vars_raise(self):
env_vars = [
OTEL_ATTRIBUTE_COUNT_LIMIT,
OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
]

bad_values = ["bad", "-1"]
test_cases = {
env_var: bad_value
for env_var in env_vars
for bad_value in bad_values
}

for env_var, bad_value in test_cases.items():
with self.subTest(f"Testing {env_var}={bad_value}"):
with self.assertRaises(ValueError) as error, patch.dict(
"os.environ", {env_var: bad_value}, clear=True
):
LogLimits()

expected_msg = f"{env_var} must be a non-negative integer but got {bad_value}"
self.assertEqual(
expected_msg,
str(error.exception),
f"Unexpected error message for {env_var}={bad_value}",
)
31 changes: 31 additions & 0 deletions opentelemetry-sdk/tests/trace/test_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -2007,6 +2007,37 @@ def _test_span_no_limits(self, tracer):
for attr_val in root.attributes.values():
self.assertEqual(attr_val, self.long_val)

def test_invalid_env_vars_raise(self):
env_vars = [
OTEL_SPAN_EVENT_COUNT_LIMIT,
OTEL_SPAN_LINK_COUNT_LIMIT,
OTEL_ATTRIBUTE_COUNT_LIMIT,
OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT,
OTEL_LINK_ATTRIBUTE_COUNT_LIMIT,
OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
]
bad_values = ["bad", "-1"]
test_cases = {
env_var: bad_value
for env_var in env_vars
for bad_value in bad_values
}

for env_var, bad_value in test_cases.items():
with self.subTest(f"Testing {env_var}={bad_value}"):
with self.assertRaises(ValueError) as error, patch.dict(
"os.environ", {env_var: bad_value}, clear=True
):
trace.SpanLimits()

expected_msg = f"{env_var} must be a non-negative integer but got {bad_value}"
self.assertEqual(
expected_msg,
str(error.exception),
f"Unexpected error message for {env_var}={bad_value}",
)


class TestTraceFlags(unittest.TestCase):
def test_constant_default(self):
Expand Down