Specify process.runtime for Python. #1160
Conversation
| Note that on Linux, there is an actual newline in the `sys.version` string, | ||
| and the CPython string had a trailing space in the first line. | ||
|
|
||
| Pypy provided a CPython-compatible version in `sys.implementation.version` instead of the actual implementation version which is available in `sys.version`. |
There was a problem hiding this comment.
This is a bit sad. We could another special case for PyPy to the runtime.version determination to use
>>>> sys.pypy_version_info
(major=7, minor=3, micro=2, releaselevel='alpha', serial=0)
There was a problem hiding this comment.
Sounds reasonable since PyPy is not conforming to the docs here
There was a problem hiding this comment.
I guess it is too late to change this in this PR now, unless there is a strong opinion. @open-telemetry/python-approvers feel free to suggest a follow-up spec PR if this comes up again when implementing.
| Fill in the [`sys.implementation.version`][py_impl] values separated by dots. | ||
| Fall back to using [`sys.version_info`](https://docs.python.org/3/library/sys.html#sys.version_info) if that is not available. |
There was a problem hiding this comment.
I think we may need an additional field for the version of the language (which defines the syntax and standard library API) after reading the docs for sys.implementation:
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, whichsys.version_inforepresents. For example, for PyPy 1.8sys.implementation.versionmight besys.version_info(1, 8, 0, 'final', 0), whereassys.version_infowould besys.version_info(2, 7, 2, 'final', 0). For CPython they are the same value, since it is the reference implementation.
For CPython they are the same but for PyPy they are independent.
There was a problem hiding this comment.
For PyPy they are also the same, unfortunately (see note below). We could add additional fields for that, true, maybe python.runtime.lang_version. But I did not want to add new fields in this PR.
There was a problem hiding this comment.
Ya, that is funny considering the quote from the python docs no longer holds true for PyPy 😕 . But could this effect other languages and would be worth adding process.runtime.language_version or similar?
There was a problem hiding this comment.
I think that would be useful, but I think it is out of scope for this PR.
The new attribute might better be Python-specific though. For example, I don't think it makes much sense for JVMs or .NET runtimes which support multiple languages.
| Note that on Linux, there is an actual newline in the `sys.version` string, | ||
| and the CPython string had a trailing space in the first line. | ||
|
|
||
| Pypy provided a CPython-compatible version in `sys.implementation.version` instead of the actual implementation version which is available in `sys.version`. |
There was a problem hiding this comment.
Sounds reasonable since PyPy is not conforming to the docs here
| result = ".".join(map( | ||
| str, | ||
| vinfo[:3] | ||
| if vinfo.releaselevel == "final" and not vinfo.serial | ||
| else vinfo | ||
| )) |
There was a problem hiding this comment.
This should be OK now. Example result might be
In [1]: from packaging import version
In [2]: version.parse('3.9.1.alpha.3')
Out[2]: <Version('3.9.1a3')>so it is still a correct version.
|
@codeboten Any opinion/approval? |
Fixes open-telemetry/opentelemetry-python#1127
Changes
Specifies
process.runtime.*attributes for Python.CC @open-telemetry/python-approvers