Allow to generate more informative profile file name #638
Conversation
k4rl85
force-pushed
the
add_extended_profile_name
branch
2 times, most recently
from
bd410d2
to
1be0b85
Compare
Codecov Report
@@ Coverage Diff @@
## master #638 +/- ##
==========================================
+ Coverage 86.42% 86.51% +0.08%
==========================================
Files 52 52
Lines 2078 2091 +13
==========================================
+ Hits 1796 1809 +13
Misses 282 282
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@k4rl85 can you try rebasing this pull request to get the tests to pass? |
k4rl85
force-pushed
the
add_extended_profile_name
branch
from
1be0b85
to
98d034a
Compare
|
@albertyw rebased the pr |
silk/collector.py
Outdated
| """Retrieve the profile file name to be proposed to the storage""" | ||
|
|
||
| if SilkyConfig().SILKY_PYTHON_PROFILER_EXTENDED_FILE_NAME: | ||
| request_path = self.request.path.replace('/', '_').lstrip('_') |
There was a problem hiding this comment.
I'm concerned that there are some other characters that could be in the request path that would cause issues. For example, if .. was in the path, it might lead to path traversal. A \ might also cause problems on windows systems.
I think it would be safer to strip all characters not in [a-z0-9_\-]+ (and maybe a few more) and also to limit the path to N (50?) characters.
There was a problem hiding this comment.
@albertyw I changed the logic to remove any not ASCII char and replace with _ any char not contained in [a-z0-9_].
I took as limit for path name length 50.
…e informative profile file name
k4rl85
force-pushed
the
add_extended_profile_name
branch
2 times, most recently
from
3299ce5
to
5e60656
Compare
k4rl85
force-pushed
the
add_extended_profile_name
branch
from
5e60656
to
0dd5a0b
Compare
The aim of this pull request is to generate a more informative profile file name. This would allow us to identify the endpoint that generates a profile file without the need to open it.
To avoid breaking any existent use case I set this behavior behind the setting
SILKY_PYTHON_PROFILER_EXTENDED_FILE_NAMEthat by default is disabled.If you think we can change the generated filename without worry we can simply apply this change always without the need to introduce the new settings.