Add typing information for contexts, token

[mrnicegyu11]

Description

Trying to get #4164 in.

Opening this 1:1 similar PR for now, with changes from upstream/main, to see how the tests look. Locally, all tests are green.

This is WIP for now.

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?

Ran the tox full test suite locally, all green. Waiting for maintainer approval for the github actions tests

Does This PR Require a Contrib Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

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

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: hyoinandout (1dadd33, 8c6b4c6)
• ✅ login: mrnicegyu11 / name: Dustin Kaiser (311e7ad, 6347121, bbe9769, ead2776, bb9da02, d078548)

[mrnicegyu11]

Dear maintainers, on the original PR #4164 I cannot see the details of the github action test failures, to me the code contribution by @hyoinandout looks good and mergeable. If there are incompatabilities with the python-contrib-instrumentations I am willing to look into it, but for this I'd need to see the github-action-results/failures ;)

[Kludex]

Why is the type: ignore needed?

Suggested change

	context.detach("some garbage") # type:ignore
	context.detach("some garbage") # type: ignore[?]

[mrnicegyu11]

True thanks this was actually not necessary, slipped past me. I removed the type:ignore

[Kludex]

The opentelemetry/context directory needs to be included in the mypy setup.

---

Not related to this PR, but I already pointed out that this setup doesn't make sense. mypy needs to run once, and it should have the context of the local imports.

I've shared my thoughts here: open-telemetry/opentelemetry-python-contrib#3116 (comment)

I'm happy to push a PR switching the type checker used here, and I'm happy to help this project to have proper type hints. Otherwise, I think someone should try to make the mypy setup work properly.

I'll bring the above to the next Python SIG meeting.

[mrnicegyu11]

The opentelemetry/context directory needs to be included in the mypy setup.

Not related to this PR, but I already pointed out that this setup doesn't make sense. mypy needs to run once, and it should have the context of the local imports.

I've shared my thoughts here: open-telemetry/opentelemetry-python-contrib#3116 (comment)

I'm happy to push a PR switching the type checker used here, and I'm happy to help this project to have proper type hints. Otherwise, I think someone should try to make the mypy setup work properly.

I'll bring the above to the next Python SIG meeting.

Thanks for the review, as some other people before me I am implementing opentelemetry in code I am working on, and I have type-checking for this code setup which accounts for dependencies. There was a bug (on my side, wrongly using open-telemetry) that could have been avoided or would have been caught by opentelemetry-python providing the correct types. For this reason I have decided to PR this small atomic change (again). This is just to spare some pain for successors, at this point I was not planning to involve myself in overhauling the whole typechecking in this opentelemtry-python-repo 😅 @Kludex

From where I am sitting, merging this small PR would bring immediate improvements for everyone that runs a type-checker on their code and imports from opentelemetry-python. I am not sure what the procedure is, who to talk to and such, to get it in. @hyoinandout actually already opened a PR with these exact changes ( #4164 ), but I guess they could not get it merged.

Any advice or support would be welcome from my side, as I am fairly sure other people will run into issues that could be prevented by proper typing here :--)

[mrnicegyu11]

Or, well, if any maintainer feels that realistically this PR wont be merged, then please let me know as well, to spare some work&time ;)

[Kludex]

The opentelemetry/context directory needs to be included in the mypy setup.

[mrnicegyu11]

Hey @Kludex and others , I hope I am not stealing too much of your time thanks for the consideration :--)

I read up on your pyright vs. mypy issue mentioned, and checked my PR here for this and your comment to include the opentelemetry/context folder in mypy.

From what I can see:

• The mypy that can be run in this repo via tox -e mypy does include the files I have changed in this PR, and specifically checks the files in opentelemetry-api/src/opentelemetry/context/*. So the types of these modified files are being correctly checked, as per the current setup at least.
• There was another typing bug in my PR (:innocent:), which is now detected and fixed (the previous context.detach(string) has wrong types, one can only detach tokens)
• I saw that your recently changed the mypy.ini so I am not sure if you might have adressed your previous concerns before?

Summarizing, w.r.t. The opentelemetry/context directory needs to be included in the mypy setup., it seems that this is the case in this repo (unless I am still not fully following, maybe you are referring to other related repositories of opentelemetry?). The tox suite tests run through 100% green locally for me, for this PR. Would you reconsider this typing contribution with this info?

Thanks a lot, Cheers!

[Kludex]

LGTM!

[Kludex]

Sorry for being annoying. This looks great. Thanks! :)