EXT-1486 Heartbeat setting for audit logging #24359
Conversation
|
🟢 |
|
⚪ |
|
⚪ |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| #define AUDIT_PART_COND(key, value, condition) \ | ||
| do { \ | ||
| if (condition && !value.empty()) { \ | ||
| if (condition && !TStringBuf(value).empty()) { \ |
There was a problem hiding this comment.
It allows to specify in AUDIT_LOG macro literals.
Now we can write:
AUDIT_PART("status", "SUCCESS")
While before this "feature" we had to write:
AUDIT_PART("status", TString("SUCCESS"))
And if we call the macro in this way, the code will be less effective, because it creates TString twice: during the check and while inserting to the auditParts
ydb/core/protos/config.proto
Outdated
| optional TFileBackend FileBackend = 2; | ||
| optional TUnifiedAgentBackend UnifiedAgentBackend = 3; | ||
| repeated TLogClassConfig LogClassConfig = 4; | ||
| optional uint32 HeartbeatIntervalSeconds = 5; // Make heartbeat records to audit log every HeartbeatInterval seconds. 0 means that heartbeat is disabled |
There was a problem hiding this comment.
Wouldn't it be better create new field in ELogClass (HeartBeat as example) and verify condition 'HeartbeatIntervalSeconds > 0'?
There was a problem hiding this comment.
I think that it will not be very convenient and will not work as expected. If I were a person that sets up the cluster, I would expect that specifying HeartbeatIntervalSeconds setting should run the heartbeat feature.
There was a problem hiding this comment.
On the one hand, you are right. But on the other hand, I don't like the loss of uniformity in the config. We have all types of logs listed in the ELogClass enum, except for one special one.
Of course, HeartBeat audit is special case, but other types can have special setting in future too
There was a problem hiding this comment.
Discussed. It was decided that there will be new protobuf message with options for HeatBeat logs. Moreover, we agreed that adding new field in ELogClass is better.
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
flown4qqqq
left a comment
flown4qqqq
left a comment
There was a problem hiding this comment.
I think it's worth doing the following:
- add a check that IntervalSeconds > 0;
- set the default IntervalSeconds value, for example, 30
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
UgnineSirdis
added
the
rebase-and-check
github-actions
bot
removed
the
rebase-and-check
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
(cherry picked from commit 884b117)
(cherry picked from commit 884b117)
(cherry picked from commit 884b117)
(cherry picked from commit 884b117)
alexv-smirnov
changed the title
2 participants
Changelog entry
...
Changelog category
Description for reviewers
We need to know if something goes wrong with audit events process - for example, if someone (occasionally or on their purpose) disabled audit logging. At the same time, it can be normal that for some cluster there are no records for a long time. With this setting we can setup alerts on audit and not to get false positive on them.