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

issues #413 : Debug mode which allows HTTP requests being sent to the… #450

Merged
merged 5 commits into from
Jun 4, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,12 @@
<version>2.18.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.github.stephenc.jcip</groupId>
<artifactId>jcip-annotations</artifactId>
<version>1.0-1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.squareup.retrofit2</groupId>
<artifactId>retrofit</artifactId>
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/influxdb/InfluxDB.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
*/
public interface InfluxDB {

/**
* The system property key to set the http logging level across the JVM.
* @see LogLevel for available values
*/
public static final String LOG_LEVEL_PROP = "org.influxdb.InfluxDB.logLevel";

/** Controls the level of logging of the REST layer. */
public enum LogLevel {
/** No logging. */
Expand Down
26 changes: 24 additions & 2 deletions src/main/java/org/influxdb/impl/InfluxDBImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public InfluxDBImpl(final String url, final String username, final String passwo
this.username = username;
this.password = password;
this.loggingInterceptor = new HttpLoggingInterceptor();
this.loggingInterceptor.setLevel(Level.NONE);
initLogLevel();
this.gzipRequestInterceptor = new GzipRequestInterceptor();
this.retrofit = new Retrofit.Builder()
.baseUrl(url)
Expand All @@ -105,7 +105,7 @@ public InfluxDBImpl(final String url, final String username, final String passwo
this.username = username;
this.password = password;
this.loggingInterceptor = new HttpLoggingInterceptor();
this.loggingInterceptor.setLevel(Level.NONE);
initLogLevel();
this.gzipRequestInterceptor = new GzipRequestInterceptor();
this.retrofit = new Retrofit.Builder()
.baseUrl(url)
Expand Down Expand Up @@ -739,4 +739,26 @@ public void dropRetentionPolicy(final String rpName, final String database) {
execute(this.influxDBService.postQuery(this.username, this.password,
Query.encode(queryBuilder.toString())));
}

private void initLogLevel() {
String value = System.getProperty(LOG_LEVEL_PROP);

LogLevel logLevel = LogLevel.NONE;

if (value != null) {
switch (value.toUpperCase()) {
case "BASIC":
logLevel = LogLevel.BASIC;
break;
case "HEADERS":
logLevel = LogLevel.HEADERS;
break;
case "FULL":
logLevel = LogLevel.FULL;
break;
default:
}
}
setLogLevel(logLevel);
}
}
53 changes: 53 additions & 0 deletions src/test/java/org/influxdb/InfluxDBLogLevelTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.influxdb;

import java.util.Arrays;
import java.util.List;
import java.util.Optional;

import org.influxdb.dto.Pong;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.platform.runner.JUnitPlatform;
import org.junit.runner.RunWith;

import net.jcip.annotations.NotThreadSafe;

/**
* Test the InfluxDBimpl log level setting from system property.
*
* @author hoan.le [at] bonitoo.io
*
*/
@RunWith(JUnitPlatform.class)
@NotThreadSafe
Copy link
Contributor

@fmachado fmachado May 25, 2018

Choose a reason for hiding this comment

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

Are you 100% sure this annotation will affect how JUnit works?

Anyway, instead of fixing the bad practice you introduced by making changes that may affect other tests you are here trying to do something that would make the tests not going in parallel? I don't get it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Update: I found later on your email the link for the specific configuration for the surefire plugin, so it will work.
But my question from the previous comment is still valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I follow the Surefire plugin document, please check
http://maven.apache.org/surefire/maven-surefire-plugin/examples/fork-options-and-parallel-execution.html

For tests that may change the global context, I think making it isolated and run in sequential is quite reasonable. If I've got you right, that means every test must be able to run in parallel ?

public class InfluxDBLogLevelTest {
@Test
public void testLogLevelProperties() {
String oldLogLevel = System.getProperty(InfluxDB.LOG_LEVEL_PROP);
try {
List<String> logLevels = Arrays.asList(null, "NONE", "BASIC", "HEADERS", "FULL", "abc");
logLevels.forEach(logLevel -> {
System.out.println("LogLevel = " + logLevel);
Optional.ofNullable(logLevel).ifPresent(value -> {
System.setProperty(InfluxDB.LOG_LEVEL_PROP, value);
});

try {
InfluxDB influxDB = TestUtils.connectToInfluxDB();
Pong result = influxDB.ping();
Assertions.assertNotNull(result);
influxDB.close();
} catch (Exception e) {
Assertions.fail(e);
}
});
} finally {
if (oldLogLevel == null) {
System.clearProperty(InfluxDB.LOG_LEVEL_PROP);
} else {
System.setProperty(InfluxDB.LOG_LEVEL_PROP, oldLogLevel);
}
}

}
}