-
Notifications
You must be signed in to change notification settings - Fork 27
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
Support placeholders #3
Comments
The problem with placeholders is that they're dynamic. The regexp are compiled ahead of time for performance. If they weren't, then it would be much slower. While the replacements don't need to be compiled, they are prepared ahead of time to be passed to https://pkg.go.dev/golang.org/x/text/transform which performs the actual replacements (via https://github.com/icholy/replace which implements the regexp part). It might be possible to use |
Yeah, so, we can add placeholders BUT, as Francis said, the values are compiled ahead of time (so we could only perform replacements on things like env variables and other, non-HTTP-request-specific variables), for performance reasons. Is that sufficient? What's your actual use case? |
My use case has to do with, among other things modifying javascript returned through a reverse proxy. It does require HTTP-request specific variables but no regex - although regex would help :-) My case, there will probably never be more than 30 versions of a regex so lazy creation of the compiled regex and keeping a lru capped at 100 or 1000 of them would be more than enough for our code. |
@mheyman To clarify, all replacements are processed at config load (startup), not at every request, whether using regular expressions or not: Lines 88 to 99 in 9d5652c
Unless there's a clever solution I haven't thought of, it's likely that implementing this feature will cause a performance regression. |
I would hope there would be no performance regression because in the case where the information is available at config load, no change in In our use case, we probably wouldn't notice the slowdown this alternative middleware would cause over the fast path because we only have a few users hitting the pages over an already-slow link. Also, like I mentioned earlier, there would only be 30ish different possible searches in our case so a reasonably sized LRU cache would easily hold all our possible searches. We can code these 30ish search strings manually in the current implementation but it means rewriting the config and restarting the reverse proxy when any of the 30ish proxied services change (happens often). Additionally, we may not always have insight into when things change except when things break. And we'd rather not run with that method of operation :-) |
I guess that's technically possible, but it adds a lot of complexity. |
Currently, placeholders are not supported in the search or replacement text. It would be really nice to be able to use them.
The text was updated successfully, but these errors were encountered: