[2.1] Bots - reduce log_online updates#9125
[2.1] Bots - reduce log_online updates#9125sbulen wants to merge 3 commits intoSimpleMachines:release-2.1from
Conversation
Signed-off-by: Shawn Bulen <bulens@pacbell.net>
|
@sbulen Do you have the related updates for 3.0 on this PR? |
|
I was going to add 3.0 versions once the 2.1 ones were approved. Trying to avoid duplication of rework, etc. This one has clear tradeoffs, but given there's a switch, folks can turn it on only when needed. |
|
Just a quick nit: shoudn't this new sertting be in camel case to match the existing convention for |
Good question... We have more underscore_var_names in Settings.php today, which is why I went with that. (It's like 55-45...) I don't think there's a variable or key name PSR suggestion, it's wide open on the standards front, I think. And in our usage... |
In SMF 2.1, case conventions are a complete mess. But in SMF 3.0, the convention is to use underscores for variable names and camel case for function/method names. There are a few exceptions to this (most notably, So @sbulen is doing the right thing. By using underscores in the variable names in his 2.1 code now, he's ensuring that he won't need to change the variable names in his 3.0 code later. |
|
If the default setting is disabled and we check for an enabled feature, do we need the upgrader or installer to provide those values? |
other/install_2-1_mysql.sql
Outdated
| ('knownThemes', '1'), | ||
| ('enableThemes', '1'), | ||
| ('who_enabled', '1'), | ||
| ('no_guest_logging', '0'), |
There was a problem hiding this comment.
This is not necessary. Because the value is 0 and the checks for it use !empty($modSettings['no_guest_logging']), the setting doesn't need to be inserted into the table until the first time it is enabled by the admin.
other/install_2-1_postgresql.sql
Outdated
| ('knownThemes', '1'), | ||
| ('enableThemes', '1'), | ||
| ('who_enabled', '1'), | ||
| ('no_guest_logging', '0'), |
There was a problem hiding this comment.
This is not necessary. See previous comment.
other/upgrade_2-1_mysql.sql
Outdated
| ---# Add new setting to prevent guest/bot logging | ||
| ---{ | ||
| if (!isset($modSettings['no_guest_logging'])) | ||
| $smcFunc['db_insert']('insert', | ||
| '{db_prefix}settings', | ||
| array('variable' => 'string', 'value' => 'string'), | ||
| array('no_guest_logging', '0'), | ||
| array('variable') | ||
| ); | ||
| ---} | ||
| ---# | ||
|
|
There was a problem hiding this comment.
This is not necessary. See previous comment.
other/upgrade_2-1_postgresql.sql
Outdated
| ---# Add new setting to prevent guest/bot logging | ||
| ---{ | ||
| if (!isset($modSettings['no_guest_logging'])) | ||
| $smcFunc['db_insert']('ignore', | ||
| '{db_prefix}settings', | ||
| array('variable' => 'string', 'value' => 'string'), | ||
| array('no_guest_logging', '0'), | ||
| array('variable') | ||
| ); | ||
| ---} | ||
| ---# | ||
|
|
There was a problem hiding this comment.
This is not necessary. See previous comment.
I had deliberately set it up so behavior is unchanged if no var is found. Felt safest. Also, easiest to patch, no update needed. But I did add to the installer & upgrader, mainly because I like seeing all the 'real' SMF vars doc'd in that fashion. To my knowledge, we've always added them. I'll go either way, just letting you know my thoughts. |
|
There are a number of settings that aren't initially populated during install or upgrade, but rather only created on the fly the first time the admin enables them. |
|
Ah, poop. OK, I'll remove. |
Signed-off-by: Shawn Bulen <bulens@pacbell.net>
|
Done. |
Signed-off-by: Shawn Bulen <bulens@pacbell.net>
|
Merge conflicts resolved. A couple of these PRs touched adjacent code. |
4 participants
This PR aims to minimize I/O during a bot attack by eliminating the logging of guests & bots in log_online. The problem is that the really troublesome bots are indistinguishable from guests via normal means, e.g., useragent.
An option is added, allowing the user to turn this behavior on or off. Users may wish to disable this only during high-CPU bot attacks.
Note that some of these bot attacks are what I call 'long tail' attacks, in which many IPs are used once and only once. This was a trademark of the 2025 Q3 Brazil/Vietnam attack; on some days, I would have 150,000+ IPs with only one request each... The problem being that each would get its own unique session & log_online record, causing a flood of writes & log activity. And then, of course, they would all need to be deleted during garbage collection.
With this change, they wouldn't be written at all.
I've been running this code on my prod forum with no issues. Though, I must admit, forum members asked this one to be removed, as they liked following those stats. Spikes usually indicated cross-forum discussions with other gear forums. Somehow they gleaned that thru all the bot noise (tbh, I dunno how...). Still, having the option to disable during a botnet attack with high CPU is an important option.
If this is approved, I can submit a 3.0 version.
For more discussion see:
https://www.simplemachines.org/community/index.php?topic=592442.0
https://www.simplemachines.org/community/index.php?topic=592345.0
Feedback welcome.