matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.74k stars 2.63k forks source link

Refactor referrer detection to be more flexible / replaceable #20067

Open sgiehl opened 1 year ago

sgiehl commented 1 year ago

Our current referrer detection is hard coded into the referrer plugin, without any possibility to hook into it. This makes it hard for plugins to manipulate that part in a useful way as it's only possible to overwrite the detected values afterwards.

I would suggest to refactor the whole detection part into a more flexible design.

First proposal:

Once this was implemented, we need to adjust the MarketingCampaignsReporting plugin, so it's detection uses the new possibilities.

refs L3-362

tsteur commented 1 year ago

@sgiehl is this similar / related to https://github.com/matomo-org/matomo/issues/18511 ?

sgiehl commented 1 year ago

Yes. That should make it possible to fix that one

Starker3 commented 1 year ago

FYI this issue shows up in the visitor profile, visits log and in the reports. For example if you have segments that use a specific campaign property as the condition, the visit would be included but because of the bug the segment reports could show it as ‘Direct Entry’ or as a different campaign completely.

 This also means that the standard campaign reports could also be wrong, i.e. showing a higher number of direct entry even when a visitor actually entered through a campaign or showing as a potentially different campaign.

mattab commented 1 year ago

@sgiehl Couple of questions:

sgiehl commented 1 year ago

@mattab I guess we could implement that in a way that it won't break BC. And yes, this should also allow to fix #18511

Stan-vw commented 1 year ago

@mattab why is this tagged as 5.1.0, this doesn't sound like extremely high value to a large group of users. Perhaps you can help me understand what value this unlocks?

Starker3 commented 1 year ago

@Stan-vw Matt might have more to add here, but I believe the main reason this is 5.1.0 is because it'll be required to fix the related issue: https://github.com/matomo-org/matomo/issues/18511 Which currently might impact a large number of users, many of which likely won't even notice it unless they look closely between the reports and visits log.

sgiehl commented 1 year ago

@Stan-vw There might actually also be a couple more undiscovered bugs around campaign detection when using MarketingCampaignsReporting plugin. The problem is, that the detection in core and the plugin might differ, which could even end up in incorrect data being displayed. That might e.g. already be the case when core detects a campaign, while the plugin doesn't (or vice versa). I can explain that in detail in a quick call if you want.

mattab commented 1 year ago

Maybe to keep things simpler, @sgiehl @tsteur we could simply integrate the Marketing Campaign plugin into core and make the 5 dimensions part of the default functionality of Matomo? it might reduce some complexity, and wouldn't really impact users anyway...

sgiehl commented 1 year ago

@mattab Guess that would make some things easier, but we should still aim to refactor the referrer detection, as the current structure is quite complex to understand

Stan-vw commented 1 year ago

@sgiehl would you recommend amending the scope you set out in the ticket with Matt's suggestion, or is there further work you're suggesting? If it's the former, perhaps you can adjust the original post and we can plan the work as such 👍

sgiehl commented 1 year ago

Integrating the MarketingCampaignsReporting plugin to core will require some additional effort. There wouldn't be much benefit in simply shipping the plugin with core. To make code simpler and less error prone we need to restructure the referrer detection to be a bit more flexible (what this issue originally is about), we could then maybe move the campaign detection parts from referrer plugin to the integrated MarketingCampaignsReporting plugin, so all campaign related reporting would end up in that plugin.