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

FEATURE: Allow skip_paths config to contain regular expressions #461

Merged
merged 1 commit into from Sep 3, 2020
Merged

FEATURE: Allow skip_paths config to contain regular expressions #461

merged 1 commit into from Sep 3, 2020

Conversation

OsamaSayegh
Copy link
Collaborator

@SamSaffron when I was trying to enable snapshots on Discourse, I noticed we don't use Mini Profiler's skip_paths
config but instead use pre_authorize_cb to skip paths:

https://github.com/discourse/discourse/blob/90eeb8f7d95bf8e06adb34acbbb65e2c743e551f/config/initializers/006-mini_profiler.rb#L53-L58

We also use pre_authorize_cb to disable profiling on phones/tablets.

Skipping paths in pre_authorize_cb is problematic for snapshotting because requests that fail the pre_authorize_cb check are eligible for snapshotting, and we do want requests from phones/tablets to be eligible for snapshotting, but we never want (I think) requests to skipped paths to be eligible for snapshotting (e.g. I don't think we want snapshots for assets requests).

The only thing stopping us from using skip_paths is that we want regular expressions but skip_paths currently accepts strings only. This PR allows both strings and regular expressions in skip_paths.

@SamSaffron
Copy link
Member

this seems fine to me... thanks!

@SamSaffron SamSaffron merged commit 264b2e0 into MiniProfiler:master Sep 3, 2020
@OsamaSayegh OsamaSayegh deleted the skip-paths-regex branch September 3, 2020 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants