Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Config] Update 'Audit log' to using a new config #3983

Merged
merged 4 commits into from
Feb 8, 2024
Merged

Conversation

xuniq
Copy link
Contributor

@xuniq xuniq commented Jan 9, 2024

Fixes #3667
Fixes https://github.com/tarantool/enterprise_doc/issues/258
Fixes https://github.com/tarantool/enterprise_doc/issues/257
Fixes https://github.com/tarantool/enterprise_doc/issues/221 - also related to 2.11, don't forget to update this in 2.11 branch
Fixes https://github.com/tarantool/enterprise_doc/issues/248

@xuniq xuniq force-pushed the gh-3667-audit-new branch 3 times, most recently from e853fc1 to a696542 Compare January 18, 2024 11:01
@xuniq xuniq force-pushed the gh-3667-audit-new branch 2 times, most recently from 51c1d26 to 559bf9c Compare January 23, 2024 16:46
@xuniq xuniq marked this pull request as ready for review January 23, 2024 16:46
@xuniq xuniq force-pushed the gh-3667-audit-new branch 2 times, most recently from eaf6ff3 to 1904098 Compare January 24, 2024 09:56
@xuniq xuniq requested a review from andreyaksenov January 24, 2024 09:57
@xuniq xuniq force-pushed the gh-3667-audit-new branch 2 times, most recently from ac90435 to 9e07fff Compare January 24, 2024 10:20
doc/enterprise/audit_log.rst Outdated Show resolved Hide resolved
doc/enterprise/audit_log.rst Outdated Show resolved Hide resolved
doc/enterprise/audit_log.rst Outdated Show resolved Hide resolved
doc/enterprise/audit_log.rst Outdated Show resolved Hide resolved
doc/enterprise/audit_log.rst Outdated Show resolved Hide resolved
doc/reference/configuration/configuration_reference.rst Outdated Show resolved Hide resolved
doc/reference/configuration/configuration_reference.rst Outdated Show resolved Hide resolved
doc/reference/configuration/configuration_reference.rst Outdated Show resolved Hide resolved
doc/reference/configuration/configuration_reference.rst Outdated Show resolved Hide resolved
@xuniq xuniq force-pushed the gh-3667-audit-new branch 2 times, most recently from b50c05a to 2ab386d Compare February 2, 2024 08:40
Copy link
Contributor

@andreyaksenov andreyaksenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LGTM


Specify a pipe for the audit log destination.
You can set the ``pipe`` type using the :ref:`audit_log.to <configuration_reference_audit_to>` option.
If log is a program, its pid is stored in the ``audit_log.logger_pid`` variable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is audit_log.logger_pid means audit_log module (obtained by require('audit_log')) and its field logger_pid?

Now it looks very similar to audit_log.to and others, which are configuration options in this context. Maybe it worth to clarify it here to avoid a confusion.

Comment on lines +170 to +179
If the ``audit_log`` string starts with '|',
the string is interpreted as a Unix `pipeline <https://en.wikipedia.org/wiki/Pipeline_%28Unix%29>`_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pipeline connected to process'es stdin, I guess?

So, if there is no pipe at start, where the logs will be send?

(These my questions as a reader, I don't remember how it works.)

Comment on lines 624 to 625
However, it is not recommended to use the entire memory, as this may cause performance degradation
and even loss of some logs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an unclear recommendation, because there are no configurable memory limit for the audit log.

Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections from my side. I left several questions to discuss if you like.

@xuniq xuniq force-pushed the gh-3667-audit-new branch from 97456da to 83f0975 Compare February 7, 2024 08:26
@xuniq xuniq force-pushed the gh-3667-audit-new branch from 83f0975 to 849bb3d Compare February 8, 2024 13:57
@xuniq xuniq merged commit 4b018aa into latest Feb 8, 2024
1 check passed
@xuniq xuniq deleted the gh-3667-audit-new branch February 8, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Config] Update 'Audit log' to using a new config
3 participants