Remove current path from the module path in ProfileEncoder. #35
Conversation
Running the uwsgi sample application with Python 3.8.10-3.9.2 showed incorrect module path in the UI. This was narrowed down to traceback returning different results (a path with "/."). This change makes sure the path is not leaked out.
PapaPedro
left a comment
PapaPedro
left a comment
There was a problem hiding this comment.
Looks good to me, just a few comments/questions
| def _remove_prefix_path(module_path, sys_paths): | ||
| current_path = str(Path().absolute()) | ||
| if current_path in module_path: | ||
| return module_path.replace(current_path, "").replace("/.", "") |
There was a problem hiding this comment.
Should we not replace "/./" by "/" ?
There was a problem hiding this comment.
also that would not work in windows because the "/" would have been replaced already right? Are we sure we cannot have that problem in windows?
There was a problem hiding this comment.
Sounds better to replace /./, I'll update. I didn't test anything on windows. The existing unit tests with Windows passed, but I didn't add anything new.
| # When running the sample app with uwsgi for Python 3.8.10-3.9.2, the traceback command | ||
| # returns a file path that contains "/.". | ||
| # To not let the path go into the module name, we are removing it later in the ProfileEncoder. | ||
| # Examples: | ||
| # - file '/Users/mirelap/Documents/workspace/JSON/aws-codeguru-profiler-python-demo-application/sample-demo-django-app/./polls/views.py', line 104, code get_queryset>, 104 | ||
| # - file '/Users/mirelap/Documents/workspace/JSON/aws-codeguru-profiler-python-demo-application/sample-demo-django-app/polls/views.py', line 104, code get_queryset>, 104 |
There was a problem hiding this comment.
this comment seems to be very specific about the /./ problem but is not next to the code that actually fixes this issue, should we not simply add a comment here saying only that the full path will be removed from frame names later in the encoder and only talk about the /./ issue when we do the replace about it?
There was a problem hiding this comment.
Okay, I'll update.
2 participants
Running the uwsgi sample application with Python 3.8.10-3.9.2 showed incorrect module path in the UI.
This was narrowed down to
tracebackreturning different results (a path with "/.").This change makes sure the path is not leaked out.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Issue #, if available:
Description of changes:
Testing:
With this change, this is the logged module after removing the prefix :
INFO /polls/views.py--
uwsgi:/Users/mirelap/Documents/workspace/JSON/aws-codeguru-profiler-python-demo-application/sample-demo-django-app//polls/viewspy--
runserver:/polls/views.pyBefore this change when running this sample app with uwsgi command (not with the local runserver) starting Python 3.8.10:
.
After this change: