-
Notifications
You must be signed in to change notification settings - Fork 342
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
Parse Apache log etime and display average per command #5717
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rcritten
thanks for the PR. Please find inline comments
contrib/perflog
Outdated
super(parselog, self).run() | ||
|
||
if not is_ipa_configured(): | ||
logger.error("IPA client is not configured on this system.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message should be "IPA server is not configured on this system".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably missed this comment: should be "IPA is not configured".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rcritten,
thanks for the update. Please find an additional inline comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rcritten please find 2 minor inline nitpicks, other comments already addressed.
contrib/perflog
Outdated
super(parselog, self).run() | ||
|
||
if not is_ipa_configured(): | ||
logger.error("IPA client is not configured on this system.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably missed this comment: should be "IPA is not configured".
Including execution time (etime) was added in commit 4d716d3 This is a parser that will collect API executions and average them by command. If > 5 requests of the same type then the fastest and slowest results will be dropped to try to smooth the average. These averages will be used for two purposes: 1. Identify potential bottlenecks in API performance 2. Provide a baseline so that future performance changes can be measured. It is included in contrib because this is not going to be shipped with a distribution but is useful to have with the code. A sample execution is: Successful commands: Mean user_show: 12234152.5 of 2 executions Mean command_defaults: 3284363.0 of 3 executions Mean user_add: 594369554.5 of 2 executions Exceptions: Mean user_del: 232540327 ns of 2 executions The parselog command was successful Times are in nanoseconds. https://pagure.io/freeipa/issue/8809 Signed-off-by: Rob Crittenden <rcritten@redhat.com>
@rcritten thanks for the update, works for me. ACK for when PRCI completes |
master:
|
Including execution time (etim) was added in commit
4d716d3
This is a parser that will collect API executions and
average them by command.
If > 5 requests of the same type then the fastest and slowest
results will be dropped to try to smooth the average.
These averages will be used for two purposes:
measured.
It is included in contrib because this is not going to be shipped
with a distribution but is useful to have with the code.
A sample execution is:
Mean user_show: 12234152.5 of 2 executions
Mean command_defaults: 3284363.0 of 3 executions
Mean user_add: 594369554.5 of 2 executions
The parselog command was successful
Times are in nanoseconds.
https://pagure.io/freeipa/issue/8809
Signed-off-by: Rob Crittenden rcritten@redhat.com