-
Notifications
You must be signed in to change notification settings - Fork 8k
support reset log level or stack trace level separately for admin log #56642
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: xin.li <xin.li@daocloud.io>
Signed-off-by: xin.li <xin.li@daocloud.io>
/test integ-ds |
/test integ-security-multicluster |
/test integ-ds |
istioctl/pkg/admin/istiodconfig.go
Outdated
return &istiodConfigLog{state: &resetState{ctrzClient}} | ||
} else if outputLogLevel != "" { | ||
case logRest: |
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.
Also, does this mean that if both --log-reset and -stracktrace-reset are passed, only the log level will be reset?
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.
This is a problem. I just added a check to remind users that they cannot specify --log-reset
and --stack-trace-reset
at the same time.
Signed-off-by: xin.li <xin.li@daocloud.io>
Signed-off-by: xin.li <xin.li@daocloud.io>
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.
Sorry for the delay, but is there a reason why we can't pass both flags? Would also be good to have a test for it.
No, just an option, I re-evaluated it and specifying both seems better. I have updated and added the test. Please review again, thanks |
Signed-off-by: xin.li <xin.li@daocloud.io>
Please provide a description of this PR:
Currently there is only one
--reset
flag to reset the log level for both log and stack trace. It would be better to be able to reset two convenient log levels separately (like--level
and--stack-trace-level
for each).To help us figure out who should review this PR, please put an X in all the areas that this PR affects.
Please check any characteristics that apply to this pull request.