Problem/Motivation

Hello everyone,
Recently in one of our projects we had an issue where the config_log table had a huge size. The issue was not caused by the config log module but it led us to discover that the module does not have a way to delete logs in the config_log table. We had to implement such a solution so I'm leaving here the patch in case it could help anyone.

Proposed resolution

Add a solution similar to what the dblog module uses. We added a new configuration to the config page where the user can select how many logs they would want to store. Based on that we also added a cron job to delete the logs in the config_log table. Nothing was done on the logs stored on the watchdog table.

CommentFileSizeAuthor
#10 add-retention-policy-to-logs-3547908-3.patch2.54 KByazanmajadba
#5 add-retention-policy-to-logs-3547908-5.patch3.52 KBnsalves
#2 add-retention-policy-to-logs-3547908-2.patch2.6 KBnsalves

Issue fork config_log-3547908

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

nsalves created an issue. See original summary.

nsalves’s picture

nsalves commented
StatusFileSize
new add-retention-policy-to-logs-3547908-2.patch2.6 KB
nsalves’s picture

nsalves commented
Status: Active » Needs review
nagy.balint’s picture

nagy.balint commented
Version: 3.0.x-dev » 4.x-dev
Status: Needs review » Needs work

Thank you for the patch!

I have the following feedback:

1. We use ConfigLogDatabaseSubscriber::$type to define "custom" so the line
+ ':input[name="log_destination[custom]"]' => ['checked' => TRUE],
should also use that.

And likely below in the getValue as well.

2. $config_log_conf->get('logs_to_keep') might be problematic if a site updates to this version and the config does not exist. It would be better to add a fallback value to that.

3. config_log.schema.yml and config_log.settings.yml is missing the new config item.

4. If logs_to_keep is int type, then it might be better to store a 0 in the else instead of $config->set('logs_to_keep', '');

nsalves’s picture

nsalves commented
StatusFileSize
new add-retention-policy-to-logs-3547908-5.patch3.52 KB

Hey nagy.balint, thanks for the feedback. Here's a new version of the patch where I believe I addressed all the feedback. Thanks !

nsalves’s picture

nsalves commented
Status: Needs work » Needs review
nagy.balint’s picture

nagy.balint commented

Hi!

Thank you!

Can you reroll the patch for the 4.x branch? as that is the main supported branch?

Also in the cron we might need to do the same
$row_limit = \Drupal::config('config_log.settings')->get('logs_to_keep');
to add ?: 0

even though NULL > 0 i think is false, would be better.

yazanmajadba made their first commit to this issue’s fork.

yazanmajadba opened merge request !24

yazanmajadba’s picture

yazanmajadba commented
StatusFileSize
new add-retention-policy-to-logs-3547908-3.patch2.54 KB

Here’s the updated patch for the 4.x branch.

https://git.drupalcode.org/project/config_log/-/merge_requests/24.patch

nagy.balint’s picture

nagy.balint commented

Hmm,
The add-retention-policy-to-logs-3547908-5.patch in #5 already had a cron which was simpler, you have not found it good enough?

yazanmajadba’s picture

yazanmajadba commented

The patch is great, but it doesn't work with 4.x.
I rererolled the patch to work with 4.x and followed your instructions on #7

nagy.balint’s picture

nagy.balint commented
Status: Needs review » Fixed

Thank You!

The two patches have different cron, but it seems the second one is better.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.