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.68k stars 2.62k forks source link

Added a config for custom_dimensions_max_length issue-16150 #22582

Open jorgeuos opened 1 week ago

jorgeuos commented 1 week ago

Added a config for custom_dimensions_max_length and refactor plugins/CustomDimensions/Tracker/CustomDimensionsRequestProcessor.php:prepareValue() to use the config instead of being hardcoded.

Description:

I will change the max length for custom dimensions in: https://github.com/matomo-org/matomo/blob/4.11.0-rc1/plugins/CustomDimensions/Tracker/CustomDimensionsRequestProcessor.php#L175

To see how it behaves before I apply my fix.

I'm tracking custom dimensions with a default installation of Matomo. No extra configuration or plugins added.

The default custom_dimension_1 type is varchar(255) in the database.

The tracking code is:

    var _paq = window._paq = window._paq || [];
    var customDimensionId = 1;
    var customDimensionValue = 'LongValue1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456
    789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901ThisShouldBeCutOff';
    _paq.push(['setCustomDimension', customDimensionId, customDimensionValue]);

I'm modifying the php function like this:

private static function prepareValue($value)
{
    $inputMaxLength = 267;
    $return = mb_substr(trim($value), 0, $inputMaxLength);
    return $return;
}

Story 1:

Story 2:

Story 3:

Story 4:

  1. I need to change the default column type for custom dimensions in the database from varchar(255) to text. According to this issue in Github: https://github.com/matomo-org/matomo/issues/16150 we can alter the column type in the database like this:
    ALTER TABLE matomo_log_visit
    MODIFY COLUMN custom_dimension_1 TEXT,
    MODIFY COLUMN custom_dimension_2 TEXT,
    MODIFY COLUMN custom_dimension_3 TEXT,
    MODIFY COLUMN custom_dimension_4 TEXT,
    MODIFY COLUMN custom_dimension_5 TEXT;

    Which would also help with another unrelated issue: https://forum.matomo.org/t/error-while-updating-to-4-0-0-b1-have-to-change-some-columns-to-text-or-blobs/39482

After a successful change in the database, I will track a custom dimension with a value that is too long so that I can see how Matomo behaves. custom_dimension_1 type is now TEXT in the database.

  1. Move on to the test case:
$inputMaxLength = 256; // 204 OK

Stored value in MySQL:

LongValue1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901ThisSh

Which equals 256 characters exactly. custom_dimension_1 type is TEXT

Story 5:

$inputMaxLength = 65535; // Max length for TEXT

Stored value in MySQL:

LongValue1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901ThisShouldBeCutOff

Which equals 250 characters exactly. custom_dimension_1 type is TEXT in the database.

Conclusion

The default maximum length for custom dimensions is 250 characters. If a value exceeds this limit, it will be truncated and stored, unless the length exceeds 255 characters (the database limit for varchar(255)), in which case it will not be stored at all. Additionally, no error is displayed in the user interface, and no tracking failures are logged.

To fix this issue, a user needs to change the column type for custom dimensions in the database from varchar(255) to TEXT.

Potential edge cases thought about:

Review

jorgeuos commented 1 week ago

This is to fix the issue-16150