Skip to content

fix(sdk): use sys.orig_argv for process.command to handle python -m invocations#5083

Open
alliasgher wants to merge 6 commits intoopen-telemetry:mainfrom
alliasgher:fix-process-command-python-m
Open

fix(sdk): use sys.orig_argv for process.command to handle python -m invocations#5083
alliasgher wants to merge 6 commits intoopen-telemetry:mainfrom
alliasgher:fix-process-command-python-m

Conversation

@alliasgher
Copy link
Copy Markdown

Description

ProcessResourceDetector populates process.command, process.command_line, and process.command_args from sys.argv. For applications launched via python -m <module>, the interpreter rewrites sys.argv[0] to the resolved __main__.py path — so the ``-m `` portion of the original invocation is lost and the emitted telemetry is misleading (e.g. process.command = "-m" or process.command = "<path>/__main__.py" depending on when the detector runs).

Python 3.10+ exposes sys.orig_argv which preserves the arguments received by the interpreter. Since opentelemetry-sdk already requires Python ≥ 3.10, this PR switches the detector to read from sys.orig_argv (with a getattr fallback for safety). This also aligns with the OTel semantic conventions that reference /proc/<pid>/cmdline for these attributes.

Fixes #4518

Behaviour change

invocation before after
python -m myapp process.command = "-m" (or "<path>/__main__.py") process.command = "<python>", command_line = "<python> -m myapp"
python myscript.py process.command = "myscript.py" process.command = "<python>", command_line = "<python> myscript.py"

Users inspecting process.command_line / process.command_args will see more accurate data. process.command changes to match /proc/<pid>/cmdline[0], which is what the OTel semantic conventions reference.

Checklist

  • pytest opentelemetry-sdk/tests/resources/test_resources.py
  • New test test_process_detector_uses_orig_argv_for_python_m covers the reported scenario
  • CHANGELOG entry added

Signed-off-by: Ali alliasgher123@gmail.com

Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @alliasgher. I've left a couple of small suggestions, with the main one being now the SDK requires python 3.10+, we don't need the fallback anymore.

Comment thread CHANGELOG.md Outdated
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated
@github-project-automation github-project-automation bot moved this to Reviewed PRs that need fixes in Python PR digest Apr 13, 2026
Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Got a little more clean-up needed to remove sys.argv but otherwise looks good.

Comment thread opentelemetry-sdk/tests/resources/test_resources.py Outdated
Comment thread CHANGELOG.md
@MikeGoldsmith MikeGoldsmith moved this from Reviewed PRs that need fixes to Approved PRs in Python PR digest Apr 14, 2026
@alliasgher alliasgher force-pushed the fix-process-command-python-m branch from 6db2e8a to 0fea141 Compare April 14, 2026 09:18
Comment thread opentelemetry-sdk/tests/resources/test_resources.py Outdated
Comment thread opentelemetry-sdk/tests/resources/test_resources.py Outdated
@alliasgher alliasgher force-pushed the fix-process-command-python-m branch from 0fea141 to 72470e6 Compare April 14, 2026 20:35
@xrmx xrmx moved this from Approved PRs to Approved PRs that need fixes in Python PR digest Apr 15, 2026
alliasgher added a commit to alliasgher/opentelemetry-python that referenced this pull request Apr 15, 2026
Since ProcessResourceDetector now prefers sys.orig_argv (Python 3.10+),
the @patch('sys.argv', ...) decorator on test_process_detector is
redundant — the detector reads from sys.orig_argv which is already
patched. Remove it as suggested in review.

Addresses xrmx's review comment on open-telemetry#5083.

Signed-off-by: Ali <alliasgher123@gmail.com>
@alliasgher alliasgher requested a review from xrmx April 15, 2026 12:32
…nvocations

ProcessResourceDetector populated process.command, process.command_line,
and process.command_args from sys.argv. For applications launched via
`python -m <module>`, the interpreter rewrites sys.argv[0] to the
resolved module path, so the ``-m <module>`` portion of the original
invocation is lost and the detector emits misleading telemetry.

Python 3.10+ exposes sys.orig_argv which preserves the original
arguments received by the interpreter. Since the SDK already requires
Python >= 3.10, switch to sys.orig_argv (with a getattr fallback for
safety). This also aligns with the OTel semantic conventions that
reference /proc/<pid>/cmdline for these attributes.

Fixes open-telemetry#4518

Signed-off-by: Ali <alliasgher123@gmail.com>
Two review nits from MikeGoldsmith:

- sys.orig_argv has been available since Python 3.10; the SDK now
  requires 3.10+ so the getattr fallback is dead code. Use sys.orig_argv
  directly and update the comment.

- CHANGELOG entries should reference the PR number, not the issue
  number.

Signed-off-by: Ali <alliasgher123@gmail.com>
@alliasgher alliasgher force-pushed the fix-process-command-python-m branch from b110aa0 to d110a9c Compare April 15, 2026 16:42
Signed-off-by: Ali <alliasgher123@gmail.com>
alliasgher and others added 3 commits April 17, 2026 16:39
The detector reads only sys.orig_argv, so patching sys.argv had no
effect on the test's outcome. Keep only the sys.orig_argv patch.

Signed-off-by: Ali <alliasgher123@gmail.com>
Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Looks good to me now, thanks @alliasgher.

@github-project-automation github-project-automation bot moved this from Approved PRs that need fixes to Approved PRs in Python PR digest Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved PRs

Development

Successfully merging this pull request may close these issues.

Inaccurate process.command when using python -m <module>

3 participants