Skip to content

Commit cf65279

Browse files
khmarbaisekhmarbaise
committed
[FIXES JENKINS-35002] Usage of logging framework instead of printStackTrace
 o Fixed jenkinsci#161, Fixed jenkinsci#113 o Added Logging API slf4j-api
1 parent 2be6714 commit cf65279

7 files changed

Lines changed: 95 additions & 6 deletions

File tree

‎ReleaseNotes.md‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@
44

55
### General Changes
66

7+
[Using Logging framework][issue-161]
8+
[Using Logging framework][issue-113]
9+
[Using Logging framework][jissue-35002]
10+
11+
Added slf4j-api as foundation for logging. Using
12+
log4j2-slf4j-impl in jenkins-client-it-docker for logging.
13+
As a user you can now decide which logging framework
14+
you would like to use.
15+
716
[Changed the structure and integrated Docker IT][issue-160]
817

918
### API Changes
@@ -423,6 +432,7 @@ TestReport testReport = mavenJob.getLastSuccessfulBuild().getTestReport();
423432
[issue-111]: https://github.com/jenkinsci/java-client-api/issues/111
424433
[issue-116]: https://github.com/jenkinsci/java-client-api/issues/116
425434
[issue-108]: https://github.com/jenkinsci/java-client-api/issues/108
435+
[issue-113]: https://github.com/jenkinsci/java-client-api/issues/113
426436
[issue-119]: https://github.com/jenkinsci/java-client-api/issues/119
427437
[issue-120]: https://github.com/jenkinsci/java-client-api/issues/120
428438
[issue-121]: https://github.com/jenkinsci/java-client-api/issues/121
@@ -439,7 +449,8 @@ TestReport testReport = mavenJob.getLastSuccessfulBuild().getTestReport();
439449
[issue-157]: https://github.com/jenkinsci/java-client-api/issues/157
440450
[issue-159]: https://github.com/jenkinsci/java-client-api/issues/159
441451
[issue-160]: https://github.com/jenkinsci/java-client-api/issues/160
452+
[issue-161]: https://github.com/jenkinsci/java-client-api/issues/161
442453
[pull-123]: https://github.com/jenkinsci/java-client-api/pull/123
443454
[pull-149]: https://github.com/jenkinsci/java-client-api/pull/149
444455
[pull-158]: https://github.com/jenkinsci/java-client-api/pull/158
445-
456+
[jissue-35002]: https://issues.jenkins-ci.org/browse/JENKINS-35002

‎jenkins-client-it-docker/pom.xml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,22 @@
5656
<artifactId>httpclient</artifactId>
5757
<scope>test</scope>
5858
</dependency>
59+
60+
<dependency>
61+
<groupId>org.slf4j</groupId>
62+
<artifactId>slf4j-api</artifactId>
63+
<scope>test</scope>
64+
</dependency>
65+
<dependency>
66+
<groupId>org.apache.logging.log4j</groupId>
67+
<artifactId>log4j-core</artifactId>
68+
<scope>test</scope>
69+
</dependency>
70+
<dependency>
71+
<groupId>org.apache.logging.log4j</groupId>
72+
<artifactId>log4j-slf4j-impl</artifactId>
73+
<scope>test</scope>
74+
</dependency>
5975
</dependencies>
6076
<build>
6177
<plugins>

‎jenkins-client-it-docker/src/test/resources/log4j2-test.xml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<Configuration>
3+
<ThresholdFilter level="debug"/>
4+
5+
<Appenders>
6+
<RollingFile
7+
name="RollingFile"
8+
fileName="target/test-app.log"
9+
filePattern="target/test-$${date:yyyy-MM}/test-app-%d{MM-dd-yyyy}-%i.log.gz">
10+
<PatternLayout>
11+
<Pattern>%d %p %c{1.} [%t] %m%n</Pattern>
12+
</PatternLayout>
13+
<Policies>
14+
<TimeBasedTriggeringPolicy/>
15+
<SizeBasedTriggeringPolicy size="250 MB"/>
16+
</Policies>
17+
</RollingFile>
18+
19+
</Appenders>
20+
21+
<Loggers>
22+
<Root level="debug">
23+
<AppenderRef ref="RollingFile"/>
24+
</Root>
25+
</Loggers>
26+
27+
</Configuration>

‎jenkins-client/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@
7676
<artifactId>jaxen</artifactId>
7777
</dependency>
7878

79+
<dependency>
80+
<groupId>org.slf4j</groupId>
81+
<artifactId>slf4j-api</artifactId>
82+
</dependency>
83+
7984
<!-- Testing -->
8085
<dependency>
8186
<groupId>junit</groupId>

‎jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import org.apache.http.client.HttpResponseException;
1818
import org.apache.http.entity.ContentType;
1919
import org.dom4j.DocumentException;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
2022

2123
import com.google.common.base.Function;
2224
import com.google.common.base.Optional;
@@ -44,6 +46,7 @@
4446
* The main starting point for interacting with a Jenkins server.
4547
*/
4648
public class JenkinsServer {
49+
private final Logger LOGGER = LoggerFactory.getLogger( getClass() );
4750

4851
private final JenkinsHttpClient client;
4952

@@ -91,6 +94,7 @@ public boolean isRunning() {
9194
client.get("/");
9295
return true;
9396
} catch (IOException e) {
97+
LOGGER.debug("isRunning()", e);
9498
return false;
9599
}
96100
}
@@ -253,6 +257,7 @@ public JobWithDetails getJob(FolderJob folder, String jobName) throws IOExceptio
253257

254258
return job;
255259
} catch (HttpResponseException e) {
260+
LOGGER.debug("getJob(folder={}, jobName={}) status={}", folder.getName(), jobName, e.getStatusCode());
256261
if (e.getStatusCode() == HttpStatus.SC_NOT_FOUND) {
257262
return null;
258263
}
@@ -276,6 +281,7 @@ public MavenJobWithDetails getMavenJob(FolderJob folder, String jobName) throws
276281

277282
return job;
278283
} catch (HttpResponseException e) {
284+
LOGGER.debug("getMavenJob(jobName={}) status={}", jobName, e.getStatusCode());
279285
if (e.getStatusCode() == HttpStatus.SC_NOT_FOUND) {
280286
return null;
281287
}
@@ -294,6 +300,7 @@ public Optional<FolderJob> getFolderJob(Job job) throws IOException {
294300
return Optional.of(folder);
295301
} catch (HttpResponseException e) {
296302
if (e.getStatusCode() == HttpStatus.SC_NOT_FOUND) {
303+
//TODO: Check if this is a good idea ? What about Optional.absent() ?
297304
return null;
298305
}
299306
throw e;
@@ -480,7 +487,7 @@ public void quietDown() throws IOException {
480487
try {
481488
client.get("/quietDown/");
482489
} catch (org.apache.http.client.ClientProtocolException e) {
483-
e.printStackTrace();
490+
LOGGER.error("quietDown()", e);
484491
}
485492

486493
}
@@ -494,7 +501,7 @@ public void cancelQuietDown() throws IOException {
494501
try {
495502
client.post("/cancelQuietDown/");
496503
} catch (org.apache.http.client.ClientProtocolException e) {
497-
e.printStackTrace();
504+
LOGGER.error("cancelQuietDown()", e);
498505
}
499506
}
500507

‎jenkins-client/src/main/java/com/offbytwo/jenkins/client/JenkinsHttpClient.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.apache.http.params.HttpParams;
3838
import org.apache.http.protocol.BasicHttpContext;
3939
import org.apache.http.util.EntityUtils;
40+
import org.slf4j.Logger;
41+
import org.slf4j.LoggerFactory;
4042

4143
import com.fasterxml.jackson.databind.ObjectMapper;
4244
import com.google.common.collect.Lists;
@@ -52,7 +54,8 @@
5254
import net.sf.json.JSONObject;
5355

5456
public class JenkinsHttpClient {
55-
57+
private final Logger LOGGER = LoggerFactory.getLogger( getClass() );
58+
5659
private static final int SO_TIMEOUT_IN_MILLISECONDS = 3000;
5760
private static final int CONNECTION_TIMEOUT_IN_MILLISECONDS = 500;
5861

@@ -86,6 +89,7 @@ public JenkinsHttpClient(URI uri, CloseableHttpClient client) {
8689
this.httpResponseValidator = new HttpResponseValidator();
8790
// this.contentExtractor = new HttpResponseContentExtractor();
8891
this.jenkinsVersion = null;
92+
LOGGER.debug("uri={}", uri.toString());
8993
}
9094

9195
/**
@@ -121,6 +125,7 @@ public JenkinsHttpClient(URI uri) {
121125
httpParams.setIntParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, CONNECTION_TIMEOUT_IN_MILLISECONDS);
122126

123127
this.httpResponseValidator = new HttpResponseValidator();
128+
LOGGER.debug("uri={}", uri.toString());
124129
}
125130

126131
/**
@@ -181,13 +186,15 @@ public String get(String path) throws IOException {
181186
HttpGet getMethod = new HttpGet(api(path));
182187
HttpResponse response = client.execute(getMethod, localContext);
183188
getJenkinsVersionFromHeader(response);
189+
LOGGER.debug("get({}), version={}, responseCode={}", path, this.jenkinsVersion, response.getStatusLine().getStatusCode());
184190
try {
185191
httpResponseValidator.validateResponse(response);
186192
return IOUtils.toString(response.getEntity().getContent());
187193
} finally {
188194
EntityUtils.consume(response.getEntity());
189195
releaseConnection(getMethod);
190196
}
197+
191198
}
192199

193200
/**
@@ -208,7 +215,8 @@ public <T extends BaseModel> T getQuietly(String path, Class<T> cls) {
208215
value = get(path, cls);
209216
return value;
210217
} catch (IOException e) {
211-
e.printStackTrace();
218+
LOGGER.debug("getQuietly({}, {})", path, cls.getName(), e);
219+
//TODO: Is returing null a good idea?
212220
return null;
213221
}
214222
}

‎pom.xml‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,22 @@
187187
<version>1.7.1</version>
188188
<scope>test</scope>
189189
</dependency>
190-
190+
191+
<dependency>
192+
<groupId>org.slf4j</groupId>
193+
<artifactId>slf4j-api</artifactId>
194+
<version>1.7.21</version>
195+
</dependency>
196+
<dependency>
197+
<groupId>org.apache.logging.log4j</groupId>
198+
<artifactId>log4j-core</artifactId>
199+
<version>2.5</version>
200+
</dependency>
201+
<dependency>
202+
<groupId>org.apache.logging.log4j</groupId>
203+
<artifactId>log4j-slf4j-impl</artifactId>
204+
<version>2.5</version>
205+
</dependency>
191206
<dependency>
192207
<groupId>xml-apis</groupId>
193208
<artifactId>xml-apis</artifactId>

0 commit comments

Comments
 (0)