Skip to content
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

Update process.runtime example for the Python language #1772

Merged
merged 5 commits into from
Jan 24, 2025

Conversation

Viicos
Copy link
Contributor

@Viicos Viicos commented Jan 20, 2025

Changes

The Python runtimes specification says:

process.runtime.version- Fill in the sys.implementation.version values separated by dots.

The PyPy example currently uses version 7.3.2 (4 years old). In this version, it seems that PyPy did not implement sys.implementation.version correctly:

Python 3.7.4 (87875bf2dfd8, Sep 24 2020, 07:26:36)
[PyPy 7.3.2-alpha0 with GCC 7.3.1 20180303 (Red Hat 7.3.1-5)] on linux
>>>> sys.implementation.version
(major=3, minor=7, micro=4, releaselevel='final', serial=0)

sys.implementation.version documentation:

version is a named tuple, in the same format as sys.version_info. It represents the version of the Python implementation. This has a distinct meaning from the specific version of the Python language to which the currently running interpreter conforms, which sys.version_info represents. For example, for PyPy 1.8 sys.implementation.version might be sys.version_info(1, 8, 0, 'final', 0), whereas sys.version_info would be sys.version_info(2, 7, 2, 'final', 0). For CPython they are the same value, since it is the reference implementation.

In the latest PyPy release (7.3.17), this was fixed:

Python 3.10.14 (39dc8d3c85a7, Aug 27 2024, 14:32:27)
[PyPy 7.3.17 with GCC 10.2.1 20210130 (Red Hat 10.2.1-11)] on linux
>>>> sys.implementation.version
sys.pypy_version_info(major=7, minor=3, micro=17, releaselevel='final', serial=0)

So the example was fixed accordingly. However, Is this expected? Should sys.version_info be used instead?

Merge requirement checklist

@Viicos Viicos requested review from a team as code owners January 20, 2025 10:44
Copy link

linux-foundation-easycla bot commented Jan 20, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@mx-psi
Copy link
Member

mx-psi commented Jan 20, 2025

Thanks for the PR!

So the example was fixed accordingly. However, Is this expected? Should sys.version_info be used instead?

What would be the benefit of using sys.version_info?

@Viicos
Copy link
Contributor Author

Viicos commented Jan 20, 2025

What would be the benefit of using sys.version_info?

There's no real benefit in using sys.version_info vs sys.implementation.version, it's just that they don't provide the same information. sys.version_info provides the Python implementation version, while sys.implementation.version provides the version of the current implementation.

I believe it's still best to keep sys.implementation.version, as coupled with process.runtime.name, we know exactly which PyPy version was used and getting the Python implementation version used by this PyPy version is just a matter of looking up online at https://downloads.python.org/pypy/. If we were to use sys.version_info, we would be missing the exact PyPy version (e.g. PyPy 7.3.16 and 7.3.17 both use Python 3.10.14).

Just wanted to double check that we still want to stick to sys.implementation.version

@Viicos Viicos requested a review from a team as a code owner January 20, 2025 13:48
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's still best to keep sys.implementation.version, as coupled with process.runtime.name, we know exactly which PyPy version was used and getting the Python implementation version used by this PyPy version is just a matter of looking up online at downloads.python.org/pypy. If we were to use sys.version_info, we would be missing the exact PyPy version (e.g. PyPy 7.3.16 and 7.3.17 both use Python 3.10.14).

Just wanted to double check that we still want to stick to sys.implementation.version

This makes sense to me. LGTM

@trask
Copy link
Member

trask commented Jan 20, 2025

cc @open-telemetry/python-approvers

@Viicos Viicos changed the title Update PyPy process.runtime example Update process.runtime example for the Python language Jan 21, 2025
docs/resource/process.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added the enhancement New feature or request label Jan 24, 2025
@trask trask merged commit 91bb59c into open-telemetry:main Jan 24, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants