Skip to content

Commit 8329ae4

Browse files
committed
fix pylint, pyright and ruff errors in resource/propagator config
- _resource.py: refactor _coerce_attribute_value to dispatch table to avoid too-many-return-statements; fix short variable names k/v -> attr_key/attr_val; fix return type of _sdk_default_attributes to dict[str, str] to satisfy pyright - _propagator.py: rename short variable names e -> exc, p -> propagator - test_resource.py: move imports to top level; split TestCreateResource (25 methods) into three focused classes to satisfy too-many-public-methods - test_propagator.py: add pylint disable for protected-access Assisted-by: Claude Sonnet 4.6
1 parent 8232012 commit 8329ae4

File tree

4 files changed

+114
-100
lines changed

4 files changed

+114
-100
lines changed

opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_propagator.py

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
from opentelemetry.sdk._configuration._exceptions import ConfigurationError
2525
from opentelemetry.sdk._configuration.models import (
2626
Propagator as PropagatorConfig,
27+
)
28+
from opentelemetry.sdk._configuration.models import (
2729
TextMapPropagator as TextMapPropagatorConfig,
2830
)
2931
from opentelemetry.trace.propagation.tracecontext import (
@@ -37,9 +39,7 @@
3739
def _load_entry_point_propagator(name: str) -> TextMapPropagator:
3840
"""Load a propagator by name from the opentelemetry_propagator entry point group."""
3941
try:
40-
eps = list(
41-
entry_points(group="opentelemetry_propagator", name=name)
42-
)
42+
eps = list(entry_points(group="opentelemetry_propagator", name=name))
4343
if not eps:
4444
raise ConfigurationError(
4545
f"Propagator '{name}' not found. "
@@ -48,10 +48,10 @@ def _load_entry_point_propagator(name: str) -> TextMapPropagator:
4848
return eps[0].load()()
4949
except ConfigurationError:
5050
raise
51-
except Exception as e:
51+
except Exception as exc:
5252
raise ConfigurationError(
53-
f"Failed to load propagator '{name}': {e}"
54-
) from e
53+
f"Failed to load propagator '{name}': {exc}"
54+
) from exc
5555

5656

5757
def _propagators_from_textmap_config(
@@ -91,25 +91,24 @@ def create_propagator(
9191
propagators: list[TextMapPropagator] = []
9292
seen_types: set[type] = set()
9393

94-
def _add_deduped(p: TextMapPropagator) -> None:
95-
if type(p) not in seen_types:
96-
seen_types.add(type(p))
97-
propagators.append(p)
94+
def _add_deduped(propagator: TextMapPropagator) -> None:
95+
if type(propagator) not in seen_types:
96+
seen_types.add(type(propagator))
97+
propagators.append(propagator)
9898

9999
# Process structured composite list
100100
if config.composite:
101101
for entry in config.composite:
102-
for p in _propagators_from_textmap_config(entry):
103-
_add_deduped(p)
102+
for propagator in _propagators_from_textmap_config(entry):
103+
_add_deduped(propagator)
104104

105105
# Process composite_list (comma-separated propagator names via entry_points)
106106
if config.composite_list:
107107
for name in config.composite_list.split(","):
108108
name = name.strip()
109109
if not name or name.lower() == "none":
110110
continue
111-
p = _load_entry_point_propagator(name)
112-
_add_deduped(p)
111+
_add_deduped(_load_entry_point_propagator(name))
113112

114113
return CompositePropagator(propagators)
115114

opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_resource.py

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,40 @@
2121
from opentelemetry.sdk._configuration.models import (
2222
AttributeNameValue,
2323
AttributeType,
24-
Resource as ResourceConfig,
2524
)
25+
from opentelemetry.sdk._configuration.models import Resource as ResourceConfig
2626
from opentelemetry.sdk.resources import (
27+
_OPENTELEMETRY_SDK_VERSION,
2728
SERVICE_NAME,
2829
TELEMETRY_SDK_LANGUAGE,
2930
TELEMETRY_SDK_NAME,
3031
TELEMETRY_SDK_VERSION,
3132
Resource,
32-
_OPENTELEMETRY_SDK_VERSION,
3333
)
3434

3535
_logger = logging.getLogger(__name__)
3636

37+
# Dispatch table for scalar type coercions
38+
_SCALAR_COERCIONS = {
39+
AttributeType.string: str,
40+
AttributeType.int: int,
41+
AttributeType.double: float,
42+
}
43+
44+
# Dispatch table for array type coercions
45+
_ARRAY_COERCIONS = {
46+
AttributeType.string_array: str,
47+
AttributeType.bool_array: bool,
48+
AttributeType.int_array: int,
49+
AttributeType.double_array: float,
50+
}
51+
52+
53+
def _coerce_bool(value: object) -> bool:
54+
if isinstance(value, str):
55+
return value.lower() not in ("false", "0", "")
56+
return bool(value)
57+
3758

3859
def _coerce_attribute_value(attr: AttributeNameValue) -> object:
3960
"""Coerce an attribute value to the correct Python type based on AttributeType."""
@@ -42,26 +63,14 @@ def _coerce_attribute_value(attr: AttributeNameValue) -> object:
4263

4364
if attr_type is None:
4465
return value
45-
46-
if attr_type == AttributeType.string:
47-
return str(value)
4866
if attr_type == AttributeType.bool:
49-
if isinstance(value, str):
50-
return value.lower() not in ("false", "0", "")
51-
return bool(value)
52-
if attr_type == AttributeType.int:
53-
return int(value) # type: ignore[arg-type]
54-
if attr_type == AttributeType.double:
55-
return float(value) # type: ignore[arg-type]
56-
if attr_type == AttributeType.string_array:
57-
return [str(v) for v in value] # type: ignore[union-attr]
58-
if attr_type == AttributeType.bool_array:
59-
return [bool(v) for v in value] # type: ignore[union-attr]
60-
if attr_type == AttributeType.int_array:
61-
return [int(v) for v in value] # type: ignore[union-attr,arg-type]
62-
if attr_type == AttributeType.double_array:
63-
return [float(v) for v in value] # type: ignore[union-attr,arg-type]
64-
67+
return _coerce_bool(value)
68+
scalar_coercer = _SCALAR_COERCIONS.get(attr_type)
69+
if scalar_coercer is not None:
70+
return scalar_coercer(value) # type: ignore[arg-type]
71+
array_coercer = _ARRAY_COERCIONS.get(attr_type)
72+
if array_coercer is not None:
73+
return [array_coercer(item) for item in value] # type: ignore[union-attr,arg-type]
6574
return value
6675

6776

@@ -87,7 +96,7 @@ def _parse_attributes_list(attributes_list: str) -> dict[str, str]:
8796
return result
8897

8998

90-
def _sdk_default_attributes() -> dict[str, object]:
99+
def _sdk_default_attributes() -> dict[str, str]:
91100
"""Return the SDK telemetry attributes (equivalent to Java's Resource.getDefault())."""
92101
return {
93102
TELEMETRY_SDK_LANGUAGE: "python",
@@ -125,9 +134,9 @@ def create_resource(config: Optional[ResourceConfig]) -> Resource:
125134
if config.attributes_list:
126135
list_attrs = _parse_attributes_list(config.attributes_list)
127136
# attributes_list entries do not override explicit attributes
128-
for k, v in list_attrs.items():
129-
if k not in config_attrs:
130-
config_attrs[k] = v
137+
for attr_key, attr_val in list_attrs.items():
138+
if attr_key not in config_attrs:
139+
config_attrs[attr_key] = attr_val
131140

132141
schema_url = config.schema_url
133142

opentelemetry-sdk/tests/_configuration/test_propagator.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
import unittest
1616
from unittest.mock import MagicMock, patch
1717

18+
# CompositePropagator stores its propagators in _propagators (private).
19+
# We access it here to assert composition correctness.
20+
# pylint: disable=protected-access
1821
from opentelemetry.baggage.propagation import W3CBaggagePropagator
1922
from opentelemetry.propagators.composite import CompositePropagator
2023
from opentelemetry.sdk._configuration._propagator import (
@@ -24,6 +27,8 @@
2427
from opentelemetry.sdk._configuration.file._loader import ConfigurationError
2528
from opentelemetry.sdk._configuration.models import (
2629
Propagator as PropagatorConfig,
30+
)
31+
from opentelemetry.sdk._configuration.models import (
2732
TextMapPropagator as TextMapPropagatorConfig,
2833
)
2934
from opentelemetry.trace.propagation.tracecontext import (
@@ -49,7 +54,8 @@ def test_tracecontext_only(self):
4954
result = create_propagator(config)
5055
self.assertEqual(len(result._propagators), 1) # type: ignore[attr-defined]
5156
self.assertIsInstance(
52-
result._propagators[0], TraceContextTextMapPropagator # type: ignore[attr-defined]
57+
result._propagators[0],
58+
TraceContextTextMapPropagator, # type: ignore[attr-defined]
5359
)
5460

5561
def test_baggage_only(self):
@@ -70,7 +76,8 @@ def test_tracecontext_and_baggage(self):
7076
result = create_propagator(config)
7177
self.assertEqual(len(result._propagators), 2) # type: ignore[attr-defined]
7278
self.assertIsInstance(
73-
result._propagators[0], TraceContextTextMapPropagator # type: ignore[attr-defined]
79+
result._propagators[0],
80+
TraceContextTextMapPropagator, # type: ignore[attr-defined]
7481
)
7582
self.assertIsInstance(result._propagators[1], W3CBaggagePropagator) # type: ignore[attr-defined]
7683

@@ -152,9 +159,7 @@ def fake_entry_points(group, name):
152159
"opentelemetry.sdk._configuration._propagator.entry_points",
153160
side_effect=fake_entry_points,
154161
):
155-
config = PropagatorConfig(
156-
composite_list="tracecontext,baggage"
157-
)
162+
config = PropagatorConfig(composite_list="tracecontext,baggage")
158163
result = create_propagator(config)
159164

160165
self.assertEqual(len(result._propagators), 2) # type: ignore[attr-defined]

opentelemetry-sdk/tests/_configuration/test_resource.py

Lines changed: 57 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import os
1516
import unittest
17+
from unittest.mock import patch
1618

1719
from opentelemetry.sdk._configuration._resource import create_resource
1820
from opentelemetry.sdk._configuration.models import (
1921
AttributeNameValue,
2022
AttributeType,
21-
Resource as ResourceConfig,
2223
)
24+
from opentelemetry.sdk._configuration.models import Resource as ResourceConfig
2325
from opentelemetry.sdk.resources import (
2426
SERVICE_NAME,
2527
TELEMETRY_SDK_LANGUAGE,
@@ -29,7 +31,7 @@
2931
)
3032

3133

32-
class TestCreateResource(unittest.TestCase):
34+
class TestCreateResourceDefaults(unittest.TestCase):
3335
def test_none_config_returns_sdk_defaults(self):
3436
resource = create_resource(None)
3537
self.assertIsInstance(resource, Resource)
@@ -38,17 +40,15 @@ def test_none_config_returns_sdk_defaults(self):
3840
resource.attributes[TELEMETRY_SDK_NAME], "opentelemetry"
3941
)
4042
self.assertIn(TELEMETRY_SDK_VERSION, resource.attributes)
41-
self.assertEqual(
42-
resource.attributes[SERVICE_NAME], "unknown_service"
43-
)
43+
self.assertEqual(resource.attributes[SERVICE_NAME], "unknown_service")
4444

4545
def test_none_config_does_not_read_env_vars(self):
46-
import os
47-
from unittest.mock import patch
48-
4946
with patch.dict(
5047
os.environ,
51-
{"OTEL_RESOURCE_ATTRIBUTES": "foo=bar", "OTEL_SERVICE_NAME": "my-service"},
48+
{
49+
"OTEL_RESOURCE_ATTRIBUTES": "foo=bar",
50+
"OTEL_SERVICE_NAME": "my-service",
51+
},
5252
):
5353
resource = create_resource(None)
5454
self.assertNotIn("foo", resource.attributes)
@@ -59,6 +59,49 @@ def test_empty_resource_config(self):
5959
self.assertEqual(resource.attributes[TELEMETRY_SDK_LANGUAGE], "python")
6060
self.assertEqual(resource.attributes[SERVICE_NAME], "unknown_service")
6161

62+
def test_service_name_default_added_when_missing(self):
63+
config = ResourceConfig(
64+
attributes=[AttributeNameValue(name="env", value="staging")]
65+
)
66+
resource = create_resource(config)
67+
self.assertEqual(resource.attributes[SERVICE_NAME], "unknown_service")
68+
69+
def test_service_name_not_overridden_when_set(self):
70+
config = ResourceConfig(
71+
attributes=[
72+
AttributeNameValue(name="service.name", value="my-app")
73+
]
74+
)
75+
resource = create_resource(config)
76+
self.assertEqual(resource.attributes[SERVICE_NAME], "my-app")
77+
78+
def test_env_vars_not_read(self):
79+
"""OTEL_RESOURCE_ATTRIBUTES must not affect declarative config resource."""
80+
with patch.dict(
81+
os.environ,
82+
{"OTEL_RESOURCE_ATTRIBUTES": "injected=true"},
83+
):
84+
config = ResourceConfig(
85+
attributes=[AttributeNameValue(name="k", value="v")]
86+
)
87+
resource = create_resource(config)
88+
self.assertNotIn("injected", resource.attributes)
89+
90+
def test_schema_url(self):
91+
config = ResourceConfig(
92+
schema_url="https://opentelemetry.io/schemas/1.24.0"
93+
)
94+
resource = create_resource(config)
95+
self.assertEqual(
96+
resource.schema_url, "https://opentelemetry.io/schemas/1.24.0"
97+
)
98+
99+
def test_schema_url_none(self):
100+
resource = create_resource(ResourceConfig())
101+
self.assertEqual(resource.schema_url, "")
102+
103+
104+
class TestCreateResourceAttributes(unittest.TestCase):
62105
def test_attributes_plain(self):
63106
config = ResourceConfig(
64107
attributes=[
@@ -87,9 +130,7 @@ def test_attribute_type_string(self):
87130
def test_attribute_type_int(self):
88131
config = ResourceConfig(
89132
attributes=[
90-
AttributeNameValue(
91-
name="k", value=3.0, type=AttributeType.int
92-
)
133+
AttributeNameValue(name="k", value=3.0, type=AttributeType.int)
93134
]
94135
)
95136
resource = create_resource(config)
@@ -182,19 +223,8 @@ def test_attribute_type_bool_array(self):
182223
resource = create_resource(config)
183224
self.assertEqual(list(resource.attributes["k"]), [True, False]) # type: ignore[arg-type]
184225

185-
def test_schema_url(self):
186-
config = ResourceConfig(
187-
schema_url="https://opentelemetry.io/schemas/1.24.0"
188-
)
189-
resource = create_resource(config)
190-
self.assertEqual(
191-
resource.schema_url, "https://opentelemetry.io/schemas/1.24.0"
192-
)
193-
194-
def test_schema_url_none(self):
195-
resource = create_resource(ResourceConfig())
196-
self.assertEqual(resource.schema_url, "")
197226

227+
class TestCreateResourceAttributesList(unittest.TestCase):
198228
def test_attributes_list_parsed(self):
199229
config = ResourceConfig(
200230
attributes_list="service.name=my-svc,region=us-east-1"
@@ -238,7 +268,9 @@ def test_attributes_list_url_decoded(self):
238268
attributes_list="service.namespace=my%20namespace,region=us-east-1"
239269
)
240270
resource = create_resource(config)
241-
self.assertEqual(resource.attributes["service.namespace"], "my namespace")
271+
self.assertEqual(
272+
resource.attributes["service.namespace"], "my namespace"
273+
)
242274

243275
def test_attributes_list_invalid_pair_skipped(self):
244276
with self.assertLogs(
@@ -249,34 +281,3 @@ def test_attributes_list_invalid_pair_skipped(self):
249281
self.assertEqual(resource.attributes["foo"], "bar")
250282
self.assertNotIn("no-equals", resource.attributes)
251283
self.assertTrue(any("no-equals" in msg for msg in cm.output))
252-
253-
def test_service_name_default_added_when_missing(self):
254-
config = ResourceConfig(
255-
attributes=[AttributeNameValue(name="env", value="staging")]
256-
)
257-
resource = create_resource(config)
258-
self.assertEqual(resource.attributes[SERVICE_NAME], "unknown_service")
259-
260-
def test_service_name_not_overridden_when_set(self):
261-
config = ResourceConfig(
262-
attributes=[
263-
AttributeNameValue(name="service.name", value="my-app")
264-
]
265-
)
266-
resource = create_resource(config)
267-
self.assertEqual(resource.attributes[SERVICE_NAME], "my-app")
268-
269-
def test_env_vars_not_read(self):
270-
"""OTEL_RESOURCE_ATTRIBUTES must not affect declarative config resource."""
271-
import os
272-
from unittest.mock import patch
273-
274-
with patch.dict(
275-
os.environ,
276-
{"OTEL_RESOURCE_ATTRIBUTES": "injected=true"},
277-
):
278-
config = ResourceConfig(
279-
attributes=[AttributeNameValue(name="k", value="v")]
280-
)
281-
resource = create_resource(config)
282-
self.assertNotIn("injected", resource.attributes)

0 commit comments

Comments
 (0)