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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve GA/consent manager detection by not restricting to curl and caching result #20008

Merged
merged 3 commits into from Nov 15, 2022

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented Nov 14, 2022

Description:

Changes the Site Content Detector class to use lazy caching for detected site properties so that site content will only be checked at most once per week. A lower cache lifetime than four weeks was used so that existing Matomo users who add a content manager to their site can see the setup guide links within a few days.

Explicit curl usage has been replaced with getHTTPTransport() as a parameter to sendHttpRequestBy() so that there is no restriction to just using curl, but invalid SSL certificates can still be ignored.

Fixes #20000

(Remember to turn off development mode in config.ini.php for cache testing! 馃檪 )

Review

鈥cit curl usage to getHTTPTransport(), code tidy and added additional comments
@bx80 bx80 added the c: Performance For when we could improve the performance / speed of Matomo. label Nov 14, 2022
@bx80 bx80 added this to the 4.12.4 milestone Nov 14, 2022
@bx80 bx80 self-assigned this Nov 14, 2022
Copy link
Contributor

@justinvelluppillai justinvelluppillai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the unused import is removed and @mattab gets back to us on the timeout (5s looks good to me) this looks good to merge 馃憤馃徑

@bx80 bx80 merged commit 5dd178a into 4.x-dev Nov 15, 2022
@bx80 bx80 deleted the m20000 branch November 15, 2022 05:55
@sgiehl
Copy link
Member

sgiehl commented Nov 15, 2022

@bx80 Another suggestion for improvement:
It might be good to check enable_internet_features config before sending any request. If that one is disabled by the user, I'm pretty sure they don't want Matomo to send any requests...

@elabuwa elabuwa added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Performance For when we could improve the performance / speed of Matomo. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve GA/consent manager detection by not restricting to curl and caching result
4 participants