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

KNOX-2771 - Log HTTP client config parameters such as socket timeouts with info level #602

Merged
merged 3 commits into from Jul 6, 2022

Conversation

MrtnBalazs
Copy link
Contributor

What changes were proposed in this pull request?

Changed log level of http connection timeout and socket timeout to from DEBUG to INFO and added INFO level log for retry count and retry non safe request parameters, because sometimes clients were confused why the timeout parameters haven't changed even though they changed it in gateway-site.xml, but service.xml override-s it. Now they can see in the log what is the value of these parameters, only after the first request after topology deployment.

How was this patch tested?

I tested it manually, using curl -k -u tom:tom-password https://localhost:8443/gateway/sandbox/hive command from terminal.

  1. Nothing is set:

semmiben

  1. Timeout params set in gateway-site.xml and retry params set in sandbox.xml:

gatewayben

globalbeallitas

Screenshot 2022-07-06 at 8 56 18

  1. Timeout params set in gateway-site.xml and in service.xml, retry params set like 2.:

servicebenesgatewayben

globalbeallitas

servicekod

We can see that service.xml overrides gateway-site.xml settings.

The logs only appear after the first request after the topology is deployed.
After that it is not logged until the topology is deployed again.
masodszorra nem logol

…UG to INFO and added HTTP client retry count and retry non safe request parameter INFO level logs
@MrtnBalazs
Copy link
Contributor Author

@smolnar82 @zeroflag

Copy link
Contributor

@smolnar82 smolnar82 left a comment

Choose a reason for hiding this comment

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

LGTM, only small changes are required.

@smolnar82
Copy link
Contributor

One more general ask: instead of using screenshots of text blocks (configuration, logs, etc...) in the description or comments, please use the 'text block' representation of GitHub PRs. The way it's impossible to copy-paste that content if I wanted to use it. For instance, instead of using this screenshot
Screenshot 2022-07-06 at 15 18 57
Please do this:

    <property>
        <name>gateway.httpclient.connectionTimeout</name>
        <value>8m</value>
    </property>
    <property>
        <name>gateway.httpclient.socketTimeout</name>
        <value>10m</value>
    </property>

… logs so that the default value is also logged
Copy link
Contributor

@smolnar82 smolnar82 left a comment

Choose a reason for hiding this comment

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

LGTM

@smolnar82 smolnar82 merged commit 40139df into apache:master Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants