Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

added JestHttpClient logging information #124

Merged
merged 4 commits into from
Jun 10, 2014
Merged

added JestHttpClient logging information #124

merged 4 commits into from
Jun 10, 2014

Conversation

happyprg
Copy link
Contributor

@happyprg happyprg commented Jun 2, 2014

hello.
i use RoundrobinServerList by JestHttpClient.

normally, i think we need to know to below informations for the debugging.

  1. elasticSearchRestUrl and method info.
  2. requestBody as json string.

specially, when you use RoundRobinServer Method.
clearly need to know that information.
because according to the roundRobin method, change host at the call each time.

so i added logging information.
how are you thinking?

would you please to accept my pull request?
thank you.

1. reuqest method and url.
2. request body to jsonString.
@happyprg happyprg changed the title added JestHttpClient logging added JestHttpClient logging information Jun 2, 2014
@kramer
Copy link
Member

kramer commented Jun 2, 2014

They should probably be on debug level instead of info as it'll get too chatty otherwise 😄

@happyprg
Copy link
Contributor Author

happyprg commented Jun 2, 2014

ok. i changed logging level to debug.
is it ok?


JsonElement el = new JsonParser().parse(entity);

log.debug("request body to json - " + System.getProperty("line.separator") + new GsonBuilder().setPrettyPrinting().create().toJson(el));
Copy link
Member

Choose a reason for hiding this comment

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

Should not create a new Gson instance and use the one set on the class instead (getGson()). Also why not just print entity; yes it won't be pretty printed but that is the actual entity and doing a json parse for the second time here is kinda wasteful.

@happyprg
Copy link
Contributor Author

happyprg commented Jun 2, 2014

If you are using prettyPrint.
I think that you would help make it easier to read.

@happyprg
Copy link
Contributor Author

happyprg commented Jun 2, 2014

what do you think,
change to modifier of JestHttpClient.createJsonStringEntity.
from private to protected

@happyprg
Copy link
Contributor Author

happyprg commented Jun 2, 2014

after that,
possible below code.
e.g)
@OverRide
protected String createJsonStringEntity(Object data) {

    String createJsonStringEntity = super.createJsonStringEntity(data);

    JsonElement el = new JsonParser().parse(createJsonStringEntity);
    log.info("request body to json - " + System.getProperty("line.separator") + new GsonBuilder().setPrettyPrinting().create().toJson(el));

    return createJsonStringEntity;
}

}

2.method modifier changed.
- JestHttpClient.createJsonStringEntity(from private to protected)
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling 406a1a3 on happyprg:master into 48eeefe on searchbox-io:master.

kramer pushed a commit that referenced this pull request Jun 10, 2014
added JestHttpClient debug logging information
@kramer kramer merged commit f720b73 into searchbox-io:master Jun 10, 2014
@kramer kramer added this to the 0.1.2 milestone Jul 10, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants