diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000..5037c5c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,28 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: bug +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**Steps to Reproduce** +Steps to reproduce the behavior: +1. ... +2. ... + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Actual behavior** +What actually happened. + +**Dependency versions** +Include version info of the following libraries: client-java, logger-java-log4j + +**Additional context** +Add any other context about the problem here. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000..d217f3c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,20 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: '' +labels: enhancement +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the issue is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 037727a..3b2f077 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,3 +47,6 @@ jobs: - name: Build with Gradle run: ./gradlew build + + - name: Codecov upload + run: bash <(curl -s https://codecov.io/bash) diff --git a/CHANGELOG.md b/CHANGELOG.md index a444229..5226995 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ # Changelog ## [Unreleased] +### Added +- Logging issues skip, by @HardNorth +### Changed +- Client version updated on [5.1.11](https://github.com/reportportal/client-java/releases/tag/5.1.11), by @HardNorth +- Log4j version updated on 2.17.2, by @HardNorth +### Fixed +- A bug with wrong object casting, by @jusski ## [5.1.4] ### Changed diff --git a/README.md b/README.md index 94c3dbc..ddf127d 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,9 @@ # log4j-integration [![Maven Central](https://img.shields.io/maven-central/v/com.epam.reportportal/logger-java-log4j.svg?label=Maven%20Central)](https://search.maven.org/search?q=g:%22com.epam.reportportal%22%20AND%20a:%22logger-java-log4j%22) +[![CI Build](https://github.com/reportportal/logger-java-log4j/actions/workflows/ci.yml/badge.svg)](https://github.com/reportportal/logger-java-log4j/actions/workflows/ci.yml) +[![codecov](https://codecov.io/gh/reportportal/logger-java-log4j/branch/develop/graph/badge.svg?token=iEy7fURz1P)](https://codecov.io/gh/reportportal/logger-java-log4j) [![Join Slack chat!](https://reportportal-slack-auto.herokuapp.com/badge.svg)](https://reportportal-slack-auto.herokuapp.com) [![stackoverflow](https://img.shields.io/badge/reportportal-stackoverflow-orange.svg?style=flat)](http://stackoverflow.com/questions/tagged/reportportal) -[![UserVoice](https://img.shields.io/badge/uservoice-vote%20ideas-orange.svg?style=flat)](https://rpp.uservoice.com/forums/247117-report-portal) [![Build with Love](https://img.shields.io/badge/build%20with-❤%EF%B8%8F%E2%80%8D-lightgrey.svg)](http://reportportal.io?style=flat) [![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0) diff --git a/build.gradle b/build.gradle index 0ec51f4..c3bf0fe 100644 --- a/build.gradle +++ b/build.gradle @@ -19,18 +19,55 @@ apply plugin: 'java-library' apply from: "${project.scripts_url}/${project.scripts_branch}/build-quality.gradle" apply from: "${project.scripts_url}/${project.scripts_branch}/release-commons.gradle" apply from: "${project.scripts_url}/${project.scripts_branch}/signing.gradle" +apply from: "${project.scripts_url}/${project.scripts_branch}/jacoco.gradle" + +project.ext.limits = [ + 'instruction': 60, + 'branch' : 53, + 'line' : 65, + 'complexity' : 45, + 'method' : 49, + 'class' : 90 +] repositories { mavenCentral() + maven { url "https://jitpack.io" } } dependencies { - implementation 'org.apache.logging.log4j:log4j-core:2.17.1' + implementation 'org.apache.logging.log4j:log4j-core:2.17.2' implementation 'log4j:log4j:1.2.17' - implementation 'com.epam.reportportal:client-java:5.1.4' + implementation 'com.epam.reportportal:client-java:5.1.11' implementation 'com.epam.reportportal:commons-model:5.0.0' + + testImplementation 'com.github.reportportal:agent-java-test-utils:236a68c' + testImplementation('org.awaitility:awaitility:4.0.2') { + exclude group: 'org.hamcrest' + } + testImplementation ("org.junit.platform:junit-platform-runner:${junit_runner_version}") { + exclude module: 'junit' + } + testImplementation "org.junit.jupiter:junit-jupiter-engine:${junit_version}" + testImplementation "org.junit.jupiter:junit-jupiter-params:${junit_version}" + testImplementation 'org.hamcrest:hamcrest-core:2.2' + testImplementation "org.mockito:mockito-core:${project.mockito_version}" + testImplementation "org.mockito:mockito-junit-jupiter:${project.mockito_version}" + testImplementation 'org.apache.commons:commons-io:1.3.2' +} + +test { + useJUnitPlatform() + systemProperty("file.encoding", "utf-8") + outputs.upToDateWhen { false } + testLogging { + events "failed" + exceptionFormat "full" + } } wrapper { gradleVersion = '5.4.1' } + +build.dependsOn jacocoTestReport diff --git a/gradle.properties b/gradle.properties index fe1b04c..206a3c7 100644 --- a/gradle.properties +++ b/gradle.properties @@ -2,3 +2,7 @@ version=5.1.5-SNAPSHOT description=EPAM Report portal. Log4j Intergration scripts_url=https://raw.githubusercontent.com/reportportal/gradle-scripts scripts_branch=develop +junit_version=5.6.3 +junit_runner_version=1.6.3 +mockito_version=3.3.3 +excludeTests= diff --git a/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppender.java b/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppender.java index 8b0c20d..fec8c32 100644 --- a/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppender.java +++ b/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppender.java @@ -44,6 +44,11 @@ protected void append(final LoggingEvent event) { return; } + //make sure we are not logging themselves + if (Util.isInternal(event.getLoggerName())) { + return; + } + emitLog(itemUuid -> { SaveLogRQ request = new SaveLogRQ(); request.setLevel(event.getLevel().toString()); diff --git a/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2Appender.java b/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2Appender.java index 53ff877..969a2c7 100644 --- a/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2Appender.java +++ b/src/main/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2Appender.java @@ -55,8 +55,8 @@ protected ReportPortalLog4j2Appender(String name, Filter filter, Layout layout) { + public static ReportPortalLog4j2Appender createAppender(@PluginAttribute("name") String name, + @PluginElement("filter") Filter filter, @PluginElement("layout") Layout layout) { if (name == null) { LOGGER.error("No name provided for ReportPortalLog4j2Appender"); @@ -78,6 +78,11 @@ public void append(final LogEvent logEvent) { return; } + //make sure we are not logging themselves + if (Util.isInternal(event.getLoggerName())) { + return; + } + emitLog(itemUuid -> { SaveLogRQ request = new SaveLogRQ(); request.setItemUuid(itemUuid); @@ -99,7 +104,7 @@ public void append(final LogEvent logEvent) { byteSource = rpMessage.getData(); message = rpMessage.getMessage(); } else if (objectMessage instanceof File) { - final File file = (File) event.getMessage(); + final File file = (File) objectMessage; byteSource = new TypeAwareByteSource(asByteSource(file), detect(file)); message = "File reported"; diff --git a/src/main/java/com/epam/ta/reportportal/log4j/appender/Util.java b/src/main/java/com/epam/ta/reportportal/log4j/appender/Util.java index ed02d8f..16b9102 100644 --- a/src/main/java/com/epam/ta/reportportal/log4j/appender/Util.java +++ b/src/main/java/com/epam/ta/reportportal/log4j/appender/Util.java @@ -18,14 +18,31 @@ import com.epam.reportportal.message.HashMarkSeparatedMessageParser; import com.epam.reportportal.message.MessageParser; +import java.util.Collections; +import java.util.List; + /** * @author Andrei Varabyeu */ final class Util { - static final MessageParser MESSAGE_PARSER = new HashMarkSeparatedMessageParser(); + private static final List LOGGING_ISSUE = Collections.singletonList( + "com.epam.reportportal.service.logs.LoggingSubscriber"); private Util() { //statics only } + + static boolean isInternal(String loggerName) { + if (null == loggerName) { + return false; + } + + for (String packagePrefix : LOGGING_ISSUE) { + if (loggerName.startsWith(packagePrefix)) { + return true; + } + } + return false; + } } diff --git a/src/test/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppenderTest.java b/src/test/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppenderTest.java new file mode 100644 index 0000000..729e4f1 --- /dev/null +++ b/src/test/java/com/epam/ta/reportportal/log4j/appender/ReportPortalAppenderTest.java @@ -0,0 +1,118 @@ +package com.epam.ta.reportportal.log4j.appender; + +import com.epam.reportportal.service.Launch; +import com.epam.reportportal.service.LoggingContext; +import com.epam.reportportal.service.ReportPortalClient; +import com.epam.reportportal.service.logs.LoggingSubscriber; +import com.epam.reportportal.util.test.CommonUtils; +import com.epam.ta.reportportal.ws.model.BatchSaveOperatingRS; +import io.reactivex.Maybe; +import io.reactivex.Scheduler; +import io.reactivex.schedulers.Schedulers; +import okhttp3.MediaType; +import okhttp3.MultipartBody; +import okhttp3.RequestBody; +import org.apache.commons.io.IOUtils; +import org.apache.log4j.Level; +import org.apache.log4j.Logger; +import org.apache.log4j.PatternLayout; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; + +import java.io.IOException; +import java.io.InputStream; +import java.util.List; +import java.util.concurrent.ExecutorService; + +import static java.util.Optional.ofNullable; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +public class ReportPortalAppenderTest { + @Mock + private ReportPortalClient client; + + private final ExecutorService executor = CommonUtils.testExecutor(); + private final Scheduler scheduler = Schedulers.from(executor); + + private static Logger createLoggerFor(Class clazz) { + PatternLayout py = new PatternLayout("%date %level [%thread] %logger{10} [%file:%line] %msg%n"); + ReportPortalAppender appender = new ReportPortalAppender(); + appender.setLayout(py); + + Logger logger = Logger.getLogger(clazz); + logger.addAppender(appender); + logger.setLevel(Level.DEBUG); + return logger; + } + + @SuppressWarnings("unchecked") + private static void mockBatchLogging(ReportPortalClient client) { + when(client.log(any(List.class))).thenReturn(Maybe.just(new BatchSaveOperatingRS())); + } + + @AfterEach + public void tearDown() { + CommonUtils.shutdownExecutorService(executor); + } + + @Test + @SuppressWarnings({ "unchecked", "ReactiveStreamsUnusedPublisher" }) + public void test_logger_append() { + mockBatchLogging(client); + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + Logger logger = createLoggerFor(Launch.class); + logger.info("test message"); + LoggingContext.complete(); + verify(client).log(any(List.class)); + } + + @Test + @SuppressWarnings({ "unchecked", "ReactiveStreamsUnusedPublisher" }) + public void test_logger_skip() { + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + Logger logger = createLoggerFor(LoggingSubscriber.class); + logger.info("test message"); + LoggingContext.complete(); + verify(client, timeout(100).times(0)).log(any(List.class)); + } + + @Test + @SuppressWarnings({ "unchecked", "ReactiveStreamsUnusedPublisher" }) + public void test_binary_file_message_encoding() throws IOException { + mockBatchLogging(client); + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + String message = "test message"; + Logger logger = createLoggerFor(this.getClass()); + byte[] content; + try (InputStream is = ofNullable(Thread.currentThread() + .getContextClassLoader() + .getResourceAsStream("pug/unlucky.jpg")).orElseThrow(() -> new IllegalStateException( + "Unable to find test image file"))) { + content = IOUtils.toByteArray(is); + } + logger.info(String.format("RP_MESSAGE#FILE#%s#%s", "src/test/resources/pug/unlucky.jpg", message)); + LoggingContext.complete(); + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(client).log(captor.capture()); + + List request = captor.getValue(); + assertThat(request, hasSize(2)); + + RequestBody jsonPart = request.get(0).body(); + MediaType jsonPartType = jsonPart.contentType(); + assertThat(jsonPartType, notNullValue()); + assertThat(jsonPartType.toString(), Matchers.startsWith("application/json")); + + RequestBody binaryPart = request.get(1).body(); + MediaType binaryPartType = binaryPart.contentType(); + assertThat(binaryPartType, notNullValue()); + assertThat(binaryPartType.toString(), equalTo("image/jpeg")); + assertThat(binaryPart.contentLength(), equalTo((long) content.length)); + } +} diff --git a/src/test/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2AppenderTest.java b/src/test/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2AppenderTest.java new file mode 100644 index 0000000..b188f85 --- /dev/null +++ b/src/test/java/com/epam/ta/reportportal/log4j/appender/ReportPortalLog4j2AppenderTest.java @@ -0,0 +1,172 @@ +package com.epam.ta.reportportal.log4j.appender; + +import com.epam.reportportal.message.ReportPortalMessage; +import com.epam.reportportal.service.Launch; +import com.epam.reportportal.service.LoggingContext; +import com.epam.reportportal.service.ReportPortalClient; +import com.epam.reportportal.service.logs.LoggingSubscriber; +import com.epam.reportportal.util.test.CommonUtils; +import com.epam.ta.reportportal.ws.model.BatchSaveOperatingRS; +import com.google.common.io.ByteSource; +import io.reactivex.Maybe; +import io.reactivex.Scheduler; +import io.reactivex.schedulers.Schedulers; +import okhttp3.MediaType; +import okhttp3.MultipartBody; +import okhttp3.RequestBody; +import org.apache.commons.io.IOUtils; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.Filter; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configurator; +import org.apache.logging.log4j.core.config.builder.api.AppenderComponentBuilder; +import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilder; +import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory; +import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration; +import org.apache.logging.log4j.message.ObjectMessage; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.util.List; +import java.util.concurrent.ExecutorService; + +import static java.util.Optional.ofNullable; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +public class ReportPortalLog4j2AppenderTest { + @Mock + private ReportPortalClient client; + + private final ExecutorService executor = CommonUtils.testExecutor(); + private final Scheduler scheduler = Schedulers.from(executor); + + @SuppressWarnings("resource") + private static Logger createLoggerFor(Class clazz) { + ConfigurationBuilder builder = ConfigurationBuilderFactory.newConfigurationBuilder(); + builder.setPackages("com.epam.ta.reportportal.log4j.appender"); + builder.setStatusLevel(Level.DEBUG); + builder.setConfigurationName("BuilderTest"); + AppenderComponentBuilder appenderBuilder = builder.newAppender("ReportPortalAppender", + "ReportPortalLog4j2Appender" + ); + appenderBuilder.add(builder.newLayout("PatternLayout") + .addAttribute("pattern", "%date %level [%thread] %logger{10} [%file:%line] %msg%n")); + appenderBuilder.add(builder.newFilter("ThresholdFilter", Filter.Result.ACCEPT, Filter.Result.NEUTRAL) + .addAttribute("level", Level.DEBUG)); + builder.add(appenderBuilder); + builder.add(builder.newRootLogger(Level.DEBUG).add(builder.newAppenderRef("ReportPortalAppender"))); + LoggerContext ctx = Configurator.initialize(builder.build()); + return ctx.getLogger(clazz); + } + + @SuppressWarnings("unchecked") + private static void mockBatchLogging(ReportPortalClient client) { + when(client.log(any(List.class))).thenReturn(Maybe.just(new BatchSaveOperatingRS())); + } + + @AfterEach + public void tearDown() { + CommonUtils.shutdownExecutorService(executor); + } + + @Test + @SuppressWarnings({ "unchecked", "ReactiveStreamsUnusedPublisher" }) + public void test_logger_append() { + mockBatchLogging(client); + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + Logger logger = createLoggerFor(Launch.class); + logger.info("test message"); + LoggingContext.complete(); + verify(client).log(any(List.class)); + } + + @Test + @SuppressWarnings({ "unchecked", "ReactiveStreamsUnusedPublisher" }) + public void test_logger_skip() { + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + Logger logger = createLoggerFor(LoggingSubscriber.class); + logger.info("test message"); + LoggingContext.complete(); + verify(client, timeout(100).times(0)).log(any(List.class)); + } + + @SuppressWarnings({ "unchecked" }) + private void verify_binary_logging(long contentLength) throws IOException { + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(client).log(captor.capture()); + + List request = captor.getValue(); + assertThat(request, hasSize(2)); + + RequestBody jsonPart = request.get(0).body(); + MediaType jsonPartType = jsonPart.contentType(); + assertThat(jsonPartType, notNullValue()); + assertThat(jsonPartType.toString(), Matchers.startsWith("application/json")); + + RequestBody binaryPart = request.get(1).body(); + MediaType binaryPartType = binaryPart.contentType(); + assertThat(binaryPartType, notNullValue()); + assertThat(binaryPartType.toString(), equalTo("image/jpeg")); + assertThat(binaryPart.contentLength(), equalTo(contentLength)); + } + + @Test + @SuppressWarnings({ "ReactiveStreamsUnusedPublisher" }) + public void test_binary_file_message_encoding() throws IOException { + mockBatchLogging(client); + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + String message = "test message"; + Logger logger = createLoggerFor(this.getClass()); + byte[] content; + try (InputStream is = ofNullable(Thread.currentThread() + .getContextClassLoader() + .getResourceAsStream("pug/unlucky.jpg")).orElseThrow(() -> new IllegalStateException( + "Unable to find test image file"))) { + content = IOUtils.toByteArray(is); + } + logger.info(String.format("RP_MESSAGE#FILE#%s#%s", "src/test/resources/pug/unlucky.jpg", message)); + LoggingContext.complete(); + verify_binary_logging(content.length); + } + + @Test + @SuppressWarnings({ "ReactiveStreamsUnusedPublisher" }) + public void test_reportportal_message_logging() throws IOException { + mockBatchLogging(client); + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + String messageText = "test message"; + Logger logger = createLoggerFor(this.getClass()); + byte[] content; + try (InputStream is = ofNullable(Thread.currentThread() + .getContextClassLoader() + .getResourceAsStream("pug/unlucky.jpg")).orElseThrow(() -> new IllegalStateException( + "Unable to find test image file"))) { + content = IOUtils.toByteArray(is); + } + ReportPortalMessage message = new ReportPortalMessage(ByteSource.wrap(content), "image/jpeg", messageText); + logger.info(new ObjectMessage(message)); + LoggingContext.complete(); + verify_binary_logging(content.length); + } + + @Test + @SuppressWarnings({ "ReactiveStreamsUnusedPublisher" }) + public void test_file_logging() throws IOException { + mockBatchLogging(client); + LoggingContext.init(Maybe.just("launch_uuid"), Maybe.just("item_uuid"), client, scheduler); + Logger logger = createLoggerFor(this.getClass()); + logger.info(new ObjectMessage(new File("src/test/resources/pug/unlucky.jpg"))); + LoggingContext.complete(); + verify_binary_logging(90404L); + } +} diff --git a/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension b/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension new file mode 100644 index 0000000..02593ef --- /dev/null +++ b/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension @@ -0,0 +1 @@ +org.mockito.junit.jupiter.MockitoExtension diff --git a/src/test/resources/junit-platform.properties b/src/test/resources/junit-platform.properties new file mode 100644 index 0000000..6efc0d5 --- /dev/null +++ b/src/test/resources/junit-platform.properties @@ -0,0 +1 @@ +junit.jupiter.extensions.autodetection.enabled=true diff --git a/src/test/resources/pug/unlucky.jpg b/src/test/resources/pug/unlucky.jpg new file mode 100644 index 0000000..91e8af0 Binary files /dev/null and b/src/test/resources/pug/unlucky.jpg differ