Skip to content

Commit 6d2e988

Browse files
authored
Test Common w/ multiple OTel versions & add compat with old OTel (#4312)
With the switch to OTel, the Common test suite is now dependent on an otel package, so it technically fits the toxgen usecase. By letting toxgen take care of it, we're making sure we're always testing a good range of otel versions, including the oldest one (to catch regressions) and the newest one (to catch incompatibilities early). Couple things surfaced in terms of incompatibility with older versions: - Some semantic attributes we're using weren't there from the get go open-telemetry/opentelemetry-python@495d705. Changed the code that uses them to handle failure. - The signature of `span.set_status()` changed at some point open-telemetry/opentelemetry-python@6e282d2. Added a compat version of `set_status()` for older otel. Also included: - removing the `opentelemetry-experimental` extra (not used anymore) - ❗ switching to using `opentelemetry-sdk` instead of `opentelemetry-distro` -- the `distro` only seems to [be setting up some defaults](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/8390db35ae2062c09d4d74a08d310c7bde1912c4/opentelemetry-distro/src/opentelemetry/distro/__init__.py) that we're not using Closes #3241
1 parent a7e24d9 commit 6d2e988

File tree

11 files changed

+133
-50
lines changed

11 files changed

+133
-50
lines changed

scripts/populate_tox/README.md

+27-8
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ then determining which versions make sense to test to get good coverage.
1818

1919
The lowest supported and latest version of a framework are always tested, with
2020
a number of releases in between:
21+
2122
- If the package has majors, we pick the highest version of each major. For the
2223
latest major, we also pick the lowest version in that major.
2324
- If the package doesn't have multiple majors, we pick two versions in between
@@ -35,7 +36,8 @@ the main package (framework, library) to test with; any additional test
3536
dependencies, optionally gated behind specific conditions; and optionally
3637
the Python versions to test on.
3738

38-
Constraints are defined using the format specified below. The following sections describe each key.
39+
Constraints are defined using the format specified below. The following sections
40+
describe each key.
3941

4042
```
4143
integration_name: {
@@ -46,6 +48,7 @@ integration_name: {
4648
},
4749
"python": python_version_specifier,
4850
"include": package_version_specifier,
51+
"test_on_all_python_versions": bool,
4952
}
5053
```
5154

@@ -68,11 +71,12 @@ The test dependencies of the test suite. They're defined as a dictionary of
6871
in the package list of a rule will be installed as long as the rule applies.
6972

7073
`rule`s are predefined. Each `rule` must be one of the following:
71-
- `*`: packages will be always installed
72-
- a version specifier on the main package (e.g. `<=0.32`): packages will only
73-
be installed if the main package falls into the version bounds specified
74-
- specific Python version(s) in the form `py3.8,py3.9`: packages will only be
75-
installed if the Python version matches one from the list
74+
75+
- `*`: packages will be always installed
76+
- a version specifier on the main package (e.g. `<=0.32`): packages will only
77+
be installed if the main package falls into the version bounds specified
78+
- specific Python version(s) in the form `py3.8,py3.9`: packages will only be
79+
installed if the Python version matches one from the list
7680

7781
Rules can be used to specify version bounds on older versions of the main
7882
package's dependencies, for example. If e.g. Flask tests generally need
@@ -101,6 +105,7 @@ Python versions, you can say:
101105
...
102106
}
103107
```
108+
104109
This key is optional.
105110

106111
### `python`
@@ -145,14 +150,26 @@ The `include` key can also be used to exclude a set of specific versions by usin
145150
`!=` version specifiers. For example, the Starlite restriction above could equivalently
146151
be expressed like so:
147152

148-
149153
```python
150154
"starlite": {
151155
"include": "!=2.0.0a1,!=2.0.0a2",
152156
...
153157
}
154158
```
155159

160+
### `test_on_all_python_versions`
161+
162+
By default, the script will cherry-pick a few Python versions to test each
163+
integration on. If you want a test suite to run on all supported Python versions
164+
instead, set `test_on_all_python_versions` to `True`.
165+
166+
```python
167+
"common": {
168+
# The common test suite should run on all Python versions
169+
"test_on_all_python_versions": True,
170+
...
171+
}
172+
```
156173

157174
## How-Tos
158175

@@ -176,7 +193,8 @@ A handful of integration test suites are still hardcoded. The goal is to migrate
176193
them all to `populate_tox.py` over time.
177194

178195
1. Remove the integration from the `IGNORE` list in `populate_tox.py`.
179-
2. Remove the hardcoded entries for the integration from the `envlist` and `deps` sections of `tox.jinja`.
196+
2. Remove the hardcoded entries for the integration from the `envlist` and `deps`
197+
sections of `tox.jinja`.
180198
3. Run `scripts/generate-test-files.sh`.
181199
4. Run the test suite, either locally or by creating a PR.
182200
5. Address any test failures that happen.
@@ -185,6 +203,7 @@ You might have to introduce additional version bounds on the dependencies of the
185203
package. Try to determine the source of the failure and address it.
186204

187205
Common scenarios:
206+
188207
- An old version of the tested package installs a dependency without defining
189208
an upper version bound on it. A new version of the dependency is installed that
190209
is incompatible with the package. In this case you need to determine which

scripts/populate_tox/config.py

+12
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@
2929
"clickhouse_driver": {
3030
"package": "clickhouse-driver",
3131
},
32+
"common": {
33+
"package": "opentelemetry-sdk",
34+
"test_on_all_python_versions": True,
35+
"deps": {
36+
"*": ["pytest", "pytest-asyncio"],
37+
# See https://github.com/pytest-dev/pytest/issues/9621
38+
# and https://github.com/pytest-dev/pytest-forked/issues/67
39+
# for justification of the upper bound on pytest
40+
"py3.7": ["pytest<7.0.0"],
41+
"py3.8": ["hypothesis"],
42+
},
43+
},
3244
"cohere": {
3345
"package": "cohere",
3446
"python": ">=3.9",

scripts/populate_tox/populate_tox.py

+20-11
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
"asgi",
6262
"aws_lambda",
6363
"cloud_resource_context",
64-
"common",
6564
"gevent",
6665
"opentelemetry",
6766
"potel",
@@ -348,22 +347,28 @@ def supported_python_versions(
348347
return supported
349348

350349

351-
def pick_python_versions_to_test(python_versions: list[Version]) -> list[Version]:
350+
def pick_python_versions_to_test(
351+
python_versions: list[Version], test_all: bool = False
352+
) -> list[Version]:
352353
"""
353354
Given a list of Python versions, pick those that make sense to test on.
354355
355356
Currently, this is the oldest, the newest, and the second newest Python
356357
version.
357358
"""
358-
filtered_python_versions = {
359-
python_versions[0],
360-
}
359+
if test_all:
360+
filtered_python_versions = python_versions
361361

362-
filtered_python_versions.add(python_versions[-1])
363-
try:
364-
filtered_python_versions.add(python_versions[-2])
365-
except IndexError:
366-
pass
362+
else:
363+
filtered_python_versions = {
364+
python_versions[0],
365+
}
366+
367+
filtered_python_versions.add(python_versions[-1])
368+
try:
369+
filtered_python_versions.add(python_versions[-2])
370+
except IndexError:
371+
pass
367372

368373
return sorted(filtered_python_versions)
369374

@@ -517,6 +522,9 @@ def _add_python_versions_to_release(
517522

518523
time.sleep(PYPI_COOLDOWN) # give PYPI some breathing room
519524

525+
test_on_all_python_versions = (
526+
TEST_SUITE_CONFIG[integration].get("test_on_all_python_versions") or False
527+
)
520528
target_python_versions = TEST_SUITE_CONFIG[integration].get("python")
521529
if target_python_versions:
522530
target_python_versions = SpecifierSet(target_python_versions)
@@ -525,7 +533,8 @@ def _add_python_versions_to_release(
525533
supported_python_versions(
526534
determine_python_versions(release_pypi_data),
527535
target_python_versions,
528-
)
536+
),
537+
test_all=test_on_all_python_versions,
529538
)
530539

531540
release.rendered_python_versions = _render_python_versions(release.python_versions)

scripts/populate_tox/tox.jinja

-12
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ requires =
1717
# This version introduced using pip 24.1 which does not work with older Celery and HTTPX versions.
1818
virtualenv<20.26.3
1919
envlist =
20-
# === Common ===
21-
{py3.7,py3.8,py3.9,py3.10,py3.11,py3.12,py3.13}-common
22-
2320
# === Gevent ===
2421
{py3.8,py3.10,py3.11,py3.12}-gevent
2522

@@ -157,15 +154,6 @@ deps =
157154
linters: -r requirements-linting.txt
158155
linters: werkzeug<2.3.0
159156
160-
# === Common ===
161-
py3.8-common: hypothesis
162-
common: pytest-asyncio
163-
# See https://github.com/pytest-dev/pytest/issues/9621
164-
# and https://github.com/pytest-dev/pytest-forked/issues/67
165-
# for justification of the upper bound on pytest
166-
py3.7-common: pytest<7.0.0
167-
{py3.8,py3.9,py3.10,py3.11,py3.12,py3.13}-common: pytest
168-
169157
# === Gevent ===
170158
{py3.7,py3.8,py3.9,py3.10,py3.11}-gevent: gevent>=22.10.0, <22.11.0
171159
{py3.12}-gevent: gevent

sentry_sdk/integrations/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ def iter_default_integrations(with_auto_enabling_integrations):
131131
"celery": (4, 4, 7),
132132
"chalice": (1, 16, 0),
133133
"clickhouse_driver": (0, 2, 0),
134+
"common": (1, 4, 0), # opentelemetry-sdk
134135
"cohere": (5, 4, 0),
135136
"django": (2, 0),
136137
"dramatiq": (1, 9),

sentry_sdk/opentelemetry/utils.py

+7-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,13 @@ def infer_status_from_attributes(span_attributes):
282282

283283
def get_http_status_code(span_attributes):
284284
# type: (Mapping[str, str | bool | int | float | Sequence[str] | Sequence[bool] | Sequence[int] | Sequence[float]]) -> Optional[int]
285-
http_status = span_attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE)
285+
try:
286+
http_status = span_attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE)
287+
except AttributeError:
288+
# HTTP_RESPONSE_STATUS_CODE was added in 1.21, so if we're on an older
289+
# OTel version SpanAttributes.HTTP_RESPONSE_STATUS_CODE will throw an
290+
# AttributeError
291+
http_status = None
286292

287293
if http_status is None:
288294
# Fall back to the deprecated attribute

sentry_sdk/tracing.py

+9-2
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
get_current_span,
1212
INVALID_SPAN,
1313
)
14-
from opentelemetry.trace.status import StatusCode
14+
from opentelemetry.trace.status import Status, StatusCode
1515
from opentelemetry.sdk.trace import ReadableSpan
16+
from opentelemetry.version import __version__ as otel_version
1617

1718
import sentry_sdk
1819
from sentry_sdk.consts import (
@@ -41,6 +42,7 @@
4142
from sentry_sdk.utils import (
4243
_serialize_span_attribute,
4344
get_current_thread_meta,
45+
parse_version,
4446
should_be_treated_as_error,
4547
)
4648

@@ -70,6 +72,8 @@
7072
from sentry_sdk.tracing_utils import Baggage
7173

7274

75+
_OTEL_VERSION = parse_version(otel_version)
76+
7377
tracer = otel_trace.get_tracer(__name__)
7478

7579

@@ -531,7 +535,10 @@ def set_status(self, status):
531535
otel_status = StatusCode.ERROR
532536
otel_description = status
533537

534-
self._otel_span.set_status(otel_status, otel_description)
538+
if _OTEL_VERSION is None or _OTEL_VERSION >= (1, 12, 0):
539+
self._otel_span.set_status(otel_status, otel_description)
540+
else:
541+
self._otel_span.set_status(Status(otel_status, otel_description))
535542

536543
def set_measurement(self, name, value, unit=""):
537544
# type: (str, float, MeasurementUnit) -> None

setup.py

+1-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def get_file_text(file_name):
4141
install_requires=[
4242
"urllib3>=1.26.11",
4343
"certifi",
44-
"opentelemetry-distro>=0.35b0", # XXX check lower bound
44+
"opentelemetry-sdk>=1.4.0",
4545
],
4646
extras_require={
4747
"aiohttp": ["aiohttp>=3.5"],
@@ -70,7 +70,6 @@ def get_file_text(file_name):
7070
"openai": ["openai>=1.0.0", "tiktoken>=0.3.0"],
7171
"openfeature": ["openfeature-sdk>=0.7.1"],
7272
"opentelemetry": ["opentelemetry-distro>=0.35b0"],
73-
"opentelemetry-experimental": ["opentelemetry-distro"],
7473
"pure-eval": ["pure_eval", "executing", "asttokens"],
7574
"pymongo": ["pymongo>=3.1"],
7675
"pyspark": ["pyspark>=2.4.4"],

tests/integrations/threading/test_threading.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
from concurrent import futures
33
from textwrap import dedent
44
from threading import Thread
5+
import sys
56

67
import pytest
78

89
import sentry_sdk
910
from sentry_sdk import capture_message
1011
from sentry_sdk.integrations.threading import ThreadingIntegration
12+
from sentry_sdk.tracing import _OTEL_VERSION
1113

1214
original_start = Thread.start
1315
original_run = Thread.run
@@ -104,13 +106,18 @@ def double(number):
104106
assert len(event["spans"]) == 0
105107

106108

109+
@pytest.mark.skipif(
110+
sys.version[:3] == "3.8" and (1, 12) <= _OTEL_VERSION < (1, 16),
111+
reason="Fails in CI on 3.8 and specific OTel versions",
112+
)
107113
def test_circular_references(sentry_init, request):
108114
sentry_init(default_integrations=False, integrations=[ThreadingIntegration()])
109115

110-
gc.collect()
111116
gc.disable()
112117
request.addfinalizer(gc.enable)
113118

119+
gc.collect()
120+
114121
class MyThread(Thread):
115122
def run(self):
116123
pass

0 commit comments

Comments
 (0)