Skip to content

Conversation

@sgiehl
Copy link
Member

@sgiehl sgiehl commented Oct 30, 2025

Description:

Review

@sgiehl sgiehl force-pushed the dev-19555 branch 10 times, most recently from 308560f to 51d0da8 Compare October 31, 2025 17:33
@sgiehl sgiehl marked this pull request as ready for review October 31, 2025 17:33
@sgiehl sgiehl requested a review from a team October 31, 2025 17:33
Copy link
Member Author

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Note: We need to update the tracking documentation afterwards. I'll create a PR for that once this PR has an approval.

Plugins[] = CustomDimensions
Plugins[] = JsTrackerInstallCheck
Plugins[] = FeatureFlags
Plugins[] = BotTracking
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: This will only enable the plugin for new installs by default. Once the plugin is feature complete we can consider to add an update script to also enabled it for existing installs automatically.

Comment on lines 30 to 33
* This is the first method called when processing a tracker request.
*
* Derived classes can use this method to manipulate a tracker request before the request
* is handled. Plugins could change the URL, add custom variables, etc.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* This is the first method called when processing a tracker request.
*
* Derived classes can use this method to manipulate a tracker request before the request
* is handled. Plugins could change the URL, add custom variables, etc.
* This method is called last.
*
* Derived classes should use this method to insert log data.

Or something like that to document this is for writing, and not for manipulation/filtering.

* @param Date $date Delete records older than this date
* @return int Number of deleted records
*/
public function deleteOldRecords(Date $date): int
Copy link
Member

Choose a reason for hiding this comment

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

Do we also (right now that is) need a way to delete data for a site? Like a new PrivacyManager.deleteDataForDeletedSites event we can hook onto because a LogTable won't work?

Or can we delay that a for a little while?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I think log data removal for deleted sites might work by implementing a LogTable. Didn't hink of this case. For removal by date that didn't work as it first looks up all visit and then iterates over the logtables that have a visitid column, which isn't the case here. I'll see if I can add a LogTable class and an according test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. It doesn't work aswell, as it still tries to check how the table can be join with log_visit. So a LogTable remains useless for now. Will add a new event for it


$idRequest = $this->dao->insert($data);

Common::printDebug('Bot request recorded: idrequest=' . $idRequest);
Copy link
Member

Choose a reason for hiding this comment

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

We should replace that with a DI injected LoggerInterface.

@mneudert mneudert removed their assignment Nov 6, 2025
sgiehl added a commit to matomo-org/developer-documentation that referenced this pull request Nov 7, 2025
Expanded the documentation on bot tracking capabilities and parameters in Matomo, including details on request processing modes and specific parameters evaluated during bot tracking introduced with matomo-org/matomo#23725
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants