-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Add Tracking Parameters to Varnish Configuration #39188
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
Add Tracking Parameters to Varnish Configuration #39188
Conversation
Hi @sprankhub. Thank you for your contribution! Add the comment under your pull request to deploy test or vanilla Magento instance:
❗ Automated tests can be triggered manually with an appropriate comment:
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
If you have doubts regarding the parameters, check https://github.com/mpchadwick/tracking-query-params-registry/blob/master/_data/params.csv, where all of them are mentioned including a reference. |
It would be good to handle this PR together with #35228 ( |
Thanks for the feedback, @hostep. I guess it would be easiest if I include the parameter here in this PR. Or should I try to cherry-pick that other commit to keep the author information? What do you think? |
Yeah cherry-picking is probably the best, this PR has a lot more upvotes than the other and probably will be picked up sooner than the other, so having everything fixed in one PR is probably the fastest way to get everything in the core. |
- Related to Issue magento#35227 - Klaviyo's emails include the `_kx` query parameter. - Klaviyo's services and modules are becoming more popular, so this query parameter should be stripped in the default Varnish configs provided with Magento.
I now cherry-picked the changes from #35228 and also applied the change to the Varnish 7 VCL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great contribution, thanks! I don't think any tests are required in this case, since it's only about vcl file modifications, not about the core platform logic.
@magento run all tests |
Hi @sprankhub , Thanks for the collaboration & contribution! ✔️ QA PassedPreconditions:
Steps to reproduce
Before: ✖️ ![]() After: ✔️ ![]() Builds are failed. Hence, moving this PR to Extended Testing. Thanks. |
@magento run all tests |
Cheers |
@magento run Functional Tests B2B, Functional Tests EE |
@magento run Functional Tests B2B |
@magento run Functional Tests B2B |
Few of the functional B2B test failures are not consistent and one of them is a known issue. Hence moving this PR in Merge in Progress now. ![]() ![]() Known issues: |
bb3efa8
into
magento:2.4-develop
Description (*)
Add additional tracking parameters to the Varnish configuration, so that the same page will be cached independent of those tracking parameters.
Related Pull Requests
#38299
#38302
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Previous PRs have not been merged and not been updated. Hence, here is another try to get this over the line.
Contribution checklist (*)