Custom dimensions in Log Analytics to replace custom variables

[kaplun]

• Integrate Custom dimensions, as a replacement for Custom
  Variables. (Closes Use Custom Dimensions instead of Custom Variables (for Bots/Http-Status tracking and the feature --w3c-field-regex and --track-http-method) #188)

  Signed-off-by: Samuele Kaplun [email protected]

[kaplun]

Hi @mattab, this is really WIP, completely untested, but to get to see if am kind of implementing what you had in mind.

I tried to keep the code backward compatible, albeit making clear that custom variables are being deprecated.

[kaplun]

So, I am currently running this on production 🎉

[mattab]

Thanks for the PR @kaplun 👍

In general, will you continue working on the PR until all of custom variables stuff is removed & converted to use custom dimensions? For example still seeing regex_group_to_visit_cvars_map and regex_group_to_page_cvars_map and possibly others

I think we may want to wait for Matomo 4 for merging this (although maybe not?) when we also close matomo-org/matomo#11524

[kaplun]

I kept support for regex_group_to_visit_cvars_map and regex_group_to_page_cvars_map but declared them DEPRECATED in the documentation. I have ported to custom dimensions Bot/Not-Bot and HTTP-code/HTTP-method.

I think all implicit usage of custom variables have disappeared in this PR, in favor of custom dimensions. Have I missed something?

[kaplun]

(Note that this PR contains a first commit where I just improve the code style and fix some quick issues. So the real implementation is in the second commit).

[diosmosis]

Code looks great, nothing to say there (except there is some code redundancy, but not super important right now). Some larger issues around the feature itself:

• Removing the Bot/Not-Bot/Http-Code custom variables is a BC break. If existing users depend on them they will be forced to immediately move over to custom dimensions, which I think will be a problem. I think it would be better to have a parameter that switches from custom variables => custom dimensions.
  • Same for the --regex-group-to-...-cvar options. If someone's depending on them, this will cause problems for them.
• I haven't tested, but will this change result in errors if the CustomDimensions plugin is disabled? If so, that could cause problems for users.
• Looks like the code will always try and create a custom dimension for Http-Code/Bot/Not-Bot if one doesn't exist. I can't imagine everyone will want custom dimension slots taken up for those when there could be other more valuable (to them at least) dimensions they could be recording. This would also be an annoying surprise to someone who wasn't expecting it. I think this functionality should be off by default and enabled via CLI option.

👍 for the tests

CC @mattab / @tsteur (In fact, I wouldn't make changes based on these review comments until these two have weighed in.)

[diosmosis]

One other thing: before merging we should add system tests to ImportLogsTest.php in matomo. In case you don't have time (or just don't want to), I'd be happy to add those myself (when this PR is ready to merge).

[kaplun]

Removing the Bot/Not-Bot/Http-Code custom variables is a BC break. If existing users depend on them they will be forced to immediately move over to custom dimensions, which I think will be a problem. I think it would be better to have a parameter that switches from custom variables => custom dimensions.

Sure! Would you have some specs of which parameters you'd expect to choose between custom vars Vs. custom dims?

Same for the --regex-group-to-...-cvar options. If someone's depending on them, this will cause problems for them.

Note that these are still supported. I just amended the documentation with DEPRECATED.

I haven't tested, but will this change result in errors if the CustomDimensions plugin is disabled? If so, that could cause problems for users.

👍 indeed I would need to gracefully communicate the error to the user. Also I haven't figured out a way to discover when Dimensions are exhausted.

Looks like the code will always try and create a custom dimension for Http-Code/Bot/Not-Bot if one doesn't exist. I can't imagine everyone will want custom dimension slots taken up for those when there could be other more valuable (to them at least) dimensions they could be recording. This would also be an annoying surprise to someone who wasn't expecting it. I think this functionality should be off by default and enabled via CLI option.

There I mimicked the custom variables behavior. Before this PR custom variables were implicitly consumed to store Bot/Not-Bot/HTTP-Code/HTTP-Method. So there is probably a need to further amend the CLI, and make things more explicit. I can take care of amending it you can come up with exact specs of the CLI you think would be the most correct.

Re: tests in PHP, I am currently slower on PHP than Python, so I'd appreciate help on that side 😄

[diosmosis]

Since no one else commented, I'll put my opinions:

Sure! Would you have some specs of which parameters you'd expect to choose between custom vars Vs. custom dims?

Since the default behavior before was to always add them as custom variables, I think that should be retained. Then there could be a new option like --use-custom-dimensions (can't think of a better name) that would use custom dimensions instead.

Note that these are still supported. I just amended the documentation with DEPRECATED.

Gotcha, 👍

There I mimicked the custom variables behavior. Before this PR custom variables were implicitly consumed to store Bot/Not-Bot/HTTP-Code/HTTP-Method. So there is probably a need to further amend the CLI, and make things more explicit. I can take care of amending it you can come up with exact specs of the CLI you think would be the most correct.

Here are some ideas:

• an --auto-create-dimensions option that when specified will try to create dimensions. if not supplied, and a dimension doesn't exist, we throw an exception. would probably be good in this case to check for each dimension's existence when starting the script, so we don't end up importing some logs & then failing. Maybe it would also be a good idea to check for the dimensions specified in --regex-group-to-...-cdim options, in case of a typo.
• ... actually that should do it I think, don't need more options.

Thoughts?

Re: tests in PHP, I am currently slower on PHP than Python, so I'd appreciate help on that side 😄

No worries, the system tests in matomo are particularly unfun to newcomers, and I wouldn't want you to stop contributing :)