ISSUE-3317: Remove maximum version constraint, increase minimum version on starlette by fixing unit tests #3456

MattiasDC · 2025-04-29T20:05:16Z

Description

Remove maximum version constraint on starlette, by fixing unit tests. Fixed some warnings in unit tests.

Problem was that the environment was patched only during the setUp execution, and not during the tests themselves. By putting the patch decorator on the class itself, the environment variables are patched during the test class, as is done in another place in the same file.

This PR also includes some small changes to reduce the amount of warnings encountered in the unit tests.

Get rid of deprecated calls to the route and route_websocket decorators
As StarletteInstrumentor._uninstrument can clear all instrumented apps, use a discard i.s.o. remove in the deletor of _InstrumentedStarlette to prevent a KeyError being raised

Fixes #3317

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

All existing unit tests pass with the newest version of starlette (0.46.2)

Does This PR Require a Core Repo Change?

Yes. - Link to PR:
No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

Followed the style guidelines of this project
Changelogs have been updated
Unit tests have been added
Documentation has been updated

… unit tests. Fixed some warnings in unit tests.

linux-foundation-easycla · 2025-04-29T20:05:19Z

The committers listed above are authorized under a signed CLA.

✅ login: MattiasDC / name: Mattias De Charleroy (695c345, b6ac320, 8135dc5, 4d4e2de, 82d860b, 92ef8a0, 08df2be, b473e8e)

MattiasDC · 2025-04-29T20:11:42Z

@Kludex you mentioned you were interested in doing a review for #3317. If this is a bad time, would it be possible for you appoint someone else?

MattiasDC · 2025-04-29T20:28:18Z

@aabmass could you review as well and are you able to upgrade the requirements? I'm not comfortable enough to update the lockfiles.

emdneto · 2025-04-29T20:36:34Z

instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py



+@patch.dict(


How about using this in setUp?

test_env_patch = mock.patch.dict("os.environ", {content here})
test_env_patch.start()

Done, stopped the patches in tearDown

emdneto · 2025-04-29T20:36:52Z

...entelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/package.py

@@ -13,6 +13,6 @@
 # limitations under the License.


-_instruments = ("starlette >= 0.13, <0.15",)
+_instruments = ("starlette >= 0.13",)


Need to run tox -e generate

Should be ok now

emdneto · 2025-04-29T20:37:14Z

instrumentation/opentelemetry-instrumentation-starlette/pyproject.toml

@@ -34,7 +34,7 @@ dependencies = [
 ]

 [project.optional-dependencies]
-instruments = ["starlette >= 0.13, <0.15"]
+instruments = ["starlette >= 0.13"]


I think we need to update the requirements.latest.txt as well.

Updated test-requirements.latest.txt and uv.lock

Also updated test-requirements.oldest.txt as I had to increase minimum version

…trap/test-requirements. Use setUp/tearDown iso decorator on test class

Kludex · 2025-05-02T15:03:17Z

We should test the min version.

MattiasDC · 2025-05-02T15:11:30Z

We should test the min version.

Hi @Kludex thanks for joining,

Do you mean something else than this test:
'instrumentation-starlette-oldest 3.8 Ubuntu', which uses

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-starlette/test-requirements.oldest.txt

Line 75 in e6869cd

starlette==0.13.8

Or am I missing something?

…ldest' tests.

MattiasDC · 2025-05-02T15:51:54Z

@Kludex those oldest tests were indeed failing. I've increased the minimal version to 0.37.2. This one is a year old and didn't cause any problems with me. Is this acceptable? Older versions were causing this issue for me encode/starlette#2770

This reverts commit 08df2be.

…time keyword

MattiasDC · 2025-05-04T15:19:59Z

PR is ready to be reviewed again @emdneto, if anything needs a change, feel free to put me to work again ;)

xrmx · 2025-05-05T14:18:48Z

CHANGELOG.md

+  ([#3448](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3448
+- `opentelemetry-instrumentation-starlette` Remove max version constraint on starlete, increase min version ([#3456](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3456))


Suggested change

([#3448](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3448

- `opentelemetry-instrumentation-starlette` Remove max version constraint on starlete, increase min version ([#3456](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3456))

([#3448](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3448))

- `opentelemetry-instrumentation-starlette` Bump starlette supported version to 0.37.2

([#3456](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3456))

Is there a reason we picked 0.37.2?

xrmx · 2025-05-05T14:20:59Z

instrumentation/opentelemetry-instrumentation-starlette/test-requirements.oldest.txt

    # via requests
-wrapt==1.16.0
+wrapt==1.10.0


This is strange

It was supposed to not work on this version and raise an error like:
ImportError: cannot import name 'getargspec' from 'inspect'

It is failing with that error (as was expected?). I got these new versions of oldest.txt and latest.txt by using the 'uv pip compile' commands in the respective files. Should these commands be updated, or should I update the '.in' file?

@emdneto did you manually change the version of wrapt in oldest.txt, because for me uv pip compile (without-ooption) resolves to 1.10.0

@MattiasDC could you please change the ".in" file to

# Used to generate test-requirements.{oldest,latest}.txt with uv pip compile pytest~=7.4.4 requests~=2.32 httpx~=0.28 wrapt~=1.14.0 -e opentelemetry-instrumentation -e instrumentation/opentelemetry-instrumentation-asgi -e util/opentelemetry-util-http -e instrumentation/opentelemetry-instrumentation-starlette[instruments]

and run uv pip compile command again?

xrmx · 2025-05-05T14:21:38Z

instrumentation/opentelemetry-instrumentation-starlette/test-requirements.oldest.txt

+    # via
+    #   httpx
+    #   starlette
+asgiref==3.0.0


Why are you downgrading this?

All changes to oldest.txt and latest.txt were obtained by running the 'uv pip compile' commands which were listed on top of the respective files. Should I manually adjust them or should the pip compile command be changed?

What command did you run?

using uv 0.6.14:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-starlette/test-requirements.oldest.txt

Line 2 in 82d860b

# uv pip compile --python 3.8 --universal --resolution lowest-direct --no-emit-package opentelemetry-api --no-emit-package opentelemetry-semantic-conventions -c dev-requirements.txt instrumentation/opentelemetry-instrumentation-starlette/test-requirements.in -o instrumentation/opentelemetry-instrumentation-starlette/test-requirements.oldest.txt

Note that if a package version in oldest.txt was manually increased, uv pip compile will leave it higher than it's own resolution.. You can check this if you omit the -o option

You can for example see that here, 3.0 is the minimum allowed:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-asgi/pyproject.toml

Line 29 in 82d860b

"asgiref ~= 3.0",

@aabmass, do you remember manually changing things after running the uv pip compile ?

xrmx · 2025-05-05T14:24:07Z

@Kludex those oldest tests were indeed failing. I've increased the minimal version to 0.37.2. This one is a year old and didn't cause any problems with me. Is this acceptable? Older versions were causing this issue for me encode/starlette#2770

That maybe be fixed by using an older httpx in the requirements.

MattiasDC · 2025-05-06T17:55:09Z

@Kludex those oldest tests were indeed failing. I've increased the minimal version to 0.37.2. This one is a year old and didn't cause any problems with me. Is this acceptable? Older versions were causing this issue for me encode/starlette#2770

That maybe be fixed by using an older httpx in the requirements.

Conditional dependencies are not supported in requirements.in files. Do you propose to add a .in file for latest and one for oldest?

ISSUE-3317: Remove maximum version constraint on starlette, by fixing…

695c345

… unit tests. Fixed some warnings in unit tests.

Updae changelog

8135dc5

MattiasDC requested a review from a team as a code owner April 29, 2025 20:06

emdneto reviewed Apr 29, 2025

View reviewed changes

MattiasDC force-pushed the issue/3317 branch from 8abc437 to 6f3633a Compare May 2, 2025 14:45

Implement code review remarks: update starlette dependencies in boots…

b6ac320

…trap/test-requirements. Use setUp/tearDown iso decorator on test class

MattiasDC force-pushed the issue/3317 branch from 6f3633a to b6ac320 Compare May 2, 2025 14:46

Increase minimal version of starlette to 0.37.2 to have functional 'o…

4d4e2de

…ldest' tests.

MattiasDC force-pushed the issue/3317 branch from bf196d1 to 4d4e2de Compare May 2, 2025 15:47

Merge branch 'main' into issue/3317

b473e8e

add uv.lock

08df2be

MattiasDC changed the title ~~ISSUE-3317: Remove maximum version constraint on starlette, by fixing unit tests~~ ISSUE-3317: Remove maximum version constraint, increase minimum version on starlette by fixing unit tests May 2, 2025

MattiasDC added 2 commits May 4, 2025 16:58

Revert "add uv.lock"

92ef8a0

This reverts commit 08df2be.

Prevent massive changes of uv.lock due to newer uv which adds upload-…

82d860b

…time keyword

xrmx reviewed May 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE-3317: Remove maximum version constraint, increase minimum version on starlette by fixing unit tests #3456

ISSUE-3317: Remove maximum version constraint, increase minimum version on starlette by fixing unit tests #3456

MattiasDC commented Apr 29, 2025 •

edited by xrmx

Loading

linux-foundation-easycla bot commented Apr 29, 2025 •

edited

Loading

MattiasDC commented Apr 29, 2025

MattiasDC commented Apr 29, 2025

emdneto Apr 29, 2025

MattiasDC May 2, 2025

emdneto Apr 29, 2025

MattiasDC May 2, 2025

emdneto Apr 29, 2025

MattiasDC May 2, 2025

MattiasDC May 4, 2025

Kludex commented May 2, 2025 •

edited

Loading

MattiasDC commented May 2, 2025 •

edited

Loading

MattiasDC commented May 2, 2025

MattiasDC commented May 4, 2025

xrmx May 5, 2025

xrmx May 5, 2025

emdneto May 5, 2025

MattiasDC May 5, 2025

MattiasDC May 6, 2025

emdneto May 6, 2025

xrmx May 5, 2025

MattiasDC May 5, 2025 •

edited

Loading

emdneto May 5, 2025

MattiasDC May 6, 2025 •

edited

Loading

MattiasDC May 6, 2025

MattiasDC May 6, 2025

emdneto May 6, 2025

xrmx commented May 5, 2025

MattiasDC commented May 6, 2025 •

edited

Loading

		([#3448](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3448
		- `opentelemetry-instrumentation-starlette` Remove max version constraint on starlete, increase min version ([#3456](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3456))



		@patch.dict(

ISSUE-3317: Remove maximum version constraint, increase minimum version on starlette by fixing unit tests #3456

Are you sure you want to change the base?

ISSUE-3317: Remove maximum version constraint, increase minimum version on starlette by fixing unit tests #3456

Conversation

MattiasDC commented Apr 29, 2025 • edited by xrmx Loading

Description

Type of change

How Has This Been Tested?

Does This PR Require a Core Repo Change?

Checklist:

linux-foundation-easycla bot commented Apr 29, 2025 • edited Loading

MattiasDC commented Apr 29, 2025

MattiasDC commented Apr 29, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kludex commented May 2, 2025 • edited Loading

MattiasDC commented May 2, 2025 • edited Loading

MattiasDC commented May 2, 2025

MattiasDC commented May 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MattiasDC May 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MattiasDC May 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xrmx commented May 5, 2025

MattiasDC commented May 6, 2025 • edited Loading

MattiasDC commented Apr 29, 2025 •

edited by xrmx

Loading

linux-foundation-easycla bot commented Apr 29, 2025 •

edited

Loading

Kludex commented May 2, 2025 •

edited

Loading

MattiasDC commented May 2, 2025 •

edited

Loading

MattiasDC May 5, 2025 •

edited

Loading

MattiasDC May 6, 2025 •

edited

Loading

MattiasDC commented May 6, 2025 •

edited

Loading