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

Add configurable channel queue_size for audit/logger webhook targets #13819

Merged
merged 1 commit into from Dec 20, 2021

Conversation

harshavardhana
Copy link
Member

Description

Add configurable channel queue_size for audit/logger webhook targets

Motivation and Context

Also, log all the missed events and logs instead of silently
swallowing the events.

Bonus: Extend the logger webhook to support mTLS
similar to audit webhook target.

How to test this PR?

Build locally and run

~ MINIO_AUDIT_WEBHOOK_ENABLE_test=on MINIO_AUDIT_WEBHOOK_ENDPOINT_test=https://test123.free.beeceptor.com MINIO_AUDIT_WEBHOOK_QUEUE_SIZE_test=10000 minio server /tmp/test{1...4}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Documentation updated
  • Unit tests added/updated

Also log all the missed events and logs instead of silently
swallowing the events.

Bonus: Extend the logger webhook to support mTLS
similar to audit webhook target.
@minio-trusted
Copy link
Contributor

Mint Automation

Test Result
mint-large-bucket.sh ✔️
mint-fs.sh ✔️
mint-gateway-s3.sh ✔️
mint-erasure.sh ✔️
mint-dist-erasure.sh ✔️
mint-gateway-nas.sh ✔️
mint-compress-encrypt-dist-erasure.sh ✔️
mint-pools.sh ✔️
Deleting image on docker hub
Deleting image locally

Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

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

I presume you have a need for this.

TBH I don't see any case where increasing beyond 100K would improve things. If that buffer is filled it will likely not be able to keep up.

@harshavardhana
Copy link
Member Author

I presume you have a need for this.

TBH I don't see any case where increasing beyond 100K would improve things. If that buffer is filled it will likely not be able to keep up.

yes there is a need but going to make our httpLogger concurrency friendly by adding more workers, currently its serialized - adding more workers would help here.

@harshavardhana harshavardhana merged commit 499872f into minio:master Dec 20, 2021
@harshavardhana harshavardhana deleted the audit-queue-size branch December 20, 2021 21:17
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Mar 15, 2022
PR introduced in minio#13819 was incorrect and was not
handling the situation where a buffer is full can
cause incessant amount of logs that would keep the
logger webhook overrun by the requests.

To avoid this only log failures to console logger
instead of all targets as it can cause self reference,
leading to an infinite loop.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Mar 15, 2022
PR introduced in minio#13819 was incorrect and was not
handling the situation where a buffer is full can
cause incessant amount of logs that would keep the
logger webhook overrun by the requests.

To avoid this only log failures to console logger
instead of all targets as it can cause self reference,
leading to an infinite loop.
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Mar 15, 2022
PR introduced in minio#13819 was incorrect and was not
handling the situation where a buffer is full can
cause incessant amount of logs that would keep the
logger webhook overrun by the requests.

To avoid this only log failures to console logger
instead of all targets as it can cause self reference,
leading to an infinite loop.
harshavardhana added a commit that referenced this pull request Mar 16, 2022
PR introduced in #13819 was incorrect and was not
handling the situation where a buffer is full can
cause incessant amount of logs that would keep the
logger webhook overrun by the requests.

To avoid this only log failures to console logger
instead of all targets as it can cause self reference,
leading to an infinite loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants