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

LOG-5998: fix max_line_bytes for audit logs #2844

Open
wants to merge 1 commit into
base: release-6.0
Choose a base branch
from

Conversation

jcantrill
Copy link
Contributor

Description

This PR:

  • bumps the max_line_bytes for audit logs
  • bumps the max_read_line_bytes for audit logs

Links

https://issues.redhat.com/browse/LOG-5998

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 24, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 24, 2024

@jcantrill: This pull request references LOG-5998 which is a valid jira issue.

In response to this:

Description

This PR:

  • bumps the max_line_bytes for audit logs
  • bumps the max_read_line_bytes for audit logs

Links

https://issues.redhat.com/browse/LOG-5998

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jcantrill
Copy link
Contributor Author

/cherrypick master

@openshift-cherrypick-robot

@jcantrill: once the present PR merges, I will cherry-pick it on top of master in a new PR and assign it to you.

In response to this:

/cherrypick master

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

openshift-ci bot commented Oct 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2024
@jcantrill
Copy link
Contributor Author

/cherrypick release-5.9

@openshift-cherrypick-robot

@jcantrill: once the present PR merges, I will cherry-pick it on top of release-5.9 in a new PR and assign it to you.

In response to this:

/cherrypick release-5.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Comment on lines +16 to +17
max_line_bytes = 3145728
max_read_bytes = 262144
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be the other way around?

Suggested change
max_line_bytes = 3145728
max_read_bytes = 262144
max_read_bytes = 3145728
max_line_bytes = 262144

max_line_bytes
optional
uint
The maximum size of a line before it is discarded.

This protects against malformed lines or tailing incorrect files.

default: 102400 (bytes)
max_read_bytes
optional
uint
Max amount of bytes to read from a single file before switching over to the next file. Note: This does not apply when oldest_first is true.

This allows distributing the reads more or less evenly across the files.

default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree. in the docs the defaults identify read < max which should apply in our case too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, strike my last comment. The "max_line_bytes" is essentially the largest length a line can be and the assumption is that when completely read, the result is a well formed line message (e.g. json). max_read_bytes is the amount of bytes read in a "read" operation but doesn't say it has completely read a log entry

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed but under normal circumstances you could read a batch of lines that in total are longer than the longest acceptable single line.

Copy link
Contributor

Choose a reason for hiding this comment

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

If max "read from a single file before switching over to the next file" is set 12 times lower than before, are we sure of the impact?

Copy link
Contributor

Choose a reason for hiding this comment

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

Scanning the discord, it does seem that if not set to the same value, the "read" is approx 10x smaller in general
ex.
max_line_bytes: 16777216 max_read_bytes: 16777216
or
max_line_bytes = 1024000 max_read_bytes = 102400

@jcantrill
Copy link
Contributor Author

/test e2e-target

@cahartma
Copy link
Contributor

cahartma commented Nov 5, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2024
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD a047172 and 2 for PR HEAD cfb49fc in total

Copy link
Contributor

openshift-ci bot commented Nov 5, 2024

@jcantrill: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-target cfb49fc link true /test e2e-target

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. release/6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants