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 env variable "ODO_LOG_LEVEL" to control odo verbosity (#2351) #2378

Merged
merged 1 commit into from Nov 12, 2019

Conversation

@kanchwala-yusuf
Copy link
Collaborator

kanchwala-yusuf commented Nov 8, 2019

  • Add enviroment variable "ODO_LOG_LEVEL" to control odo verbosity. Command line flag "-v", if set, will take precedence over "ODO_LOG_LEVEL"

What kind of PR is this?
/kind enhancement

What does does this PR do / why we need it:
This PR enable a user to set odo log level by setting the environment variable "ODO_LOG_LEVEL"

Which issue(s) this PR fixes:
Fixes #2351

@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Nov 8, 2019

Hi @kanchwala-yusuf. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

@amitkrout

This comment has been minimized.

Copy link
Collaborator

amitkrout commented Nov 8, 2019

/ok-to-test

@amitkrout

This comment has been minimized.

Copy link
Collaborator

amitkrout commented Nov 8, 2019

@kanchwala-yusuf - https://travis-ci.com/openshift/odo/jobs/254383933#L563

$ make golint
golangci-lint run ./... --timeout 5m
cmd/odo/odo.go:50:23: Error return value of `flag.CommandLine.Set` is not checked (errcheck)
		flag.CommandLine.Set("v", level)
		                    ^
make: *** [golint] Error 1
The command "make golint" exited with 2.
@kanchwala-yusuf kanchwala-yusuf force-pushed the kanchwala-yusuf:2351 branch from 59f45ce to d723d0d Nov 8, 2019
@amitkrout

This comment has been minimized.

Copy link
Collaborator

amitkrout commented Nov 8, 2019

Please document the env var ODO_LOG_LEVEL.

// The "-v" flag set on command line will take precedence over ODO_LOG_LEVEL env
v := flag.CommandLine.Lookup("v").Value.String()
if level, ok := os.LookupEnv("ODO_LOG_LEVEL"); ok && v == "0" {
_ = flag.CommandLine.Set("v", level)

This comment has been minimized.

Copy link
@kadel

kadel Nov 8, 2019

Member

Should we check if the error was returned or not?

This comment has been minimized.

Copy link
@kanchwala-yusuf

kanchwala-yusuf Nov 10, 2019

Author Collaborator

We can, if we want to take some corrective action.

@kadel

This comment has been minimized.

Copy link
Member

kadel commented Nov 8, 2019

@kanchwala-yusuf It would be great to document this somewhere. https://github.com/openshift/odo/blob/master/docs/dev/logging.adoc might be a good place for it.

* Add enviroment variable "ODO_LOG_LEVEL" to control odo verbosity. Command line flag "-v", if set, will take precedence over "ODO_LOG_LEVEL"
@kanchwala-yusuf kanchwala-yusuf force-pushed the kanchwala-yusuf:2351 branch from d723d0d to 2274944 Nov 10, 2019
@openshift-ci-robot openshift-ci-robot added size/S and removed size/XS labels Nov 10, 2019
@kanchwala-yusuf

This comment has been minimized.

Copy link
Collaborator Author

kanchwala-yusuf commented Nov 10, 2019

@kanchwala-yusuf It would be great to document this somewhere. https://github.com/openshift/odo/blob/master/docs/dev/logging.adoc might be a good place for it.

@kadel, @amitkrout, documentation added for ODO_LOG_LEVEL. Kindly review:
https://github.com/openshift/odo/pull/2378/files#diff-6bde9f69cdf69930cd6e07b7e02961a5R9

@kanchwala-yusuf kanchwala-yusuf changed the title [WIP] Add env variable "ODO_LOG_LEVEL" to control odo verbosity (#2351) Add env variable "ODO_LOG_LEVEL" to control odo verbosity (#2351) Nov 11, 2019
@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Nov 11, 2019

@kanchwala-yusuf: The label(s) kind/enhancement cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga

In response to this:

  • Add enviroment variable "ODO_LOG_LEVEL" to control odo verbosity. Command line flag "-v", if set, will take precedence over "ODO_LOG_LEVEL"

What kind of PR is this?
/kind enhancement

What does does this PR do / why we need it:
This PR enable a user to set odo log level by setting the environment variable "ODO_LOG_LEVEL"

Which issue(s) this PR fixes:
Fixes #2351

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/test-infra repository.

Copy link
Collaborator

dharmit left a comment

Minor change request for documentation. Looks good otherwise! Congrats on your first PR to the project @kanchwala-yusuf 🎉 👯‍♂

@@ -6,7 +6,9 @@ https://godoc.org/github.com/golang/glog[Glog] is used for V style logging in `o

Every `odo` command takes an optional flag `-v` that must be used with an integer log level in the range from 0-9. Any INFO severity log statement that is logged at a level lesser than or equal to the one passed with the command alongside `-v` flag will be logged to STDOUT.

All ERROR severity level log messages will always be logged regardless of the passed `v` level.
Alternatively, enviroment variable `ODO_LOG_LEVEL` can be used to set the verbosity of the log level in the integer range 0-9. The value set by `ODO_LOG_LEVEL` can be overridden by explicitly passing the `-v` command line flag to the `odo` command, in such an event `-v` flag takes precedence over the enviroment variable `ODO_LOG_LEVEL`.

This comment has been minimized.

Copy link
@dharmit

dharmit Nov 11, 2019

Collaborator

Changing command line flag to the odo command, in such an event to command line flag to the odo command. In such an event makes more sense to me.

Copy link
Collaborator

mohammedzee1000 left a comment

Can we maybe have a test.
Even something as simple as setting the env, then running a command and see if one of the messages expected at that log level, appear?

Otherwise, looks good

@@ -6,7 +6,9 @@ https://godoc.org/github.com/golang/glog[Glog] is used for V style logging in `o

Every `odo` command takes an optional flag `-v` that must be used with an integer log level in the range from 0-9. Any INFO severity log statement that is logged at a level lesser than or equal to the one passed with the command alongside `-v` flag will be logged to STDOUT.

All ERROR severity level log messages will always be logged regardless of the passed `v` level.
Alternatively, enviroment variable `ODO_LOG_LEVEL` can be used to set the verbosity of the log level in the integer range 0-9. The value set by `ODO_LOG_LEVEL` can be overridden by explicitly passing the `-v` command line flag to the `odo` command, in such an event `-v` flag takes precedence over the enviroment variable `ODO_LOG_LEVEL`.

This comment has been minimized.

Copy link
@mohammedzee1000
Copy link
Collaborator

mohammedzee1000 left a comment

/approve
/assign @dharmit

@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Nov 11, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mohammedzee1000

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

@mohammedzee1000

This comment has been minimized.

Copy link
Collaborator

mohammedzee1000 commented Nov 11, 2019

/lint

Copy link

openshift-ci-robot left a comment

@mohammedzee1000: 0 warnings.

In response to this:

/lint

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/test-infra repository.

@dharmit

This comment has been minimized.

Copy link
Collaborator

dharmit commented Nov 11, 2019

/lgtm

@kadel

This comment has been minimized.

Copy link
Member

kadel commented Nov 11, 2019

/retest
infra issues

error: unable to extract layer sha256:563db9253d3fba7dc4fadecc7baf0954a6f037aa4524204fb5c3df6cddea847b from registry.svc.ci.openshift.org/ci-op-66c14ilz/stable@sha256:3c44af447c1a915af64ea142a2e0d2ca5f8c40252f943064b04d04640892dcec: stream error: stream ID 73; INTERNAL_ERROR

@kanchwala-yusuf

This comment has been minimized.

Copy link
Collaborator Author

kanchwala-yusuf commented Nov 11, 2019

/retest

@dharmit

This comment has been minimized.

Copy link
Collaborator

dharmit commented Nov 12, 2019

level=fatal msg="failed to initialize the cluster: Multiple errors are preventing progress:\n*

/test v4.1-e2e-scenarios

error: unable to upload the new image manifest: received unexpected HTTP status: 500 Internal Server Error

/test v4.1-integration

@kanchwala-yusuf

This comment has been minimized.

Copy link
Collaborator Author

kanchwala-yusuf commented Nov 12, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit 5eb0ff5 into openshift:master Nov 12, 2019
12 checks passed
12 checks passed
Travis CI - Pull Request Build Passed
Details
ci/prow/v4.1-benchmark Job succeeded.
Details
ci/prow/v4.1-e2e-scenarios Job succeeded.
Details
ci/prow/v4.1-integration Job succeeded.
Details
ci/prow/v4.1-unit Job succeeded.
Details
ci/prow/v4.2-benchmark Job succeeded.
Details
ci/prow/v4.2-e2e-scenarios Job succeeded.
Details
ci/prow/v4.2-integration Job succeeded.
Details
ci/prow/v4.3-benchmark Job succeeded.
Details
ci/prow/v4.3-e2e-scenarios Job succeeded.
Details
ci/prow/v4.3-integration Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.