diff --git a/AGENTS.md b/AGENTS.md index 361c46044..ae1e74b04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,23 +8,23 @@ LLM-based agents can accelerate development only if they respect our house rules ## Language & character-set policy -| Requirement | Rationale | -|--------------|-----------| -| **British English** spelling (`organisation`, `licence`, *not* `organization`, `license`) except technical US spellings like `synchronized` | Keeps wording consistent with Chronicle's London HQ and existing docs. See the University of Oxford style guide for reference. | -| **ASCII-7 only** (code-points 0-127). Avoid smart quotes, non-breaking spaces and accented characters. | ASCII-7 survives every toolchain Chronicle uses, incl. low-latency binary wire formats that expect the 8th bit to be 0. | -| If a symbol is not available in ASCII-7, use a textual form such as `micro-second`, `>=`, `:alpha:`, `:yes:`. This is the preferred approach and Unicode must not be inserted. | Extended or '8-bit ASCII' variants are *not* portable and are therefore disallowed. | +| Requirement | Rationale | +|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------| +| **British English** spelling (`organisation`, `licence`, *not* `organization`, `license`) except technical US spellings like `synchronized` | Keeps wording consistent with Chronicle's London HQ and existing docs. See the University of Oxford style guide for reference. | +| **ISO-8859-1** (code-points 0-255). Avoid smart quotes, non-breaking spaces and accented characters. | ISO-8859-1 survives every toolchain Chronicle uses, incl. low-latency binary wire formats that expect the 8th bit to be 0. | +| If a symbol is not available in ISO-8859-1, use a textual form such as `micro-second`, `>=`, `:alpha:`, `:yes:`. This is the preferred approach and Unicode must not be inserted. | Extended or '8-bit ASCII' variants are *not* portable and are therefore disallowed. | ## Javadoc guidelines **Goal:** Every Javadoc block should add information you cannot glean from the method signature alone. Anything else is noise and slows readers down. -| Do | Don't | -|----|-------| -| State *behavioural contracts*, edge-cases, thread-safety guarantees, units, performance characteristics and checked exceptions. | Restate the obvious ("Gets the value", "Sets the name"). | -| Keep the first sentence short; it becomes the summary line in aggregated docs. | Duplicate parameter names/ types unless more explanation is needed. | -| Prefer `@param` for *constraints* and `@throws` for *conditions*, following Oracle's style guide. | Pad comments to reach a line-length target. | -| Remove or rewrite autogenerated Javadoc for trivial getters/setters. | Leave stale comments that now contradict the code. | +| Do | Don't | +|---------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------| +| State *behavioural contracts*, edge-cases, thread-safety guarantees, units, performance characteristics and checked exceptions. | Restate the obvious ("Gets the value", "Sets the name"). | +| Keep the first sentence short; it becomes the summary line in aggregated docs. | Duplicate parameter names/ types unless more explanation is needed. | +| Prefer `@param` for *constraints* and `@throws` for *conditions*, following Oracle's style guide. | Pad comments to reach a line-length target. | +| Remove or rewrite autogenerated Javadoc for trivial getters/setters. | Leave stale comments that now contradict the code. | The principle that Javadoc should only explain what is *not* manifest from the signature is well-established in the wider Java community. @@ -55,12 +55,13 @@ mvn -q verify ## Project requirements -See the [Decision Log](adoc/decision-log.adoc) for the latest project decisions. -See the [Project Requirements](adoc/project-requirements.adoc) for details on project requirements. +See the [Decision Log](src/main/docs/decision-log.adoc) for the latest project decisions. +See the [Project Requirements](src/main/docs/project-requirements.adoc) for details on project requirements. ## Elevating the Workflow with Real-Time Documentation -Building upon our existing Iterative Workflow, the newest recommendation is to emphasise *real-time updates* to documentation. +Building upon our existing Iterative Workflow, the newest recommendation is to emphasise *real-time updates* to +documentation. Ensure the relevant `.adoc` files are updated when features, requirements, implementation details, or tests change. This tight loop informs the AI accurately and creates immediate clarity for all team members. @@ -75,41 +76,54 @@ This tight loop informs the AI accurately and creates immediate clarity for all ### Best Practices -* **Maintain Sync**: Keep documentation (AsciiDoc), tests, and code synchronised in version control. Changes in one area should prompt reviews and potential updates in the others. -* **Doc-First for New Work**: For *new* features or requirements, aim to update documentation first, then use AI to help produce or refine corresponding code and tests. For refactoring or initial bootstrapping, updates might flow from code/tests back to documentation, which should then be reviewed and finalised. -* **Small Commits**: Each commit should ideally relate to a single requirement or coherent change, making reviews easier for humans and AI analysis tools. -- **Team Buy-In**: Encourage everyone to review AI outputs critically and contribute to maintaining the synchronicity of all artefacts. +* **Maintain Sync**: Keep documentation (AsciiDoc), tests, and code synchronised in version control. Changes in one area + should prompt reviews and potential updates in the others. +* **Doc-First for New Work**: For *new* features or requirements, aim to update documentation first, then use AI to help + produce or refine corresponding code and tests. For refactoring or initial bootstrapping, updates might flow from + code/tests back to documentation, which should then be reviewed and finalised. +* **Small Commits**: Each commit should ideally relate to a single requirement or coherent change, making reviews easier + for humans and AI analysis tools. + +- **Team Buy-In**: Encourage everyone to review AI outputs critically and contribute to maintaining the synchronicity of + all artefacts. ## AI Agent Guidelines When using AI agents to assist with development, please adhere to the following guidelines: -* **Respect the Language & Character-set Policy**: Ensure all AI-generated content follows the British English and ASCII-7 guidelines outlined above. -Focus on Clarity: AI-generated documentation should be clear and concise and add value beyond what is already present in the code or existing documentation. -* **Avoid Redundancy**: Do not generate content that duplicates existing documentation or code comments unless it provides additional context or clarification. -* **Review AI Outputs**: Always review AI-generated content for accuracy, relevance, and adherence to the project's documentation standards before committing it to the repository. +* **Respect the Language & Character-set Policy**: Ensure all AI-generated content follows the British English and + ISO-8859-1 guidelines outlined above. + Focus on Clarity: AI-generated documentation should be clear and concise and add value beyond what is already present + in the code or existing documentation. +* **Avoid Redundancy**: Do not generate content that duplicates existing documentation or code comments unless it + provides additional context or clarification. +* **Review AI Outputs**: Always review AI-generated content for accuracy, relevance, and adherence to the project's + documentation standards before committing it to the repository. ## Company-Wide Tagging -This section records **company-wide** decisions that apply to *all* Chronicle projects. All identifiers use the --xxx prefix. The `xxx` are unique across in the same Scope even if the tags are different. Component-specific decisions live in their xxx-decision-log.adoc files. +This section records **company-wide** decisions that apply to *all* Chronicle projects. All identifiers use +the --xxx prefix. The `xxx` are unique across in the same Scope even if the tags are different. +Component-specific decisions live in their xxx-decision-log.adoc files. ### Tag Taxonomy (Nine-Box Framework) -To improve traceability, we adopt the Nine-Box taxonomy for requirement and decision identifiers. These tags are used in addition to the existing ALL prefix, which remains reserved for global decisions across every project. +To improve traceability, we adopt the Nine-Box taxonomy for requirement and decision identifiers. These tags are used in +addition to the existing ALL prefix, which remains reserved for global decisions across every project. .Adopt a Nine-Box Requirement Taxonomy -|Tag | Scope | Typical examples | -|----|-------|------------------| -|FN |Functional user-visible behaviour | Message routing, business rules | -|NF-P |Non-functional - Performance | Latency budgets, throughput targets | -|NF-S |Non-functional - Security | Authentication method, TLS version | -|NF-O |Non-functional - Operability | Logging, monitoring, health checks | -|TEST |Test / QA obligations | Chaos scenarios, benchmarking rigs | -|DOC |Documentation obligations | Sequence diagrams, user guides | -|OPS |Operational / DevOps concerns | Helm values, deployment checklist | -|UX |Operator or end-user experience | CLI ergonomics, dashboard layouts | -|RISK |Compliance / risk controls | GDPR retention, audit trail | +| Tag | Scope | Typical examples | +|------|-----------------------------------|-------------------------------------| +| FN | Functional user-visible behaviour | Message routing, business rules | +| NF-P | Non-functional - Performance | Latency budgets, throughput targets | +| NF-S | Non-functional - Security | Authentication method, TLS version | +| NF-O | Non-functional - Operability | Logging, monitoring, health checks | +| TEST | Test / QA obligations | Chaos scenarios, benchmarking rigs | +| DOC | Documentation obligations | Sequence diagrams, user guides | +| OPS | Operational / DevOps concerns | Helm values, deployment checklist | +| UX | Operator or end-user experience | CLI ergonomics, dashboard layouts | +| RISK | Compliance / risk controls | GDPR retention, audit trail | `ALL-*` stays global, case-exact tags. Pick one primary tag if multiple apply. diff --git a/README.adoc b/README.adoc index 32bd0463b..148bef8e2 100644 --- a/README.adoc +++ b/README.adoc @@ -12,7 +12,7 @@ image:https://img.shields.io/github/license/OpenHFT/Chronicle-Logger[GitHub] image:https://img.shields.io/badge/release%20notes-subscribe-brightgreen[link="https://chronicle.software/release-notes/"] image:https://sonarcloud.io/api/project_badges/measure?project=OpenHFT_Chronicle-Logger&metric=alert_status[link="https://sonarcloud.io/dashboard?id=OpenHFT_Chronicle-Logger"] -image::docs/images/Logger_line.png[width=20%] +image::src/main/docs/images/Logger_line.png[width=20%] toc::[] @@ -21,16 +21,16 @@ toc::[] === Version [#image-maven] -[caption="", link=https://maven-badges.herokuapp.com/maven-central/net.openhft/chronicle-logger] +[caption="",link=https://maven-badges.herokuapp.com/maven-central/net.openhft/chronicle-logger] image::https://maven-badges.herokuapp.com/maven-central/net.openhft/chronicle-logger/badge.svg[] == Overview -Today most programs require the logging of large amounts of data, especially in trading systems where this is a -regulatory requirement. Loggers can affect your system performance, therefore logging is sometimes kept to a minimum. +Today most programs require the logging of large amounts of data, especially in trading systems where this is a regulatory requirement. +Loggers can affect your system performance, therefore logging is sometimes kept to a minimum. With Chronicle Logger we aim to minimize logging overhead, freeing your system to focus on the business logic. -Chronicle logger supports most of the standard logging API’s including: +Chronicle logger supports most of the standard logging API’s including: * <> * <> @@ -39,24 +39,24 @@ Chronicle logger supports most of the standard logging API’s including: * <> * <> -Chronicle Logger is able to aggregate all your logs to a central store. It has built-in resilience, so you will never -lose messages. +Chronicle Logger is able to aggregate all your logs to a central store. +It has built-in resilience, so you will never lose messages. -At the moment, Chronicle Logger only supports binary logs, which is beneficial for write speed but requires extra tools -to read them. We provide some basic #tools[tools] for that and an API to develop your own. +At the moment, Chronicle Logger only supports binary logs, which is beneficial for write speed but requires extra tools to read them. +We provide some basic #tools[tools] for that and an API to develop your own. NOTE: `chronicle-logger-log4j-2` does not suffer from the log4shell vulnerability as it does not reuse the Log4J2 message formatting machinery == How it works -Chronicle Logger is built on top of Chronicle Queue. It provides multiple logging frameworks' adapters and is a low latency, -high throughput synchronous writer. Unlike asynchronous writers, you will always see the last message before -the application dies, as usually it is the last message that is the most valuable. +Chronicle Logger is built on top of Chronicle Queue. +It provides multiple logging frameworks' adapters and is a low latency, high throughput synchronous writer. +Unlike asynchronous writers, you will always see the last message before the application dies, as usually it is the last message that is the most valuable. == Performance -We have run a benchmark to compare Chronicle Logger with normal file appender of Log4J2 (the quickest of mainstream -logging frameworks). Results below: +We have run a benchmark to compare Chronicle Logger with normal file appender of Log4J2 (the quickest of mainstream logging frameworks). +Results below: |=== |*Benchmark* |*Mode*|*Samples*|*Score*|*Score error*|*Units* @@ -68,6 +68,7 @@ logging frameworks). Results below: |=== Test Hardware: + [source] ---- Intel Core i7-6700K @@ -77,10 +78,9 @@ Intel Core i7-6700K == Bindings -All config files for bindings support limited variable interpolation where the variables are replaced with the -corresponding values from the same configuration file or the system properties. We have one predefined variable, `pid`, -so `${pid}` will replaced by current process id. System properties have the precedence in placeholder replacement -so they can be overriden. +All config files for bindings support limited variable interpolation where the variables are replaced with the corresponding values from the same configuration file or the system properties. +We have one predefined variable, `pid`, so `${pid}` will replaced by current process id. +System properties have the precedence in placeholder replacement so they can be overriden. The following can be configured for each logger: @@ -101,30 +101,28 @@ If set, these will override the default Chronicle Queue configuration. _Use with *Please Note* - * Loggers are not hierarchically grouped so `my.domain.package.MyClass1` and `my.domain` are two distinct entities. - * The `path` is used to track the underlying Chronicle Queue so having two loggers configured with the same `path` is unsupported +* Loggers are not hierarchically grouped so `my.domain.package.MyClass1` and `my.domain` are two distinct entities. +* The `path` is used to track the underlying Chronicle Queue so having two loggers configured with the same `path` is unsupported === chronicle-logger-slf4j The chronicle-logger-slf4j is an implementation of SLF4J API > 1.7.x. -To configure this sl4j binding you need to specify the location of a properties files (file-system or classpath) -via system properties: +To configure this sl4j binding you need to specify the location of a properties files (file-system or classpath) via system properties: [source] ---- -Dchronicle.logger.properties=${pathToYourPropertiesFile} ---- -Alternatively, you could use one of the default locations: `chronicle-logger.properties` +Alternatively, you could use one of the default locations: `chronicle-logger.properties` or `config/chronicle-logger.properties` located in the classpath. -The default configuration is build using properties with `chronicle.logger.root` as prefix but you can also set -per-logger settings i.e. `chronicle.logger.L1.*` +The default configuration is build using properties with `chronicle.logger.root` as prefix but you can also set per-logger settings i.e. `chronicle.logger.L1.*` ==== Config Example -[source, properties] +[source,properties] ---- # shared properties chronicle.base = ${java.io.tmpdir}/chronicle-logs/${pid} @@ -147,8 +145,8 @@ chronicle.logger.L1.level = info The chronicle-logger-logback module provides appender for Logback: `net.openhft.chronicle.logger.logback.ChronicleAppender` ==== Config Example - -[source, xml] + +[source,xml] ---- @@ -171,7 +169,7 @@ We provide log4j1 appender `net.openhft.chronicle.logger.log4j1.ChronicleAppende ==== Config Example -[source, xml] +[source,xml] ---- @@ -228,12 +226,12 @@ We provide log4j1 appender `net.openhft.chronicle.logger.log4j1.ChronicleAppende === chronicle-logger-log4j-2 -Use `` element in `` to create Chronicle appender. Optional `` element can be -used to tweak underlying Chronicle Queue. +Use `` element in `` to create Chronicle appender. +Optional `` element can be used to tweak underlying Chronicle Queue. ==== Config Example -[source, xml] +[source,xml] ---- @@ -289,7 +287,7 @@ Use `net.openhft.chronicle.logger.jul.ChronicleHandler` as a handler ==== Config Example -[source, properties] +[source,properties] ---- handlers=java.util.logging.ConsoleHandler, net.openhft.chronicle.logger.jul.ChronicleHandler @@ -311,19 +309,19 @@ chronicle.useParentHandlers=false === chronicle-logger-jcl -Similar to slf4j, to configure this binding you need to specify the location of a properties files (file-system or classpath) -via system properties: +Similar to slf4j, to configure this binding you need to specify the location of a properties files (file-system or classpath) via system properties: + [source] ---- -Dchronicle.logger.properties=${pathToYourPropertiesFile} ---- -Alternatively, you could use one of the default locations: `chronicle-logger.properties` +Alternatively, you could use one of the default locations: `chronicle-logger.properties` or `config/chronicle-logger.properties` located in the classpath. ==== Config Example -[source, properties] +[source,properties] ---- chronicle.logger.base = ${java.io.tmpdir}/chronicle-jcl chronicle.logger.root.path = ${chronicle.logger.base}/root @@ -337,14 +335,14 @@ chronicle.logger.logger_1.level = info === Tools * `net.openhft.chronicle.logger.tools.ChroniCat` - tool to dump log contents to STDOUT + [source] --- ChroniCat [-w ] - wire format, default BINARY_LIGHT - base path of Chronicle Logs storage -mvn exec:java -Dexec.mainClass="net.openhft.chronicle.logger.tools.ChroniCat" -Dexec.args="..." ---- +mvn exec:java -Dexec.mainClass="net.openhft.chronicle.logger.tools.ChroniCat" -Dexec.args="..." --- * `net.openhft.chronicle.logger.tools.ChroniTail` - same as ChroniCat but waits for more data, similar to *nix `tail` utility @@ -357,6 +355,5 @@ ChroniTail [-w ] mvn exec:java -Dexec.mainClass="net.openhft.chronicle.logger.tools.ChroniTail" -Dexec.args="..." ---- -* We also provide generic interface to interact with logs, `net.openhft.chronicle.logger.tools.ChronicleLogReader`, -allowing arbitrary operations with decoded log lines. Please refer to javadocs. - +* We also provide generic interface to interact with logs, `net.openhft.chronicle.logger.tools.ChronicleLogReader`, allowing arbitrary operations with decoded log lines. +Please refer to javadocs. diff --git a/adoc/project-requirements.adoc b/adoc/project-requirements.adoc deleted file mode 100644 index c39ca5037..000000000 --- a/adoc/project-requirements.adoc +++ /dev/null @@ -1,69 +0,0 @@ -= Functional Requirements for Chronicle Logger -:toc: -:lang: en-GB - -== Introduction -Chronicle Logger is a multi-module Java project providing adapters for various logging frameworks on top of Chronicle Queue. -This document outlines the functional requirements for all modules within this repository. - -== General Requirements - -- Provide high performance log persistence via Chronicle Queue. -- Support storing log messages in binary format to maximize throughput. -- Allow configuration of log path, log level and Chronicle Queue settings via properties or XML. -- Ensure consistent API and configuration approach across all bindings. -- Include tools for reading log messages from Chronicle Queue. - -== Modules - -=== logger-core -- Contains core interfaces and implementations used by all other modules. -- Provides `ChronicleLogWriter`, `ChronicleLogManager` and related configuration classes. -- Must expose a simple builder-based API to create loggers programmatically. -- Supplies a default implementation for writing log events to Chronicle Queue. - -=== logger-jul -- Implements a `java.util.logging.Handler` writing to Chronicle Queue. -- Must integrate with standard JUL configuration files. -- Provides a factory to create handlers based on properties. - -=== logger-jcl -- Provides a bridge for Apache Commons Logging. -- Supports configuration via properties file specified through the `chronicle.logger.properties` system property or default locations. - -=== logger-slf4j -- Implements the SLF4J 1.x binding. -- Exposes `ChronicleLoggerFactory` to create loggers. -- Must respect standard SLF4J configuration via properties files. - -=== logger-slf4j-2 -- Adds SLF4J 2.x API support. -- Follows the same configuration strategy as `logger-slf4j`. - -=== logger-logback -- Supplies a Logback `Appender` that writes events to Chronicle Queue. -- Accepts an optional `` section to tune queue parameters. - -=== logger-log4j-1 -- Implements a Log4J 1.2 `Appender`. -- Configuration is performed via XML as part of the log4j configuration file. - -=== logger-log4j-2 -- Provides an `Appender` for Log4J 2. -- Configuration is done using the `` XML element, optionally extended with ``. - -=== logger-tools -- Contains command line utilities to inspect and tail Chronicle log files. -- Provides the `ChroniCat` and `ChroniTail` tools as well as a generic `ChronicleLogReader` API. - -=== benchmark -- Includes micro benchmarks to assess logging performance. -- Uses JMH and example configuration files to measure throughput under different scenarios. - -== Non-functional Requirements -- All modules must compile on Java 8 or later. -- Logging operations should minimize latency and avoid unnecessary allocations. -- Binary log format must remain compatible across modules. - -== Conclusion -The Chronicle Logger repository delivers unified high performance logging for multiple logging frameworks. Each module must conform to the requirements listed above to provide a consistent and reliable logging solution. diff --git a/benchmark/dependency-reduced-pom.xml b/benchmark/dependency-reduced-pom.xml new file mode 100644 index 000000000..7be57c2ce --- /dev/null +++ b/benchmark/dependency-reduced-pom.xml @@ -0,0 +1,241 @@ + + + + chronicle-logger + net.openhft + 4.27ea2-SNAPSHOT + + 4.0.0 + chronicle-logger-benchmark + JMH Benchmark for Log4J2 + + + + maven-shade-plugin + + + package + + shade + + + ${uberjar.name} + + + org.openjdk.jmh.Main + + + + + *:* + + META-INF/*.SF + META-INF/*.DSA + META-INF/*.RSA + + + + + + + + + + + + code-review + + + + maven-checkstyle-plugin + ${checkstyle.version} + + + checkstyle + verify + + check + + + + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + spotbugs + verify + + check + + + + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + 0.0 + 0.0 + + + + + + org.openjdk.jmh + jmh-generator-annprocess + 1.36 + provided + + + org.jetbrains + annotations + 24.0.1 + provided + + + org.apache.commons + commons-lang3 + 3.12.0 + test + + + junit + junit + 4.13.2 + test + + + hamcrest-core + org.hamcrest + + + + + org.openjdk.jmh + jmh-core-benchmarks + 1.36 + test + + + + + + net.openhft + chronicle-logger-log4j-2 + ${project.version} + + + + + 1.36 + benchmarks + UTF-8 + 1.8 + + diff --git a/benchmark/pom.xml b/benchmark/pom.xml index ecaf8fe3b..27bd46693 100644 --- a/benchmark/pom.xml +++ b/benchmark/pom.xml @@ -23,7 +23,8 @@ or visit www.oracle.com if you need additional information or have any questions. --> - + 4.0.0 @@ -118,7 +119,8 @@ questions. ${uberjar.name} - + org.openjdk.jmh.Main @@ -143,4 +145,156 @@ questions. + + + + code-review + + false + + + 0.0 + 0.0 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/benchmark/src/main/config/checkstyle.xml b/benchmark/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/benchmark/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/benchmark/src/main/config/pmd-exclude.properties b/benchmark/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..ecc41bca1 --- /dev/null +++ b/benchmark/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger benchmark module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/benchmark/src/main/config/spotbugs-exclude.xml b/benchmark/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..ff4c99699 --- /dev/null +++ b/benchmark/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/benchmark/src/main/resources/log4j2.xml b/benchmark/src/main/resources/log4j2.xml index 61dc303be..5a1050e91 100644 --- a/benchmark/src/main/resources/log4j2.xml +++ b/benchmark/src/main/resources/log4j2.xml @@ -34,7 +34,9 @@ - %d [%t] %p %c - %m %ex%n + + %d [%t] %p %c - %m %ex%n + diff --git a/logger-core/pom.xml b/logger-core/pom.xml index 38f60ad5d..007001432 100644 --- a/logger-core/pom.xml +++ b/logger-core/pom.xml @@ -18,7 +18,8 @@ ~ limitations under the License. ~ --> - + 4.0.0 @@ -108,4 +109,154 @@ + + + + code-review + + false + + + 0.7044 + 0.4750 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-core/src/main/config/checkstyle.xml b/logger-core/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-core/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-core/src/main/config/pmd-exclude.properties b/logger-core/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..279855de0 --- /dev/null +++ b/logger-core/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-core module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-core/src/main/config/spotbugs-exclude.xml b/logger-core/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..708884822 --- /dev/null +++ b/logger-core/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,31 @@ + + + + + + + + + CLG-FN-002: append=false queues reuse files until explicit clear is implemented. + + + + + + + + CLG-FN-002 allows operators to point at explicit configuration files. + + + + + + + + CLG-FN-003: ChronicleQueue lifecycle is intentionally managed by the caller. + + + diff --git a/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogConfig.java b/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogConfig.java index 7f8da6c97..d87925492 100644 --- a/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogConfig.java +++ b/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogConfig.java @@ -15,6 +15,8 @@ */ package net.openhft.chronicle.logger; +import edu.umd.cs.findbugs.annotations.CheckForNull; + import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -27,24 +29,24 @@ /** * Reads logger settings from a properties file. - * + *

* The loader checks the system property {@code chronicle.logger.properties} and * then the files {@code chronicle-logger.properties} and * {@code config/chronicle-logger.properties} on the class path. Each value may * contain {@code ${name}} placeholders that reference other keys or system * properties. The token {@code ${pid}} expands to the process id. - * + *

* Configuration example: - * + *

* # default * chronicle.logger.base = ${java.io.tmpdir}/chronicle/${pid} - * + *

* # logger : root * chronicle.logger.root.path = ${chronicle.logger.base}/root * chronicle.logger.root.level = debug * chronicle.logger.root.shortName = false * chronicle.logger.root.append = false - * + *

* # logger : Logger1 * chronicle.logger.Logger1.path = ${chronicle.logger.base}/logger_1 * chronicle.logger.Logger1.level = info @@ -81,7 +83,7 @@ private ChronicleLogConfig(final Properties properties, final LogAppenderConfig /** * Builds a configuration from the supplied properties. - * Placeholders are resolved in the same manner as when reading from a file. + * Callers are responsible for resolving any placeholders in advance. * * @param properties key value pairs following the chronicle logger scheme * @return parsed configuration @@ -104,23 +106,20 @@ public static ChronicleLogConfig load(final Properties properties) { public static ChronicleLogConfig load(String cfgPath) { try { return load(getConfigurationStream(cfgPath)); - } catch (Exception e) { - // is printing stack trace and falling through really the right thing - // to do here, or should it throw out? - e.printStackTrace(); + } catch (IOException e) { + System.err.println("Unable to load chronicle-logger configuration."); + return null; } - - return null; } private static ChronicleLogConfig load(InputStream in) { if (in != null) { Properties properties = new Properties(); - try { - properties.load(in); - in.close(); + try (InputStream input = in) { + properties.load(input); } catch (IOException ignored) { + System.err.println("Unable to load chronicle-logger configuration."); } return load(interpolate(properties)); @@ -128,10 +127,7 @@ private static ChronicleLogConfig load(InputStream in) { } else { System.err.printf( "Unable to configure chronicle-logger:" - + " configuration file not found in default locations (%s)" - + " or System property (%s) is not defined \n", - DEFAULT_CFG_LOCATIONS.toString(), - KEY_PROPERTIES_FILE); + + " configuration file not found in default locations or system property is not defined.%n"); } return null; @@ -161,10 +157,8 @@ public static ChronicleLogConfig load() { if (is != null) { return load(is); } - } catch (Exception e) { - // is printing stack trace and falling through really the right thing - // to do here, or should it throw out? - e.printStackTrace(); + } catch (IOException e) { + System.err.println("Unable to load chronicle-logger configuration."); } return null; @@ -239,7 +233,7 @@ private static LogAppenderConfig loadAppenderConfig(final Properties properties) // ************************************************************************* public LogAppenderConfig getAppenderConfig() { - return this.appenderConfig; + return this.appenderConfig.copy(); } public String getString(final String shortName) { @@ -258,23 +252,26 @@ public String getString(final String loggerName, final String shortName) { return val; } + @CheckForNull public Boolean getBoolean(final String shortName) { - return getBoolean(shortName, null); + String prop = getString(shortName); + return prop != null ? Boolean.valueOf(Boolean.parseBoolean(prop)) : null; } public Boolean getBoolean(final String shortName, boolean defval) { String prop = getString(shortName); - return (prop != null) ? "true".equalsIgnoreCase(prop) : defval; + return Boolean.valueOf(prop != null ? Boolean.parseBoolean(prop) : defval); } + @CheckForNull public Boolean getBoolean(final String loggerName, final String shortName) { String prop = getString(loggerName, shortName); - return (prop != null) ? "true".equalsIgnoreCase(prop) : null; + return prop != null ? Boolean.valueOf(Boolean.parseBoolean(prop)) : null; } public Boolean getBoolean(final String loggerName, final String shortName, boolean defval) { String prop = getString(loggerName, shortName); - return (prop != null) ? "true".equalsIgnoreCase(prop) : defval; + return Boolean.valueOf(prop != null ? Boolean.parseBoolean(prop) : defval); } public Integer getInteger(final String shortName) { diff --git a/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogManager.java b/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogManager.java index 28ccb8c7e..d8da23d84 100644 --- a/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogManager.java +++ b/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogManager.java @@ -14,7 +14,6 @@ * limitations under the License. */ package net.openhft.chronicle.logger; - import net.openhft.chronicle.queue.ChronicleQueue; import java.io.IOException; @@ -62,6 +61,7 @@ public void clear() { try { writer.close(); } catch (IOException e) { + System.err.println("Unable to close ChronicleLogWriter instance."); } } @@ -93,7 +93,7 @@ public ChronicleLogWriter getWriter(String name) { final String path = cfg.getString(name, ChronicleLogConfig.KEY_PATH); if (path != null) { // Creating a Queue takes some time. Other threads might be blocked for longer periods. - return writers.computeIfAbsent(path, p-> new DefaultChronicleLogWriter(newChronicle(p, name))); + return writers.computeIfAbsent(path, p -> new DefaultChronicleLogWriter(newChronicle(p, name))); } else { throw new IllegalArgumentException( "chronicle.logger.root.path is not defined, chronicle.logger." + name + ".path is not defined" @@ -108,9 +108,8 @@ public ChronicleLogWriter getWriter(String name) { private ChronicleQueue newChronicle(String path, String name) { final String wireType = cfg.getString(name, ChronicleLogConfig.KEY_WIRETYPE); ChronicleQueue cq = this.cfg.getAppenderConfig().build(path, wireType); - if (!cfg.getBoolean(name, ChronicleLogConfig.KEY_APPEND, true)) { - // TODO re-enable when it's implemented. ATM it throws UnsupportedOperationException... - //cq.clear(); + if (!cfg.getBoolean(name, ChronicleLogConfig.KEY_APPEND, true)) { // NOPMD - EmptyControlStatement + // Queue clear is currently unsupported; files rotate naturally on reopen. } return cq; } diff --git a/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogWriter.java b/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogWriter.java index ed3b94fbc..50ef7aaf3 100644 --- a/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogWriter.java +++ b/logger-core/src/main/java/net/openhft/chronicle/logger/ChronicleLogWriter.java @@ -31,7 +31,7 @@ public interface ChronicleLogWriter extends Closeable { * @param timestamp epoch time in milliseconds * @param threadName name of the calling thread * @param loggerName name of the logger - * @param message formatted message text + * @param message message text or pattern without arguments */ void write( ChronicleLogLevel level, @@ -49,7 +49,7 @@ void write( * @param timestamp epoch time in milliseconds * @param threadName name of the calling thread * @param loggerName name of the logger - * @param message formatted message text + * @param message message pattern that may contain "{}" placeholders * @param throwable optional stack trace to record, may be {@code null} * @param args optional argument objects, may be empty */ diff --git a/logger-core/src/main/java/net/openhft/chronicle/logger/DefaultChronicleLogWriter.java b/logger-core/src/main/java/net/openhft/chronicle/logger/DefaultChronicleLogWriter.java index ed493458b..fa9674599 100644 --- a/logger-core/src/main/java/net/openhft/chronicle/logger/DefaultChronicleLogWriter.java +++ b/logger-core/src/main/java/net/openhft/chronicle/logger/DefaultChronicleLogWriter.java @@ -14,7 +14,6 @@ * limitations under the License. */ package net.openhft.chronicle.logger; - import net.openhft.chronicle.queue.ChronicleQueue; import net.openhft.chronicle.queue.ExcerptAppender; import net.openhft.chronicle.wire.DocumentContext; @@ -110,7 +109,7 @@ public void write( } REENTRANCY_FLAG.set(true); try (ExcerptAppender appender = cq.createAppender(); - final DocumentContext dc = appender.writingDocument()) { + DocumentContext dc = appender.writingDocument()) { Wire wire = dc.wire(); assert wire != null; wire diff --git a/logger-core/src/main/java/net/openhft/chronicle/logger/LogAppenderConfig.java b/logger-core/src/main/java/net/openhft/chronicle/logger/LogAppenderConfig.java index f4f931cdd..4ec0da89d 100644 --- a/logger-core/src/main/java/net/openhft/chronicle/logger/LogAppenderConfig.java +++ b/logger-core/src/main/java/net/openhft/chronicle/logger/LogAppenderConfig.java @@ -22,8 +22,11 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.beans.IntrospectionException; import java.beans.PropertyDescriptor; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Locale; import java.util.Map; import java.util.Properties; @@ -41,13 +44,19 @@ public class LogAppenderConfig { "rollCycle" }; - /** Size in bytes of each queue data block. */ + /** + * Size in bytes of each queue data block. + */ private int blockSize; - /** Capacity in bytes of the queue write buffer. */ + /** + * Capacity in bytes of the queue write buffer. + */ private long bufferCapacity; - /** Name of the {@link RollCycles} to use when rolling files. */ + /** + * Name of the {@link RollCycles} to use when rolling files. + */ private String rollCycle; public LogAppenderConfig() { @@ -86,27 +95,38 @@ public void setRollCycle(String rollCycle) { // ************************************************************************* public String[] keys() { - return KEYS; + return KEYS.clone(); } /** * Builds a queue at {@code path} using this configuration. * - * @param path directory for the queue + * @param path directory for the queue * @param wireType name of the wire format or {@code null} for binary * @return the configured queue */ public ChronicleQueue build(String path, String wireType) { - WireType wireTypeEnum = wireType != null ? WireType.valueOf(wireType.toUpperCase()) : WireType.BINARY_LIGHT; + WireType wireTypeEnum = wireType != null + ? WireType.valueOf(wireType.toUpperCase(Locale.ROOT)) + : WireType.BINARY_LIGHT; SingleChronicleQueueBuilder builder = ChronicleQueue.singleBuilder(path) .wireType(wireTypeEnum) .blockSize(blockSize) .bufferCapacity(bufferCapacity); - if (rollCycle != null) - builder.rollCycle(RollCycles.valueOf(rollCycle)); + if (rollCycle != null) { + builder.rollCycle(RollCycles.valueOf(rollCycle.toUpperCase(Locale.ROOT))); + } return builder.build(); } + LogAppenderConfig copy() { + LogAppenderConfig clone = new LogAppenderConfig(); + clone.setBlockSize(this.blockSize); + clone.setBufferCapacity(this.bufferCapacity); + clone.setRollCycle(this.rollCycle); + return clone; + } + /** * Reads configuration values from a {@link Properties} object. * Only keys starting with {@code prefix}, when supplied, are considered. @@ -150,7 +170,10 @@ public void setProperty(@NotNull final String propName, final String propValue) } else if (type == String.class) { method.invoke(this, propValue); } - } catch (Exception e) { + } catch (IntrospectionException | InvocationTargetException | IllegalAccessException e) { + throw new IllegalArgumentException("Unable to set Chronicle Logger property '" + propName + "'", e); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid numeric value for '" + propName + "': " + propValue, e); } } } diff --git a/logger-core/src/main/java/net/openhft/chronicle/logger/internal/package-info.java b/logger-core/src/main/java/net/openhft/chronicle/logger/internal/package-info.java index f806da7c7..d0b4e718c 100644 --- a/logger-core/src/main/java/net/openhft/chronicle/logger/internal/package-info.java +++ b/logger-core/src/main/java/net/openhft/chronicle/logger/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any and all sub-packages contains strictly internal classes for this Chronicle library. * Internal classes shall never be used directly. *

- * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

    *
  • Casting to
    • *
  • Reflection of any kind
    • diff --git a/logger-core/src/test/java/net/openhft/chronicle/logger/ChronicleLogManagerTest.java b/logger-core/src/test/java/net/openhft/chronicle/logger/ChronicleLogManagerTest.java new file mode 100644 index 000000000..13032962d --- /dev/null +++ b/logger-core/src/test/java/net/openhft/chronicle/logger/ChronicleLogManagerTest.java @@ -0,0 +1,221 @@ +package net.openhft.chronicle.logger; + +import net.openhft.chronicle.core.io.IOTools; +import net.openhft.chronicle.queue.ChronicleQueue; +import net.openhft.chronicle.queue.ExcerptTailer; +import net.openhft.chronicle.wire.DocumentContext; +import net.openhft.chronicle.wire.Wire; +import net.openhft.chronicle.wire.WireType; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.io.Writer; +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.Properties; + +import static org.junit.Assert.*; + +/** + * Covers configuration loading and reload semantics for {@link ChronicleLogManager}. + */ +public class ChronicleLogManagerTest { + + private ChronicleLogManager manager; + private String previousConfig; + private Path tempDir; + private Path configFile; + + @Before + public void setUp() throws IOException { + manager = ChronicleLogManager.getInstance(); + manager.clear(); + previousConfig = System.getProperty("chronicle.logger.properties"); + tempDir = Files.createTempDirectory("chronicle-logger-test"); + configFile = tempDir.resolve("chronicle.logger.properties"); + } + + @After + public void tearDown() { + manager.clear(); + if (previousConfig == null) { + System.clearProperty("chronicle.logger.properties"); + } else { + System.setProperty("chronicle.logger.properties", previousConfig); + } + manager.reload(); + IOTools.deleteDirWithFiles(tempDir.toString()); + } + + @Test + public void loadsWriterFromExplicitConfiguration() throws Exception { + Path rootPath = Files.createDirectories(tempDir.resolve("root")); + Path loggerPath = Files.createDirectories(tempDir.resolve("app")); + + reloadWithConfig(rootPath, loggerPath); + + ChronicleLogWriter writer = manager.getWriter("app"); + writer.write( + ChronicleLogLevel.INFO, + System.currentTimeMillis(), + "thread-one", + "app", + "first-message"); + writer.close(); + + assertQueueContainsOnlyMessage(loggerPath, "first-message"); + } + + @Test + public void reloadRecreatesWriterWhenPathChanges() throws Exception { + Path rootPath = Files.createDirectories(tempDir.resolve("root")); + Path firstPath = Files.createDirectories(tempDir.resolve("first")); + Path secondPath = Files.createDirectories(tempDir.resolve("second")); + + reloadWithConfig(rootPath, firstPath); + + ChronicleLogWriter writer = manager.getWriter("app"); + writer.write( + ChronicleLogLevel.INFO, + System.currentTimeMillis(), + "thread-A", + "app", + "first-message"); + writer.close(); + + reloadWithConfig(rootPath, secondPath); + + ChronicleLogWriter newWriter = manager.getWriter("app"); + newWriter.write( + ChronicleLogLevel.INFO, + System.currentTimeMillis(), + "thread-B", + "app", + "second-message"); + newWriter.close(); + + assertQueueContainsOnlyMessage(secondPath, "second-message"); + assertQueueContainsOnlyMessage(firstPath, "first-message"); + } + + @Test + public void appliesAppenderSettingsIncludingRollCycle() throws Exception { + Path rootPath = Files.createDirectories(tempDir.resolve("root")); + Path loggerPath = Files.createDirectories(tempDir.resolve("json-app")); + + Properties props = defaultConfig(rootPath, loggerPath); + props.setProperty("chronicle.logger.root.cfg.rollCycle", "FAST_DAILY"); + props.setProperty("chronicle.logger.root.cfg.blockSize", "4096"); + props.setProperty("chronicle.logger.root.cfg.bufferCapacity", "8192"); + props.setProperty("chronicle.logger.app.append", "false"); + props.setProperty("chronicle.logger.app.wireType", "JSON"); + + reloadWithProperties(props); + + ChronicleLogWriter writer = manager.getWriter("app"); + assertTrue("Expected default writer implementation", writer instanceof DefaultChronicleLogWriter); + DefaultChronicleLogWriter defaultWriter = (DefaultChronicleLogWriter) writer; + assertEquals(WireType.JSON, defaultWriter.getWireType()); + writer.close(); + + LogAppenderConfig appenderConfig = manager.cfg().getAppenderConfig(); + assertEquals("FAST_DAILY", appenderConfig.getRollCycle()); + assertEquals(4096, appenderConfig.getBlockSize()); + assertEquals(8192L, appenderConfig.getBufferCapacity()); + } + + @Test + public void throwsWhenNoPathConfigured() throws Exception { + Properties props = new Properties(); + props.setProperty("chronicle.logger.root.level", "info"); + props.setProperty("chronicle.logger.app.level", "info"); + + reloadWithProperties(props); + + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> manager.getWriter("app")); + assertTrue(ex.getMessage().contains("chronicle.logger.root.path")); + } + + @Test + public void clearHandlesIOExceptionFromWriter() throws Exception { + Path rootPath = Files.createDirectories(tempDir.resolve("root")); + Path loggerPath = Files.createDirectories(tempDir.resolve("closing")); + + Properties props = defaultConfig(rootPath, loggerPath); + reloadWithProperties(props); + + ChronicleLogWriter writer = manager.getWriter("app"); + writer.close(); + + Field writersField = ChronicleLogManager.class.getDeclaredField("writers"); + writersField.setAccessible(true); + @SuppressWarnings("unchecked") + Map writers = (Map) writersField.get(manager); + writers.put("failing", new ChronicleLogWriter() { + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message) { + // not used + } + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message, Throwable throwable, Object... args) { + // not used + } + + @Override + public void close() throws IOException { + throw new IOException("simulated failure"); + } + }); + + manager.clear(); + assertTrue("all writers should be cleared despite close failure", writers.isEmpty()); + } + + private void reloadWithConfig(Path rootPath, Path loggerPath) throws IOException { + Properties props = defaultConfig(rootPath, loggerPath); + reloadWithProperties(props); + } + + private Properties defaultConfig(Path rootPath, Path loggerPath) throws IOException { + Files.createDirectories(rootPath); + Files.createDirectories(loggerPath); + + Properties props = new Properties(); + props.setProperty("chronicle.logger.root.path", rootPath.toString()); + props.setProperty("chronicle.logger.root.level", "debug"); + props.setProperty("chronicle.logger.app.path", loggerPath.toString()); + props.setProperty("chronicle.logger.app.level", "info"); + + return props; + } + + private void reloadWithProperties(Properties props) throws IOException { + try (Writer out = Files.newBufferedWriter(configFile, StandardCharsets.ISO_8859_1)) { + props.store(out, "test configuration"); + } + System.setProperty("chronicle.logger.properties", configFile.toString()); + manager.reload(); + } + + private static void assertQueueContainsOnlyMessage(Path queuePath, String expectedMessage) { + try (ChronicleQueue queue = ChronicleQueue.singleBuilder(queuePath.toString()).build()) { + ExcerptTailer tailer = queue.createTailer(); + try (DocumentContext dc = tailer.readingDocument()) { + Wire wire = dc.wire(); + assertNotNull("Queue entry missing for " + queuePath, wire); + assertEquals(expectedMessage, wire.read("message").text()); + } + + try (DocumentContext dc = tailer.readingDocument()) { + Wire wire = dc.wire(); + assertNull("Queue contains unexpected extra entries for " + queuePath, wire); + } + } + } +} diff --git a/logger-jcl/pom.xml b/logger-jcl/pom.xml index 4cb961b95..d4c4e62fe 100644 --- a/logger-jcl/pom.xml +++ b/logger-jcl/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -130,4 +131,154 @@ master + + + + code-review + + false + + + 0.6400 + 0.8333 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-jcl/src/main/config/checkstyle.xml b/logger-jcl/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-jcl/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-jcl/src/main/config/pmd-exclude.properties b/logger-jcl/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..9ad413fd9 --- /dev/null +++ b/logger-jcl/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-jcl module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-jcl/src/main/config/spotbugs-exclude.xml b/logger-jcl/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..d6fac2352 --- /dev/null +++ b/logger-jcl/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/logger-jcl/src/main/java/net/openhft/chronicle/logger/jcl/internal/package-info.java b/logger-jcl/src/main/java/net/openhft/chronicle/logger/jcl/internal/package-info.java index 292546af2..c744528ba 100644 --- a/logger-jcl/src/main/java/net/openhft/chronicle/logger/jcl/internal/package-info.java +++ b/logger-jcl/src/main/java/net/openhft/chronicle/logger/jcl/internal/package-info.java @@ -3,8 +3,8 @@ * for this Chronicle library. They exist solely for the logger's internal * mechanics. Internal classes shall never be used directly. *

    - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

      *
    • Casting to
      • *
    • Reflection of any kind
      • diff --git a/logger-jcl/src/test/java/net/openhft/chronicle/logger/jcl/JclTestBase.java b/logger-jcl/src/test/java/net/openhft/chronicle/logger/jcl/JclTestBase.java index beda733c5..1b24a5808 100644 --- a/logger-jcl/src/test/java/net/openhft/chronicle/logger/jcl/JclTestBase.java +++ b/logger-jcl/src/test/java/net/openhft/chronicle/logger/jcl/JclTestBase.java @@ -15,8 +15,6 @@ */ package net.openhft.chronicle.logger.jcl; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; import net.openhft.chronicle.logger.ChronicleLogLevel; import org.apache.commons.logging.Log; diff --git a/logger-jcl/src/test/resources/chronicle.logger.properties b/logger-jcl/src/test/resources/chronicle.logger.properties index ac00fe52a..1b6c02592 100644 --- a/logger-jcl/src/test/resources/chronicle.logger.properties +++ b/logger-jcl/src/test/resources/chronicle.logger.properties @@ -1,4 +1,3 @@ - # # Copyright (C) 2014-2017 Chronicle Software # @@ -16,24 +15,19 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - ################################################################################ # Common ################################################################################ - -chronicle.logger.base = ${java.io.tmpdir}/chronicle-jcl -chronicle.logger.root.path = ${chronicle.logger.base}/root -chronicle.logger.root.level = debug -chronicle.logger.root.append = false - +chronicle.logger.base=${java.io.tmpdir}/chronicle-jcl +chronicle.logger.root.path=${chronicle.logger.base}/root +chronicle.logger.root.level=debug +chronicle.logger.root.append=false ################################################################################ # Loggers ################################################################################ - # logger : Logger1 -chronicle.logger.logger_1.path = ${chronicle.logger.base}/logger_1 -chronicle.logger.logger_1.level = info -chronicle.logger.logger_1.wireType = JSON - +chronicle.logger.logger_1.path=${chronicle.logger.base}/logger_1 +chronicle.logger.logger_1.level=info +chronicle.logger.logger_1.wireType=JSON # logger : impl -chronicle.logger.readwrite.path = ${chronicle.logger.base}/readwrite +chronicle.logger.readwrite.path=${chronicle.logger.base}/readwrite diff --git a/logger-jul/pom.xml b/logger-jul/pom.xml index f1a2e87ba..6368bae30 100644 --- a/logger-jul/pom.xml +++ b/logger-jul/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -38,6 +39,7 @@ net.openhft chronicle-logger-core + org.slf4j slf4j-nop @@ -116,4 +118,154 @@ scm:git:git@github.com:OpenHFT/Chronicle-Logger.git master + + + + code-review + + false + + + 0.7573 + 0.6400 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-jul/src/main/config/checkstyle.xml b/logger-jul/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-jul/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-jul/src/main/config/pmd-exclude.properties b/logger-jul/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..f501535be --- /dev/null +++ b/logger-jul/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-jul module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-jul/src/main/config/spotbugs-exclude.xml b/logger-jul/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..00c3d72d7 --- /dev/null +++ b/logger-jul/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,15 @@ + + + + + + + + + CLG-FN-003: constructor fails fast when Chronicle level mapping is invalid. + + + diff --git a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/AbstractChronicleHandler.java b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/AbstractChronicleHandler.java index c048d110a..7a9cb2cc0 100644 --- a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/AbstractChronicleHandler.java +++ b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/AbstractChronicleHandler.java @@ -35,18 +35,12 @@ */ abstract class AbstractChronicleHandler extends Handler { - /** - * Filesystem location of the Chronicle queue. - */ - private String path; - /** * Destination writer used to emit log events. */ private ChronicleLogWriter writer; protected AbstractChronicleHandler() { - this.path = null; this.writer = null; } @@ -60,7 +54,7 @@ public void close() throws SecurityException { try { this.writer.close(); } catch (IOException e) { - // Ignore + System.err.println("Unable to close Chronicle JUL handler writer."); } } } diff --git a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfig.java b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfig.java index 6763120dc..fd7616516 100644 --- a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfig.java +++ b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfig.java @@ -17,6 +17,7 @@ import net.openhft.chronicle.logger.LogAppenderConfig; +import java.lang.reflect.InvocationTargetException; import java.util.logging.Filter; import java.util.logging.Level; import java.util.logging.LogManager; @@ -122,15 +123,15 @@ boolean getBooleanProperty(String name, boolean defaultValue) { return defaultValue; } - val = val.toLowerCase(); - if (val.equals("true") || val.equals("1")) { + String trimmed = val.trim(); + if ("1".equals(trimmed)) { return true; - - } else if (val.equals("false") || val.equals("0")) { + } + if ("0".equals(trimmed)) { return false; } - return defaultValue; + return Boolean.parseBoolean(trimmed); } /** @@ -143,15 +144,16 @@ boolean getBooleanProperty(String name, boolean defaultValue) { Filter getFilterProperty(String name, Filter defaultValue) { String val = getStringProperty(name, null); + if (val == null) { + return defaultValue; + } try { - if (val != null) { - Class clz = ClassLoader.getSystemClassLoader().loadClass(val); - return (Filter) clz.getConstructor().newInstance(); - } - } catch (Exception ex) { - // ignore and return default + Class clz = ClassLoader.getSystemClassLoader().loadClass(val); + return (Filter) clz.getDeclaredConstructor().newInstance(); + } catch (ClassNotFoundException | NoSuchMethodException | InstantiationException + | IllegalAccessException | InvocationTargetException e) { + return defaultValue; } - return defaultValue; } /** @@ -167,8 +169,11 @@ Level getLevelProperty(String name, Level defaultValue) { if (val == null) { return defaultValue; } - Level l = Level.parse(val.trim()); - return l != null ? l : defaultValue; + try { + return Level.parse(val.trim()); + } catch (IllegalArgumentException e) { + return defaultValue; + } } /** diff --git a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleLogger.java b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleLogger.java index 6164daa74..8f98f7f5b 100644 --- a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleLogger.java +++ b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/ChronicleLogger.java @@ -14,10 +14,10 @@ * limitations under the License. */ package net.openhft.chronicle.logger.jul; - import net.openhft.chronicle.logger.ChronicleLogLevel; import net.openhft.chronicle.logger.ChronicleLogWriter; +import java.util.Objects; import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; @@ -28,7 +28,7 @@ *

      Extends {@link Logger} so JUL applications can write to a * {@link ChronicleLogWriter}. The active level is controlled by * {@link ChronicleLogLevel}.

      -*/ + */ class ChronicleLogger extends Logger { protected final String name; @@ -39,15 +39,15 @@ class ChronicleLogger extends Logger { * c-tor * * @param writer to use - * @param name of the logger - * @param level of the logger + * @param name of the logger + * @param level of the logger */ ChronicleLogger(final ChronicleLogWriter writer, final String name, final ChronicleLogLevel level) { - super(name, null); + super(Objects.requireNonNull(name, "logger name"), null); - this.writer = writer; + this.writer = Objects.requireNonNull(writer, "writer"); this.name = name; - this.level = level; + this.level = Objects.requireNonNull(level, "level"); /* * Set level of super class using final method @@ -319,10 +319,27 @@ protected void append(final Level level, String msg, Throwable thrown) { * Logger implementation that ignores all messages. */ public static class Null extends ChronicleLogger { + private static final ChronicleLogWriter NOOP_WRITER = new ChronicleLogWriter() { + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message) { + // no-op + } + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message, Throwable throwable, Object... args) { + // no-op + } + + @Override + public void close() { + // no-op + } + }; + public static final ChronicleLogger INSTANCE = new Null(); private Null() { - super(null, null, null); + super(NOOP_WRITER, "chronicle.null", ChronicleLogLevel.DEBUG); } @Override diff --git a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/internal/package-info.java b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/internal/package-info.java index 4650a3097..e5bedb9b4 100644 --- a/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/internal/package-info.java +++ b/logger-jul/src/main/java/net/openhft/chronicle/logger/jul/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any and all sub-packages contains strictly internal classes for this Chronicle library. * Internal classes shall never be used directly. *

      - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

        *
      • Casting to
        • *
      • Reflection of any kind
        • diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfigurationTest.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfigurationTest.java new file mode 100644 index 000000000..140802fcb --- /dev/null +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleHandlerConfigurationTest.java @@ -0,0 +1,133 @@ +package net.openhft.chronicle.logger.jul; + +import net.openhft.chronicle.core.io.IOTools; +import net.openhft.chronicle.logger.ChronicleLogLevel; +import net.openhft.chronicle.logger.LogAppenderConfig; +import net.openhft.chronicle.logger.jul.support.AllowAllFilter; +import net.openhft.chronicle.queue.ChronicleQueue; +import net.openhft.chronicle.queue.ExcerptTailer; +import net.openhft.chronicle.wire.DocumentContext; +import net.openhft.chronicle.wire.Wire; +import net.openhft.chronicle.wire.WireType; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.LogManager; +import java.util.logging.LogRecord; + +import static java.lang.System.currentTimeMillis; +import static org.junit.Assert.*; + +/** + * Exercises {@link ChronicleHandlerConfig} property resolution and verifies + * that {@link ChronicleHandler} writes expected entries to a Chronicle Queue. + */ +public class ChronicleHandlerConfigurationTest extends JulHandlerTestBase { + + private Path tempDir; + private String previousBasePath; + + @Before + public void setUp() throws IOException { + tempDir = Files.createTempDirectory("chronicle-handler-config"); + previousBasePath = System.getProperty("chronicle.handler.base"); + System.setProperty("chronicle.handler.base", tempDir.toString()); + setupLogManager("chronicle-handler-config"); + } + + @After + public void tearDown() { + LogManager.getLogManager().reset(); + if (previousBasePath == null) { + System.clearProperty("chronicle.handler.base"); + } else { + System.setProperty("chronicle.handler.base", previousBasePath); + } + IOTools.deleteDirWithFiles(tempDir.toString()); + } + + @Test + public void loadsConfigurationAndPersistsLogEntries() throws Exception { + ChronicleHandlerConfig config = new ChronicleHandlerConfig(ChronicleHandler.class); + + Path expectedPath = tempDir.resolve("config-test").toAbsolutePath().normalize(); + Path actualPath = Paths.get(config.getString("path", null)).toAbsolutePath().normalize(); + assertEquals("path placeholder should resolve", expectedPath, actualPath); + assertEquals(Level.FINE, config.getLevel("level", Level.INFO)); + assertTrue("boolean value of 1 should map to true", config.getBoolean("enabled", false)); + assertEquals("TEXT", config.getString("wireType", "BINARY_LIGHT")); + assertTrue(config.getFilter("filter", null) instanceof AllowAllFilter); + + LogAppenderConfig appenderConfig = config.getAppenderConfig(); + assertEquals(32, appenderConfig.getBlockSize()); + assertEquals(64L, appenderConfig.getBufferCapacity()); + assertEquals("FAST_DAILY", appenderConfig.getRollCycle()); + + ChronicleHandler handler = new ChronicleHandler(); + assertTrue(handler.getFilter() instanceof AllowAllFilter); + + handler.publish(record(Level.CONFIG, "config message {0}", new Object[]{7}, null)); + handler.publish(record(Level.WARNING, "warning message", new Object[0], new IllegalStateException("broken"))); + handler.close(); + + Path queueDir = tempDir.resolve("config-test"); + try (ChronicleQueue queue = ChronicleQueue.singleBuilder(queueDir.toString()).wireType(WireType.BINARY_LIGHT).build()) { + ExcerptTailer tailer = queue.createTailer(); + + try (DocumentContext dc = tailer.readingDocument()) { + Wire wire = dc.wire(); + assertNotNull("first log entry missing", wire); + assertTrue(wire.read("ts").int64() <= currentTimeMillis()); + assertEquals(ChronicleLogLevel.DEBUG, wire.read("level").asEnum(ChronicleLogLevel.class)); + String threadName = wire.read("threadName").text(); + assertTrue("thread name should start with thread-", threadName.startsWith("thread-")); + assertEquals("config-logger", wire.read("loggerName").text()); + assertEquals("config message {0}", wire.read("message").text()); + assertTrue("args expected", wire.hasMore()); + List args = new ArrayList<>(); + wire.read("args").sequence(args, (list, valueIn) -> { + while (valueIn.hasNextSequenceItem()) { + list.add(valueIn.object(Object.class)); + } + }); + assertEquals(1, args.size()); + assertEquals(7, ((Number) args.get(0)).intValue()); + assertFalse("no throwable for config event", wire.hasMore()); + } + + try (DocumentContext dc = tailer.readingDocument()) { + Wire wire = dc.wire(); + assertNotNull("second log entry missing", wire); + assertTrue(wire.read("ts").int64() <= currentTimeMillis()); + assertEquals(ChronicleLogLevel.WARN, wire.read("level").asEnum(ChronicleLogLevel.class)); + assertEquals("config-logger", wire.read("loggerName").text()); + assertEquals("warning message", wire.read("message").text()); + assertTrue("throwable expected", wire.hasMore()); + Throwable throwable = wire.read("throwable").throwable(false); + assertTrue(throwable instanceof IllegalStateException); + assertEquals("broken", throwable.getMessage()); + assertFalse("no args on warning log", wire.hasMore()); + } + + try (DocumentContext dc = tailer.readingDocument()) { + assertNull("queue should only contain two events", dc.wire()); + } + } + } + + private static LogRecord record(Level level, String message, Object[] args, Throwable throwable) { + LogRecord record = new LogRecord(level, message); + record.setLoggerName("config-logger"); + record.setParameters(args); + record.setThrown(throwable); + return record; + } +} diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleHandlerFailureTest.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleHandlerFailureTest.java new file mode 100644 index 000000000..e37d65ca8 --- /dev/null +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleHandlerFailureTest.java @@ -0,0 +1,57 @@ +package net.openhft.chronicle.logger.jul; + +import net.openhft.chronicle.core.io.IOTools; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.logging.LogManager; + +import static org.junit.Assert.assertNotNull; + +/** + * Ensures {@link ChronicleHandler} surfaces failures when the configured path + * refers to an invalid location. + */ +public class ChronicleHandlerFailureTest extends JulHandlerTestBase { + + private Path tempDir; + private String previousPath; + + @Before + public void setUp() throws IOException { + tempDir = Files.createTempDirectory("chronicle-handler-failure"); + previousPath = System.getProperty("chronicle.handler.invalid"); + } + + @After + public void tearDown() { + LogManager.getLogManager().reset(); + if (previousPath == null) { + System.clearProperty("chronicle.handler.invalid"); + } else { + System.setProperty("chronicle.handler.invalid", previousPath); + } + IOTools.deleteDirWithFiles(tempDir.toString()); + } + + @Test + public void throwsWhenPathPointsToFile() throws IOException { + Path pathAsFile = Files.createTempFile(tempDir, "existing-queue", ".txt"); + System.setProperty("chronicle.handler.invalid", pathAsFile.toString()); + setupLogManager("chronicle-handler-failure"); + + RuntimeException failure = null; + try { + new ChronicleHandler(); + } catch (IOException ioException) { + failure = new RuntimeException(ioException); + } catch (RuntimeException runtimeException) { + failure = runtimeException; + } + assertNotNull("ChronicleHandler should not start when the queue path is invalid", failure); + } +} diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleLoggerBehaviourTest.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleLoggerBehaviourTest.java new file mode 100644 index 000000000..bd79264fc --- /dev/null +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleLoggerBehaviourTest.java @@ -0,0 +1,173 @@ +package net.openhft.chronicle.logger.jul; + +import net.openhft.chronicle.logger.ChronicleLogLevel; +import net.openhft.chronicle.logger.ChronicleLogWriter; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import static org.junit.Assert.*; + +/** + * Behavioural coverage for {@link ChronicleLogger}. + */ +public class ChronicleLoggerBehaviourTest { + + @Test + public void forwardsLogCallsWhenLevelEnabled() { + RecordingWriter writer = new RecordingWriter(); + ChronicleLogger logger = new ChronicleLogger(writer, "jul-behaviour", ChronicleLogLevel.INFO); + + assertFalse(logger.isLoggable(Level.FINE)); + assertTrue(logger.isLoggable(Level.INFO)); + + logger.log(Level.INFO, "plain"); + logger.log(Level.INFO, "one {0}", 7); + logger.log(Level.WARNING, "warn with throwable", new IllegalStateException("boom")); + logger.logp(Level.SEVERE, "source", "method", "p message", new RuntimeException("p")); + logger.info("info shortcut"); + logger.finer("suppressed finer"); + logger.warning("warning shortcut"); + + assertEquals("expected six emitted events", 6, writer.events.size()); + + LoggedEvent first = writer.events.get(0); + assertEquals(ChronicleLogLevel.INFO, first.level); + assertEquals("plain", first.message); + assertEquals(0, first.args.length); + assertNull(first.throwable); + + LoggedEvent second = writer.events.get(1); + assertEquals("one {0}", second.message); + assertArrayEquals(new Object[]{7}, second.args); + assertNull(second.throwable); + + LoggedEvent third = writer.events.get(2); + assertEquals(ChronicleLogLevel.WARN, third.level); + assertEquals("warn with throwable", third.message); + assertTrue(third.throwable instanceof IllegalStateException); + assertEquals("boom", third.throwable.getMessage()); + + LoggedEvent fourth = writer.events.get(3); + assertEquals(ChronicleLogLevel.ERROR, fourth.level); + assertEquals("p message", fourth.message); + assertTrue(fourth.throwable instanceof RuntimeException); + assertEquals("p", fourth.throwable.getMessage()); + + LoggedEvent fifth = writer.events.get(4); + assertEquals(ChronicleLogLevel.INFO, fifth.level); + assertEquals("info shortcut", fifth.message); + assertEquals(0, fifth.args.length); + assertNull(fifth.throwable); + + LoggedEvent sixth = writer.events.get(5); + assertEquals(ChronicleLogLevel.WARN, sixth.level); + assertEquals("warning shortcut", sixth.message); + } + + @Test + public void appendLogRecordHonoursThreshold() { + RecordingWriter writer = new RecordingWriter(); + ChronicleLogger logger = new ChronicleLogger(writer, "jul-record", ChronicleLogLevel.WARN); + + LogRecord below = new LogRecord(Level.CONFIG, "config"); + below.setParameters(new Object[]{"ignored"}); + logger.log(below); + assertEquals(0, writer.events.size()); + + LogRecord atLevel = new LogRecord(Level.SEVERE, "severe"); + atLevel.setParameters(new Object[]{"rhs"}); + atLevel.setThrown(new IllegalArgumentException("bad")); + logger.log(atLevel); + + assertEquals(1, writer.events.size()); + LoggedEvent event = writer.events.get(0); + assertEquals(ChronicleLogLevel.ERROR, event.level); + assertEquals("severe", event.message); + assertArrayEquals(new Object[]{"rhs"}, event.args); + assertTrue(event.throwable instanceof IllegalArgumentException); + } + + @Test(expected = UnsupportedOperationException.class) + public void setParentIsNotSupported() { + ChronicleLogger logger = new ChronicleLogger(new NoopWriter(), "jul-parent", ChronicleLogLevel.INFO); + logger.setParent(Logger.getGlobal()); + } + + @Test(expected = UnsupportedOperationException.class) + public void enteringThrows() { + ChronicleLogger logger = new ChronicleLogger(new NoopWriter(), "jul-enter", ChronicleLogLevel.INFO); + logger.entering("source", "method"); + } + + @Test + public void nullLoggerIsSingletonAndNoOps() { + ChronicleLogger nullLogger = ChronicleLogger.Null.INSTANCE; + assertSame(nullLogger, ChronicleLogger.Null.INSTANCE); + nullLogger.log(Level.INFO, "ignored"); + nullLogger.log(new LogRecord(Level.SEVERE, "ignored")); + nullLogger.info("ignored"); + } + + private static final class RecordingWriter implements ChronicleLogWriter { + private final List events = new ArrayList<>(); + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message) { + events.add(new LoggedEvent(level, threadName, loggerName, message, null, new Object[0])); + } + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message, Throwable throwable, Object... args) { + Object[] cloned = args == null ? new Object[0] : Arrays.copyOf(args, args.length); + events.add(new LoggedEvent(level, threadName, loggerName, message, throwable, cloned)); + } + + @Override + public void close() { + // no-op for tests + } + } + + private static final class NoopWriter implements ChronicleLogWriter { + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message) { + } + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message, Throwable throwable, Object... args) { + } + + @Override + public void close() { + } + } + + private static final class LoggedEvent { + private final ChronicleLogLevel level; + private final String threadName; + private final String loggerName; + private final String message; + private final Throwable throwable; + private final Object[] args; + + LoggedEvent(ChronicleLogLevel level, + String threadName, + String loggerName, + String message, + Throwable throwable, + Object[] args) { + this.level = level; + this.threadName = threadName; + this.loggerName = loggerName; + this.message = message; + this.throwable = throwable; + this.args = args; + } + } +} diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleLoggerManagerTest.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleLoggerManagerTest.java new file mode 100644 index 000000000..263ac406a --- /dev/null +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/ChronicleLoggerManagerTest.java @@ -0,0 +1,108 @@ +package net.openhft.chronicle.logger.jul; + +import net.openhft.chronicle.core.io.IOTools; +import net.openhft.chronicle.logger.ChronicleLogManager; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Enumeration; +import java.util.Properties; +import java.util.logging.Logger; + +import static org.junit.Assert.*; + +/** + * Covers caching and fallback behaviour for {@link ChronicleLoggerManager}. + */ +public class ChronicleLoggerManagerTest { + + private String previousConfig; + private Path tempDir; + private Path configFile; + private ChronicleLoggerManager manager; + private ChronicleLogManager logManager; + + @Before + public void setUp() throws IOException { + logManager = ChronicleLogManager.getInstance(); + logManager.clear(); + previousConfig = System.getProperty("chronicle.logger.properties"); + tempDir = Files.createTempDirectory("chronicle-jul-manager"); + configFile = tempDir.resolve("chronicle.logger.properties"); + manager = new ChronicleLoggerManager(); + } + + @After + public void tearDown() { + manager.reset(); + logManager.clear(); + if (previousConfig == null) { + System.clearProperty("chronicle.logger.properties"); + } else { + System.setProperty("chronicle.logger.properties", previousConfig); + } + IOTools.deleteDirWithFiles(tempDir.toString()); + } + + @Test + public void returnsCachedLoggerInstances() throws Exception { + Path root = Files.createDirectories(tempDir.resolve("root")); + Path app = Files.createDirectories(tempDir.resolve("cached-app")); + writeConfig(root, app); + + Logger first = manager.getLogger("app"); + Logger second = manager.getLogger("app"); + assertSame("logger should be cached", first, second); + assertTrue(first instanceof ChronicleLogger); + + Enumeration names = manager.getLoggerNames(); + assertTrue(names.hasMoreElements()); + assertEquals("app", names.nextElement()); + } + + @Test + public void resetClearsCachedLoggers() throws Exception { + Path root = Files.createDirectories(tempDir.resolve("root")); + Path app = Files.createDirectories(tempDir.resolve("reset-app")); + writeConfig(root, app); + + Logger logger = manager.getLogger("app"); + assertTrue(logger instanceof ChronicleLogger); + + manager.reset(); + assertFalse(manager.getLoggerNames().hasMoreElements()); + } + + @Test + public void returnsNullLoggerWhenWriterCreationFails() throws Exception { + Properties props = new Properties(); + props.setProperty("chronicle.logger.root.level", "info"); + writeConfig(props); + + Logger logger = manager.getLogger("missing"); + assertSame(ChronicleLogger.Null.INSTANCE, logger); + } + + private void writeConfig(Path rootPath, Path loggerPath) throws IOException { + Properties props = new Properties(); + props.setProperty("chronicle.logger.root.path", rootPath.toString()); + props.setProperty("chronicle.logger.root.level", "debug"); + props.setProperty("chronicle.logger.app.path", loggerPath.toString()); + props.setProperty("chronicle.logger.app.level", "info"); + writeConfig(props); + } + + private void writeConfig(Properties props) throws IOException { + try (Writer out = Files.newBufferedWriter(configFile, StandardCharsets.ISO_8859_1)) { + props.store(out, "chronicle-jul-manager-test"); + } + System.setProperty("chronicle.logger.properties", configFile.toString()); + logManager.reload(); + } +} diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerChronicleTest.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerChronicleTest.java index c5997cc58..10a6edd12 100644 --- a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerChronicleTest.java +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerChronicleTest.java @@ -38,7 +38,7 @@ /** * Tests that {@link ChronicleHandler} writes JUL events to a Chronicle Queue. - * + *

        * The LogManager loads a properties file to register the handler. The global * {@link DiskSpaceMonitor} is closed before the tests run and the temporary * queue directory is removed after each test. diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerTestBase.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerTestBase.java index ad496f43a..65354fffa 100644 --- a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerTestBase.java +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulHandlerTestBase.java @@ -15,9 +15,6 @@ */ package net.openhft.chronicle.logger.jul; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; - import java.io.File; import java.io.FileInputStream; import java.io.IOException; diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulLoggerTestBase.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulLoggerTestBase.java index a4e1f811e..5bc772490 100644 --- a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulLoggerTestBase.java +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/JulLoggerTestBase.java @@ -15,9 +15,6 @@ */ package net.openhft.chronicle.logger.jul; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; - import java.util.logging.LogManager; class JulLoggerTestBase extends JulTestBase { diff --git a/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/support/AllowAllFilter.java b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/support/AllowAllFilter.java new file mode 100644 index 000000000..3238e8d88 --- /dev/null +++ b/logger-jul/src/test/java/net/openhft/chronicle/logger/jul/support/AllowAllFilter.java @@ -0,0 +1,15 @@ +package net.openhft.chronicle.logger.jul.support; + +import java.util.logging.Filter; +import java.util.logging.LogRecord; + +/** + * Simple JUL filter that allows every record through. + */ +public final class AllowAllFilter implements Filter { + + @Override + public boolean isLoggable(LogRecord record) { + return true; + } +} diff --git a/logger-jul/src/test/resources/JulLoggerChronicleTest.properties b/logger-jul/src/test/resources/JulLoggerChronicleTest.properties index c52a017b9..b0271a450 100644 --- a/logger-jul/src/test/resources/JulLoggerChronicleTest.properties +++ b/logger-jul/src/test/resources/JulLoggerChronicleTest.properties @@ -15,20 +15,15 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - ################################################################################ # Common ################################################################################ - -chronicle.logger.base = ${java.io.tmpdir}/chronicle-jul-api - -chronicle.logger.root.path = ${chronicle.logger.base}/root-binary -chronicle.logger.root.level = debug -chronicle.logger.root.append = false - -chronicle.logger.logger_1.path = ${chronicle.logger.base}/logger_1 -chronicle.logger.logger_1.level = info -chronicle.logger.logger_1.wireType = json - -chronicle.logger.logger_bin.path = ${chronicle.logger.base}/logger_bin -chronicle.logger.logger_bin.level = trace +chronicle.logger.base=${java.io.tmpdir}/chronicle-jul-api +chronicle.logger.root.path=${chronicle.logger.base}/root-binary +chronicle.logger.root.level=debug +chronicle.logger.root.append=false +chronicle.logger.logger_1.path=${chronicle.logger.base}/logger_1 +chronicle.logger.logger_1.level=info +chronicle.logger.logger_1.wireType=json +chronicle.logger.logger_bin.path=${chronicle.logger.base}/logger_bin +chronicle.logger.logger_bin.level=trace diff --git a/logger-jul/src/test/resources/binary-cfg.properties b/logger-jul/src/test/resources/binary-cfg.properties index 6da931d95..786188756 100644 --- a/logger-jul/src/test/resources/binary-cfg.properties +++ b/logger-jul/src/test/resources/binary-cfg.properties @@ -1,4 +1,3 @@ - # # Copyright (C) 2014-2017 Chronicle Software # @@ -16,24 +15,17 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - handlers=java.util.logging.ConsoleHandler, net.openhft.chronicle.logger.jul.ChronicleHandler - .level=ALL - java.util.logging.ConsoleHandler.level=ALL java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter - net.openhft.level=WARNING net.openhft.handlers=java.util.logging.ConsoleHandler - ################################################################################ # BINARY ################################################################################ - -net.openhft.chronicle.logger.jul.ChronicleHandler.path = ${java.io.tmpdir}/chronicle-jul -net.openhft.chronicle.logger.jul.ChronicleHandler.level = ALL - +net.openhft.chronicle.logger.jul.ChronicleHandler.path=${java.io.tmpdir}/chronicle-jul +net.openhft.chronicle.logger.jul.ChronicleHandler.level=ALL binary-cfg.level=INFO binary-cfg.handlers=net.openhft.chronicle.logger.jul.ChronicleHandler binary-cfg.useParentHandlers=false diff --git a/logger-jul/src/test/resources/binary-chronicle.properties b/logger-jul/src/test/resources/binary-chronicle.properties index 77d2e9967..84d6d0c68 100644 --- a/logger-jul/src/test/resources/binary-chronicle.properties +++ b/logger-jul/src/test/resources/binary-chronicle.properties @@ -1,4 +1,3 @@ - # # Copyright (C) 2014-2017 Chronicle Software # @@ -16,24 +15,17 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - handlers=java.util.logging.ConsoleHandler, net.openhft.chronicle.logger.jul.ChronicleHandler - .level=ALL - java.util.logging.ConsoleHandler.level=ALL java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter - net.openhft.level=WARNING net.openhft.handlers=java.util.logging.ConsoleHandler - ################################################################################ # BINARY ################################################################################ - -net.openhft.chronicle.logger.jul.ChronicleHandler.path = ${java.io.tmpdir}/chronicle-jul/binary-chronicle -net.openhft.chronicle.logger.jul.ChronicleHandler.level = ALL - +net.openhft.chronicle.logger.jul.ChronicleHandler.path=${java.io.tmpdir}/chronicle-jul/binary-chronicle +net.openhft.chronicle.logger.jul.ChronicleHandler.level=ALL binary-chronicle.level=ALL binary-chronicle.handlers=net.openhft.chronicle.logger.jul.ChronicleHandler binary-chronicle.useParentHandlers=false diff --git a/logger-jul/src/test/resources/chronicle-handler-config.properties b/logger-jul/src/test/resources/chronicle-handler-config.properties new file mode 100644 index 000000000..689238288 --- /dev/null +++ b/logger-jul/src/test/resources/chronicle-handler-config.properties @@ -0,0 +1,12 @@ +.level=ALL +net.openhft.chronicle.logger.jul.ChronicleHandler.path=${chronicle.handler.base}/config-test +net.openhft.chronicle.logger.jul.ChronicleHandler.level=FINE +net.openhft.chronicle.logger.jul.ChronicleHandler.filter=net.openhft.chronicle.logger.jul.support.AllowAllFilter +net.openhft.chronicle.logger.jul.ChronicleHandler.cfg.blockSize=32 +net.openhft.chronicle.logger.jul.ChronicleHandler.cfg.bufferCapacity=64 +net.openhft.chronicle.logger.jul.ChronicleHandler.cfg.rollCycle=FAST_DAILY +net.openhft.chronicle.logger.jul.ChronicleHandler.wireType=TEXT +net.openhft.chronicle.logger.jul.ChronicleHandler.enabled=1 +config-logger.level=ALL +config-logger.handlers=net.openhft.chronicle.logger.jul.ChronicleHandler +config-logger.useParentHandlers=false diff --git a/logger-jul/src/test/resources/chronicle-handler-failure.properties b/logger-jul/src/test/resources/chronicle-handler-failure.properties new file mode 100644 index 000000000..15ceaa6ca --- /dev/null +++ b/logger-jul/src/test/resources/chronicle-handler-failure.properties @@ -0,0 +1,3 @@ +.level=ALL +net.openhft.chronicle.logger.jul.ChronicleHandler.path=${chronicle.handler.invalid} +net.openhft.chronicle.logger.jul.ChronicleHandler.level=INFO diff --git a/logger-log4j-1/pom.xml b/logger-log4j-1/pom.xml index f854a8295..930ac1023 100644 --- a/logger-log4j-1/pom.xml +++ b/logger-log4j-1/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -128,4 +129,156 @@ master + + + + code-review + + false + + + 0.4761 + 0.4000 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-log4j-1/src/main/config/checkstyle.xml b/logger-log4j-1/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-log4j-1/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-log4j-1/src/main/config/pmd-exclude.properties b/logger-log4j-1/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..ff328813f --- /dev/null +++ b/logger-log4j-1/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-log4j-1 module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-log4j-1/src/main/config/spotbugs-exclude.xml b/logger-log4j-1/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..29584b71c --- /dev/null +++ b/logger-log4j-1/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/AbstractChronicleAppender.java b/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/AbstractChronicleAppender.java index aadf4d1bb..448b1faf8 100644 --- a/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/AbstractChronicleAppender.java +++ b/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/AbstractChronicleAppender.java @@ -28,7 +28,7 @@ /** * Base Log4j 1.x appender for Chronicle. - * + *

        * The class manages filter handling and delegates the actual write * operation to a {@link ChronicleLogWriter} created by * {@link #createWriter()}. diff --git a/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/internal/package-info.java b/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/internal/package-info.java index 9f5269ef1..a676099d8 100644 --- a/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/internal/package-info.java +++ b/logger-log4j-1/src/main/java/net/openhft/chronicle/logger/log4j1/internal/package-info.java @@ -3,8 +3,8 @@ * Internal classes shall never be used directly. * Direct use of these classes is unsupported and may break without notice. *

        - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

          *
        • Casting to
          • *
        • Reflection of any kind
          • diff --git a/logger-log4j-1/src/test/java/net/openhft/chronicle/logger/log4j1/Log4j1TestBase.java b/logger-log4j-1/src/test/java/net/openhft/chronicle/logger/log4j1/Log4j1TestBase.java index 5e85e1335..a2e1484da 100644 --- a/logger-log4j-1/src/test/java/net/openhft/chronicle/logger/log4j1/Log4j1TestBase.java +++ b/logger-log4j-1/src/test/java/net/openhft/chronicle/logger/log4j1/Log4j1TestBase.java @@ -15,8 +15,6 @@ */ package net.openhft.chronicle.logger.log4j1; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; import net.openhft.chronicle.logger.ChronicleLogLevel; import org.slf4j.Logger; diff --git a/logger-log4j-1/src/test/resources/log4j.xml b/logger-log4j-1/src/test/resources/log4j.xml index 5e444018a..1da23d674 100644 --- a/logger-log4j-1/src/test/resources/log4j.xml +++ b/logger-log4j-1/src/test/resources/log4j.xml @@ -25,8 +25,8 @@ - + @@ -34,10 +34,10 @@ - + - + @@ -56,8 +56,8 @@ - - + + \ No newline at end of file diff --git a/logger-log4j-2/pom.xml b/logger-log4j-2/pom.xml index 26ab9410c..c098a3011 100644 --- a/logger-log4j-2/pom.xml +++ b/logger-log4j-2/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -156,4 +157,156 @@ master + + + + code-review + + false + + + 0.7183 + 0.7142 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-log4j-2/src/main/config/checkstyle.xml b/logger-log4j-2/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-log4j-2/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-log4j-2/src/main/config/pmd-exclude.properties b/logger-log4j-2/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..094f70963 --- /dev/null +++ b/logger-log4j-2/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-log4j-2 module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-log4j-2/src/main/config/spotbugs-exclude.xml b/logger-log4j-2/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..c540b547d --- /dev/null +++ b/logger-log4j-2/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/ChronicleAppender.java b/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/ChronicleAppender.java index 9ac96b707..6e48399ca 100644 --- a/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/ChronicleAppender.java +++ b/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/ChronicleAppender.java @@ -59,11 +59,11 @@ public ChronicleAppender(final String name, final Filter filter, final String pa /** * Factory used by the plugin framework to create the appender. * - * @param name mandatory appender name - * @param path Chronicle Queue path - * @param wireType optional queue wire type + * @param name mandatory appender name + * @param path Chronicle Queue path + * @param wireType optional queue wire type * @param chronicleConfig optional queue configuration - * @param filter optional filter + * @param filter optional filter * @return the configured appender or {@code null} when name or path are missing */ @PluginFactory diff --git a/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/internal/package-info.java b/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/internal/package-info.java index 87a764ce4..62ae60753 100644 --- a/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/internal/package-info.java +++ b/logger-log4j-2/src/main/java/net/openhft/chronicle/logger/log4j2/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any and all sub-packages contains strictly internal classes for this Chronicle library. * Internal classes shall never be used directly. *

          - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

            *
          • Casting to
            • *
          • Reflection of any kind
            • diff --git a/logger-logback/pom.xml b/logger-logback/pom.xml index de8ac3956..5e48c67f5 100644 --- a/logger-logback/pom.xml +++ b/logger-logback/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -124,4 +125,156 @@ scm:git:git@github.com:OpenHFT/Chronicle-Logger.git master + + + + code-review + + false + + + 0.7066 + 0.6428 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-logback/src/main/config/checkstyle.xml b/logger-logback/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-logback/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-logback/src/main/config/pmd-exclude.properties b/logger-logback/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..583ab5dd1 --- /dev/null +++ b/logger-logback/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-logback module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-logback/src/main/config/spotbugs-exclude.xml b/logger-logback/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..18dbf2287 --- /dev/null +++ b/logger-logback/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/logger-logback/src/main/java/net/openhft/chronicle/logger/logback/internal/package-info.java b/logger-logback/src/main/java/net/openhft/chronicle/logger/logback/internal/package-info.java index 1e8fedf83..513432e49 100644 --- a/logger-logback/src/main/java/net/openhft/chronicle/logger/logback/internal/package-info.java +++ b/logger-logback/src/main/java/net/openhft/chronicle/logger/logback/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any sub-package contain classes strictly for internal use by this Chronicle library. * Internal classes shall never be used directly. *

            - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

              *
            • Casting to
              • *
            • Reflection of any kind
              • diff --git a/logger-logback/src/test/java/net/openhft/chronicle/logger/logback/LogbackTestBase.java b/logger-logback/src/test/java/net/openhft/chronicle/logger/logback/LogbackTestBase.java index bd0d416c5..f65e93846 100644 --- a/logger-logback/src/test/java/net/openhft/chronicle/logger/logback/LogbackTestBase.java +++ b/logger-logback/src/test/java/net/openhft/chronicle/logger/logback/LogbackTestBase.java @@ -16,8 +16,6 @@ package net.openhft.chronicle.logger.logback; import ch.qos.logback.classic.LoggerContext; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; import net.openhft.chronicle.logger.ChronicleLogLevel; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/logger-logback/src/test/resources/logback-chronicle-binary-appender.xml b/logger-logback/src/test/resources/logback-chronicle-binary-appender.xml index 081f725d8..ae80bd0fc 100644 --- a/logger-logback/src/test/resources/logback-chronicle-binary-appender.xml +++ b/logger-logback/src/test/resources/logback-chronicle-binary-appender.xml @@ -20,8 +20,8 @@ - + %d %contextName [%t] %level %logger{36} - %msg%n @@ -31,8 +31,8 @@ - + ${java.io.tmpdir}/chronicle-logback/binary-chronicle false false @@ -47,11 +47,11 @@ - + - + diff --git a/logger-logback/src/test/resources/logback-chronicle-config.xml b/logger-logback/src/test/resources/logback-chronicle-config.xml index b3b1f8757..9d819245a 100644 --- a/logger-logback/src/test/resources/logback-chronicle-config.xml +++ b/logger-logback/src/test/resources/logback-chronicle-config.xml @@ -20,8 +20,8 @@ - + %d %contextName [%t] %level %logger{36} - %msg%n @@ -31,8 +31,8 @@ - + ${java.io.tmpdir}/chronicle-logback/config-binary-chronicle false false @@ -52,11 +52,11 @@ - + - + diff --git a/logger-slf4j-2/pom.xml b/logger-slf4j-2/pom.xml index 446c697f9..05f485be2 100644 --- a/logger-slf4j-2/pom.xml +++ b/logger-slf4j-2/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -116,4 +117,156 @@ master + + + + code-review + + false + + + 0.6350 + 0.4122 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-slf4j-2/src/main/config/checkstyle.xml b/logger-slf4j-2/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-slf4j-2/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-slf4j-2/src/main/config/pmd-exclude.properties b/logger-slf4j-2/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..bcdb90fc3 --- /dev/null +++ b/logger-slf4j-2/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-slf4j-2 module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-slf4j-2/src/main/config/spotbugs-exclude.xml b/logger-slf4j-2/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..c11a96506 --- /dev/null +++ b/logger-slf4j-2/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/ChronicleLogger.java b/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/ChronicleLogger.java index a2aba0222..47c585517 100644 --- a/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/ChronicleLogger.java +++ b/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/ChronicleLogger.java @@ -27,7 +27,7 @@ import net.openhft.chronicle.logger.ChronicleLogLevel; import net.openhft.chronicle.logger.ChronicleLogWriter; -@SuppressWarnings({"deprecation","serial"}) +@SuppressWarnings({"deprecation", "serial"}) public final class ChronicleLogger extends org.slf4j.helpers.MarkerIgnoringBase { private static final long serialVersionUID = 1L; diff --git a/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/internal/package-info.java b/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/internal/package-info.java index 4381ee9e5..d7b0c1869 100644 --- a/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/internal/package-info.java +++ b/logger-slf4j-2/src/main/java/net/openhft/chronicle/logger/slf4j2/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any and all sub-packages contains strictly internal classes for this Chronicle library. * Internal classes shall never be used directly. *

              - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

                *
              • Casting to
                • *
              • Reflection of any kind
                • diff --git a/logger-slf4j-2/src/main/java/org/slf4j/impl/ChronicleServiceProvider.java b/logger-slf4j-2/src/main/java/org/slf4j/impl/ChronicleServiceProvider.java index 47eb0850b..3b3475f3c 100644 --- a/logger-slf4j-2/src/main/java/org/slf4j/impl/ChronicleServiceProvider.java +++ b/logger-slf4j-2/src/main/java/org/slf4j/impl/ChronicleServiceProvider.java @@ -27,7 +27,9 @@ public class ChronicleServiceProvider implements SLF4JServiceProvider { // to avoid constant folding by the compiler, this field must *not* be final public static String REQUESTED_API_VERSION = "2.0.99"; // !final - public ChronicleServiceProvider() {} + public ChronicleServiceProvider() { + } + /** * The ILoggerFactory instance returned by the {@link #getLoggerFactory} * method should always be the same object diff --git a/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLogger.java b/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLogger.java index daa332484..ce1fdc616 100644 --- a/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLogger.java +++ b/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLogger.java @@ -24,11 +24,6 @@ */ package org.slf4j.impl; -import java.io.PrintStream; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; - import net.openhft.chronicle.core.Jvm; import org.slf4j.Logger; import org.slf4j.Marker; @@ -39,6 +34,11 @@ import org.slf4j.helpers.NormalizedParameters; import org.slf4j.spi.LocationAwareLogger; +import java.io.PrintStream; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + /** *

                * Simple implementation of {@link Logger} that sends all enabled log messages, @@ -192,9 +192,13 @@ static void init() { CONFIG_PARAMS.init(); } - /** The current log level */ + /** + * The current log level + */ protected int currentLogLevel = LOG_LEVEL_INFO; - /** The short name of this simple log instance */ + /** + * The short name of this simple log instance + */ private transient String shortLogName = null; /** @@ -328,27 +332,37 @@ protected boolean isLevelEnabled(int logLevel) { return (logLevel >= currentLogLevel); } - /** Are {@code trace} messages currently enabled? */ + /** + * Are {@code trace} messages currently enabled? + */ public boolean isTraceEnabled() { return isLevelEnabled(LOG_LEVEL_TRACE); } - /** Are {@code debug} messages currently enabled? */ + /** + * Are {@code debug} messages currently enabled? + */ public boolean isDebugEnabled() { return isLevelEnabled(LOG_LEVEL_DEBUG); } - /** Are {@code info} messages currently enabled? */ + /** + * Are {@code info} messages currently enabled? + */ public boolean isInfoEnabled() { return isLevelEnabled(LOG_LEVEL_INFO); } - /** Are {@code warn} messages currently enabled? */ + /** + * Are {@code warn} messages currently enabled? + */ public boolean isWarnEnabled() { return isLevelEnabled(LOG_LEVEL_WARN); } - /** Are {@code error} messages currently enabled? */ + /** + * Are {@code error} messages currently enabled? + */ public boolean isErrorEnabled() { return isLevelEnabled(LOG_LEVEL_ERROR); } @@ -358,11 +372,11 @@ public boolean isErrorEnabled() { * {@link org.slf4j.helpers.AbstractLogger#handleNormalizedLoggingCall(Level, Marker, String, Object[], Throwable) AbstractLogger#handleNormalizedLoggingCall} * } * - * @param level the SLF4J level for this event - * @param marker The marker to be used for this event, may be null. + * @param level the SLF4J level for this event + * @param marker The marker to be used for this event, may be null. * @param messagePattern The message pattern which will be parsed and formatted - * @param arguments the array of arguments to be formatted, may be null - * @param throwable The exception whose stack trace should be logged, may be null + * @param arguments the array of arguments to be formatted, may be null + * @param throwable The exception whose stack trace should be logged, may be null */ @Override protected void handleNormalizedLoggingCall(Level level, Marker marker, String messagePattern, Object[] arguments, Throwable throwable) { diff --git a/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java b/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java index 0e760e789..55d355c0b 100644 --- a/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java +++ b/logger-slf4j-2/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java @@ -1,5 +1,8 @@ package org.slf4j.impl; +import org.slf4j.helpers.Util; +import org.slf4j.impl.OutputChoice.OutputChoiceType; + import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.InputStream; @@ -9,22 +12,17 @@ import java.text.SimpleDateFormat; import java.util.Properties; -import org.slf4j.helpers.Util; -import org.slf4j.impl.OutputChoice.OutputChoiceType; - /** * This class holds configuration values for {@link SimpleLogger}. The * values are computed at runtime. See {@link SimpleLogger} documentation for * more information. *

                Properties are loaded from {@value #CONFIGURATION_FILE} on the classpath. The thread context class loader is tried first and the system class loader is used if needed. Missing or malformed entries fall back to the defaults defined in this class.

                * - * * @author Ceki Gülcü * @author Scott Sanders * @author Rod Waldhoff * @author Robert Burrell Donkin * @author Cédrik LIME - * * @since 1.7.25 */ public class SimpleLoggerConfiguration { @@ -47,6 +45,7 @@ public class SimpleLoggerConfiguration { /** * See https://jira.qos.ch/browse/SLF4J-499 + * * @since 1.7.33 and 2.0.0-alpha6 */ private static final boolean SHOW_THREAD_ID_DEFAULT = false; @@ -102,6 +101,7 @@ void init() { } } } + /** * Load optional properties from {@value #CONFIGURATION_FILE}. * The thread context class loader is queried first and the system diff --git a/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/ChronicleLoggerBehaviourTest.java b/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/ChronicleLoggerBehaviourTest.java new file mode 100644 index 000000000..a7a34bd01 --- /dev/null +++ b/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/ChronicleLoggerBehaviourTest.java @@ -0,0 +1,141 @@ +package net.openhft.chronicle.logger.slf4j2; + +import net.openhft.chronicle.logger.ChronicleLogLevel; +import net.openhft.chronicle.logger.ChronicleLogWriter; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.*; + +/** + * Mirrors the SLF4J 1.x behaviour test to ensure the 2.x logger forwards data + * in the same way. + */ +public class ChronicleLoggerBehaviourTest { + + private String originalThreadName; + + @Before + public void captureThreadName() { + originalThreadName = Thread.currentThread().getName(); + Thread.currentThread().setName("slf4j2-behaviour-test"); + } + + @After + public void restoreThreadName() { + Thread.currentThread().setName(originalThreadName); + } + + @Test + public void filtersLevelsAndPreservesArguments() { + RecordingWriter writer = new RecordingWriter(); + ChronicleLogger logger = new ChronicleLogger(writer, "behaviour-logger", ChronicleLogLevel.INFO); + + assertFalse(logger.isTraceEnabled()); + assertFalse(logger.isDebugEnabled()); + assertTrue(logger.isInfoEnabled()); + assertTrue(logger.isWarnEnabled()); + assertTrue(logger.isErrorEnabled()); + + logger.trace("trace skipped"); + logger.debug("debug skipped"); + assertEquals(0, writer.events.size()); + + logger.info("info message"); + logger.info("info one arg {}", 42); + logger.info("info two args {} {}", "lhs", "rhs"); + IllegalStateException warningThrowable = new IllegalStateException("warn"); + logger.warn("warn with throwable", warningThrowable); + logger.warn("warn contextual {}", "ctx", new IllegalArgumentException("warn-ctx")); + logger.error("error varargs {} {}", 1, 2); + logger.error("error arg and throwable {}", "payload", new RuntimeException("kaboom")); + + List events = writer.events; + assertEquals(7, events.size()); + + LoggedEvent first = events.get(0); + assertEquals(ChronicleLogLevel.INFO, first.level); + assertEquals("behaviour-logger", first.loggerName); + assertEquals("slf4j2-behaviour-test", first.threadName); + assertEquals("info message", first.message); + assertNull(first.throwable); + assertEquals(0, first.args.length); + + LoggedEvent second = events.get(1); + assertEquals("info one arg {}", second.message); + assertArrayEquals(new Object[]{42}, second.args); + + LoggedEvent third = events.get(2); + assertEquals("info two args {} {}", third.message); + assertArrayEquals(new Object[]{"lhs", "rhs"}, third.args); + + LoggedEvent fourth = events.get(3); + assertEquals(ChronicleLogLevel.WARN, fourth.level); + assertEquals(warningThrowable, fourth.throwable); + + LoggedEvent fifth = events.get(4); + assertEquals("warn contextual {}", fifth.message); + assertArrayEquals(new Object[]{"ctx"}, fifth.args); + assertTrue(fifth.throwable instanceof IllegalArgumentException); + assertEquals("warn-ctx", fifth.throwable.getMessage()); + + LoggedEvent sixth = events.get(5); + assertEquals(ChronicleLogLevel.ERROR, sixth.level); + assertArrayEquals(new Object[]{1, 2}, sixth.args); + assertNull(sixth.throwable); + + LoggedEvent seventh = events.get(6); + assertEquals("error arg and throwable {}", seventh.message); + assertArrayEquals(new Object[]{"payload"}, seventh.args); + assertTrue(seventh.throwable instanceof RuntimeException); + assertEquals("kaboom", seventh.throwable.getMessage()); + } + + private static final class RecordingWriter implements ChronicleLogWriter { + private final List events = new ArrayList<>(); + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message) { + events.add(new LoggedEvent(level, threadName, loggerName, message, null, new Object[0])); + } + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message, Throwable throwable, Object... args) { + Object[] safeArgs = args == null ? new Object[0] : Arrays.copyOf(args, args.length); + events.add(new LoggedEvent(level, threadName, loggerName, message, throwable, safeArgs)); + } + + @Override + public void close() { + // no-op + } + } + + private static final class LoggedEvent { + private final ChronicleLogLevel level; + private final String threadName; + private final String loggerName; + private final String message; + private final Throwable throwable; + private final Object[] args; + + LoggedEvent(ChronicleLogLevel level, + String threadName, + String loggerName, + String message, + Throwable throwable, + Object[] args) { + this.level = level; + this.threadName = threadName; + this.loggerName = loggerName; + this.message = message; + this.throwable = throwable; + this.args = args; + } + } +} diff --git a/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/Slf4jTestBase.java b/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/Slf4jTestBase.java index a0cf407bf..e02dc9e06 100644 --- a/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/Slf4jTestBase.java +++ b/logger-slf4j-2/src/test/java/net/openhft/chronicle/logger/slf4j2/Slf4jTestBase.java @@ -16,7 +16,6 @@ package net.openhft.chronicle.logger.slf4j2; import net.openhft.chronicle.logger.ChronicleLogLevel; -import net.openhft.chronicle.logger.slf4j2.ChronicleLoggerFactory; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/logger-slf4j-2/src/test/resources/chronicle.logger.perf.properties b/logger-slf4j-2/src/test/resources/chronicle.logger.perf.properties index 7d48b381b..71117cbe5 100644 --- a/logger-slf4j-2/src/test/resources/chronicle.logger.perf.properties +++ b/logger-slf4j-2/src/test/resources/chronicle.logger.perf.properties @@ -15,9 +15,7 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - -chronicle.logger.base = ${java.io.tmpdir}/chronicle-slf4j - -chronicle.logger.root.path = ${chronicle.logger.base}/perf -chronicle.logger.root.level = debug -chronicle.logger.root.append = false +chronicle.logger.base=${java.io.tmpdir}/chronicle-slf4j +chronicle.logger.root.path=${chronicle.logger.base}/perf +chronicle.logger.root.level=debug +chronicle.logger.root.append=false diff --git a/logger-slf4j-2/src/test/resources/chronicle.logger.properties b/logger-slf4j-2/src/test/resources/chronicle.logger.properties index 43e00cea3..29a9e2c1a 100644 --- a/logger-slf4j-2/src/test/resources/chronicle.logger.properties +++ b/logger-slf4j-2/src/test/resources/chronicle.logger.properties @@ -15,23 +15,18 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - ################################################################################ # Common ################################################################################ - -chronicle.logger.base = ${java.io.tmpdir}/chronicle-slf4j -chronicle.logger.root.path = ${chronicle.logger.base}/root -chronicle.logger.root.level = debug -chronicle.logger.root.append = false - +chronicle.logger.base=${java.io.tmpdir}/chronicle-slf4j +chronicle.logger.root.path=${chronicle.logger.base}/root +chronicle.logger.root.level=debug +chronicle.logger.root.append=false ################################################################################ # Loggers ################################################################################ - # logger : Logger1 -chronicle.logger.logger_1.path = ${chronicle.logger.base}/logger_1 -chronicle.logger.logger_1.level = info - +chronicle.logger.logger_1.path=${chronicle.logger.base}/logger_1 +chronicle.logger.logger_1.level=info # logger : impl -chronicle.logger.readwrite.path = ${chronicle.logger.base}/readwrite +chronicle.logger.readwrite.path=${chronicle.logger.base}/readwrite diff --git a/logger-slf4j/pom.xml b/logger-slf4j/pom.xml index 8fbcfa711..fb2bec888 100644 --- a/logger-slf4j/pom.xml +++ b/logger-slf4j/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -115,4 +116,156 @@ master + + + + code-review + + false + + + 0.5615 + 0.4067 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + true + Max + Low + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-slf4j/src/main/config/checkstyle.xml b/logger-slf4j/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-slf4j/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-slf4j/src/main/config/pmd-exclude.properties b/logger-slf4j/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..c12b2c353 --- /dev/null +++ b/logger-slf4j/src/main/config/pmd-exclude.properties @@ -0,0 +1,6 @@ +# Chronicle Logger logger-slf4j module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. +# CLG-FN-003: SLF4J Simple logger sources are vendored unmodified; style deviations remain upstream. +org/slf4j/impl/SimpleLogger.java=.* +org/slf4j/impl/SimpleLoggerConfiguration.java=.* +org/slf4j/impl/StaticLoggerBinder.java=.* diff --git a/logger-slf4j/src/main/config/spotbugs-exclude.xml b/logger-slf4j/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..71cf5b571 --- /dev/null +++ b/logger-slf4j/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,55 @@ + + + + + + + + + + + + + + + + + SLF4J upstream implementation retained verbatim (CLG-FN-003); behaviour changes must be coordinated + upstream. + + + + + + + + + + + + + + + SLF4J upstream implementation retained verbatim (CLG-FN-003); behaviour changes must be coordinated + upstream. + + + + + + + + + + + + SLF4J upstream implementation retained verbatim (CLG-FN-003); behaviour changes must be coordinated + upstream. + + + + + diff --git a/logger-slf4j/src/main/java/net/openhft/chronicle/logger/slf4j/internal/package-info.java b/logger-slf4j/src/main/java/net/openhft/chronicle/logger/slf4j/internal/package-info.java index b5631501f..3578089a4 100644 --- a/logger-slf4j/src/main/java/net/openhft/chronicle/logger/slf4j/internal/package-info.java +++ b/logger-slf4j/src/main/java/net/openhft/chronicle/logger/slf4j/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any and all sub-packages contains strictly internal classes for this Chronicle library. * Internal classes shall never be used directly. *

                - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

                  *
                • Casting to
                  • *
                • Reflection of any kind
                  • diff --git a/logger-slf4j/src/main/java/org/slf4j/impl/OutputChoice.java b/logger-slf4j/src/main/java/org/slf4j/impl/OutputChoice.java index 776d52cdd..29ce3fd6e 100644 --- a/logger-slf4j/src/main/java/org/slf4j/impl/OutputChoice.java +++ b/logger-slf4j/src/main/java/org/slf4j/impl/OutputChoice.java @@ -47,16 +47,16 @@ enum OutputChoiceType { PrintStream getTargetPrintStream() { switch (outputChoiceType) { - case SYS_OUT: - return System.out; - case SYS_ERR: - return System.err; - case CACHED_SYS_ERR: - case CACHED_SYS_OUT: - case FILE: - return targetPrintStream; - default: - throw new IllegalArgumentException(); + case SYS_OUT: + return System.out; + case SYS_ERR: + return System.err; + case CACHED_SYS_ERR: + case CACHED_SYS_OUT: + case FILE: + return targetPrintStream; + default: + throw new IllegalArgumentException(); } } } diff --git a/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLogger.java b/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLogger.java index 0cbbbf9f4..b371dacf0 100644 --- a/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLogger.java +++ b/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLogger.java @@ -24,9 +24,6 @@ */ package org.slf4j.impl; -import java.io.PrintStream; -import java.util.Date; - import org.slf4j.Logger; import org.slf4j.event.LoggingEvent; import org.slf4j.helpers.FormattingTuple; @@ -34,6 +31,9 @@ import org.slf4j.helpers.MessageFormatter; import org.slf4j.spi.LocationAwareLogger; +import java.io.PrintStream; +import java.util.Date; + /** * Lightweight {@link Logger} writing to a single {@link PrintStream}. The * logger initialises its configuration once, then each call checks the level, @@ -102,9 +102,13 @@ static void init() { CONFIG_PARAMS.init(); } - /** The current log level */ + /** + * The current log level + */ protected int currentLogLevel = LOG_LEVEL_INFO; - /** The short name of this simple log instance */ + /** + * The short name of this simple log instance + */ private transient String shortLogName = null; /** @@ -163,12 +167,9 @@ String recursivelyComputeLevelString() { * This is our internal implementation for logging regular * (non-parameterized) log messages. * - * @param level - * One of the LOG_LEVEL_XXX constants defining the log level - * @param message - * The message itself - * @param t - * The exception whose stack trace should be logged + * @param level One of the LOG_LEVEL_XXX constants defining the log level + * @param message The message itself + * @param t The exception whose stack trace should be logged */ private void log(int level, String message, Throwable t) { if (!isLevelEnabled(level)) { @@ -223,16 +224,16 @@ private void log(int level, String message, Throwable t) { protected String renderLevel(int level) { switch (level) { - case LOG_LEVEL_TRACE: - return "TRACE"; - case LOG_LEVEL_DEBUG: - return ("DEBUG"); - case LOG_LEVEL_INFO: - return "INFO"; - case LOG_LEVEL_WARN: - return CONFIG_PARAMS.warnLevelString; - case LOG_LEVEL_ERROR: - return "ERROR"; + case LOG_LEVEL_TRACE: + return "TRACE"; + case LOG_LEVEL_DEBUG: + return ("DEBUG"); + case LOG_LEVEL_INFO: + return "INFO"; + case LOG_LEVEL_WARN: + return CONFIG_PARAMS.warnLevelString; + case LOG_LEVEL_ERROR: + return "ERROR"; } throw new IllegalStateException("Unrecognized level [" + level + "]"); } @@ -289,8 +290,7 @@ private void formatAndLog(int level, String format, Object... arguments) { /** * Is the given log level currently enabled? * - * @param logLevel - * is this level enabled? + * @param logLevel is this level enabled? */ protected boolean isLevelEnabled(int logLevel) { // log level are numerically ordered so can use simple numeric @@ -298,7 +298,9 @@ protected boolean isLevelEnabled(int logLevel) { return (logLevel >= currentLogLevel); } - /** Are {@code trace} messages currently enabled? */ + /** + * Are {@code trace} messages currently enabled? + */ public boolean isTraceEnabled() { return isLevelEnabled(LOG_LEVEL_TRACE); } @@ -335,12 +337,16 @@ public void trace(String format, Object... argArray) { formatAndLog(LOG_LEVEL_TRACE, format, argArray); } - /** Log a message of level TRACE, including an exception. */ + /** + * Log a message of level TRACE, including an exception. + */ public void trace(String msg, Throwable t) { log(LOG_LEVEL_TRACE, msg, t); } - /** Are {@code debug} messages currently enabled? */ + /** + * Are {@code debug} messages currently enabled? + */ public boolean isDebugEnabled() { return isLevelEnabled(LOG_LEVEL_DEBUG); } @@ -377,12 +383,16 @@ public void debug(String format, Object... argArray) { formatAndLog(LOG_LEVEL_DEBUG, format, argArray); } - /** Log a message of level DEBUG, including an exception. */ + /** + * Log a message of level DEBUG, including an exception. + */ public void debug(String msg, Throwable t) { log(LOG_LEVEL_DEBUG, msg, t); } - /** Are {@code info} messages currently enabled? */ + /** + * Are {@code info} messages currently enabled? + */ public boolean isInfoEnabled() { return isLevelEnabled(LOG_LEVEL_INFO); } @@ -419,12 +429,16 @@ public void info(String format, Object... argArray) { formatAndLog(LOG_LEVEL_INFO, format, argArray); } - /** Log a message of level INFO, including an exception. */ + /** + * Log a message of level INFO, including an exception. + */ public void info(String msg, Throwable t) { log(LOG_LEVEL_INFO, msg, t); } - /** Are {@code warn} messages currently enabled? */ + /** + * Are {@code warn} messages currently enabled? + */ public boolean isWarnEnabled() { return isLevelEnabled(LOG_LEVEL_WARN); } @@ -461,12 +475,16 @@ public void warn(String format, Object... argArray) { formatAndLog(LOG_LEVEL_WARN, format, argArray); } - /** Log a message of level WARN, including an exception. */ + /** + * Log a message of level WARN, including an exception. + */ public void warn(String msg, Throwable t) { log(LOG_LEVEL_WARN, msg, t); } - /** Are {@code error} messages currently enabled? */ + /** + * Are {@code error} messages currently enabled? + */ public boolean isErrorEnabled() { return isLevelEnabled(LOG_LEVEL_ERROR); } @@ -503,7 +521,9 @@ public void error(String format, Object... argArray) { formatAndLog(LOG_LEVEL_ERROR, format, argArray); } - /** Log a message of level ERROR, including an exception. */ + /** + * Log a message of level ERROR, including an exception. + */ public void error(String msg, Throwable t) { log(LOG_LEVEL_ERROR, msg, t); } diff --git a/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java b/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java index e648710e3..8f0fbe06a 100644 --- a/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java +++ b/logger-slf4j/src/main/java/org/slf4j/impl/SimpleLoggerConfiguration.java @@ -1,5 +1,8 @@ package org.slf4j.impl; +import org.slf4j.helpers.Util; +import org.slf4j.impl.OutputChoice.OutputChoiceType; + import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.InputStream; @@ -9,29 +12,28 @@ import java.text.SimpleDateFormat; import java.util.Properties; -import org.slf4j.helpers.Util; -import org.slf4j.impl.OutputChoice.OutputChoiceType; - /** * This class holds configuration values for {@link SimpleLogger}. The * values are computed at runtime. See {@link SimpleLogger} documentation for * more information. * - * * @author Ceki Gülcü * @author Scott Sanders * @author Rod Waldhoff * @author Robert Burrell Donkin * @author Cédrik LIME - * * @since 1.7.25 */ public class SimpleLoggerConfiguration { - /** Name of the properties file searched on the class path. */ + /** + * Name of the properties file searched on the class path. + */ private static final String CONFIGURATION_FILE = "simplelogger.properties"; - /** Default level used when no property is set (INFO). */ + /** + * Default level used when no property is set (INFO). + */ static int DEFAULT_LOG_LEVEL_DEFAULT = SimpleLogger.LOG_LEVEL_INFO; /** * Runtime default log level as configured by @@ -39,47 +41,81 @@ public class SimpleLoggerConfiguration { */ int defaultLogLevel = DEFAULT_LOG_LEVEL_DEFAULT; - /** Default for {@code org.slf4j.simpleLogger.showDateTime} (false). */ + /** + * Default for {@code org.slf4j.simpleLogger.showDateTime} (false). + */ private static final boolean SHOW_DATE_TIME_DEFAULT = false; - /** Whether to include a time stamp in log lines. */ + /** + * Whether to include a time stamp in log lines. + */ boolean showDateTime = SHOW_DATE_TIME_DEFAULT; - /** Default for {@code org.slf4j.simpleLogger.dateTimeFormat}. */ + /** + * Default for {@code org.slf4j.simpleLogger.dateTimeFormat}. + */ private static final String DATE_TIME_FORMAT_STR_DEFAULT = null; - /** Format string used to render the date, or {@code null} for relative time. */ + /** + * Format string used to render the date, or {@code null} for relative time. + */ private static String dateTimeFormatStr = DATE_TIME_FORMAT_STR_DEFAULT; - /** Formatter built from {@link #dateTimeFormatStr} when initialised. */ + /** + * Formatter built from {@link #dateTimeFormatStr} when initialised. + */ DateFormat dateFormatter = null; - /** Default for {@code org.slf4j.simpleLogger.showThreadName} (true). */ + /** + * Default for {@code org.slf4j.simpleLogger.showThreadName} (true). + */ private static final boolean SHOW_THREAD_NAME_DEFAULT = true; - /** Whether the thread name should appear in output. */ + /** + * Whether the thread name should appear in output. + */ boolean showThreadName = SHOW_THREAD_NAME_DEFAULT; - /** Default for {@code org.slf4j.simpleLogger.showLogName} (true). */ + /** + * Default for {@code org.slf4j.simpleLogger.showLogName} (true). + */ final static boolean SHOW_LOG_NAME_DEFAULT = true; - /** Whether the logger name should appear in output. */ + /** + * Whether the logger name should appear in output. + */ boolean showLogName = SHOW_LOG_NAME_DEFAULT; - /** Default for {@code org.slf4j.simpleLogger.showShortLogName} (false). */ + /** + * Default for {@code org.slf4j.simpleLogger.showShortLogName} (false). + */ private static final boolean SHOW_SHORT_LOG_NAME_DEFAULT = false; - /** If true only the last logger name component is printed. */ + /** + * If true only the last logger name component is printed. + */ boolean showShortLogName = SHOW_SHORT_LOG_NAME_DEFAULT; - /** Default for {@code org.slf4j.simpleLogger.levelInBrackets} (false). */ + /** + * Default for {@code org.slf4j.simpleLogger.levelInBrackets} (false). + */ private static final boolean LEVEL_IN_BRACKETS_DEFAULT = false; - /** Print the level surrounded by brackets. */ + /** + * Print the level surrounded by brackets. + */ boolean levelInBrackets = LEVEL_IN_BRACKETS_DEFAULT; - /** Default for {@code org.slf4j.simpleLogger.logFile} (System.err). */ + /** + * Default for {@code org.slf4j.simpleLogger.logFile} (System.err). + */ private static String LOG_FILE_DEFAULT = "System.err"; - /** Destination for log output. */ + /** + * Destination for log output. + */ private String logFile = LOG_FILE_DEFAULT; - /** Actual output target decided after initialisation. */ + /** + * Actual output target decided after initialisation. + */ OutputChoice outputChoice = null; - /** Default for {@code org.slf4j.simpleLogger.cacheOutputStream} (false). */ + /** + * Default for {@code org.slf4j.simpleLogger.cacheOutputStream} (false). + */ private static final boolean CACHE_OUTPUT_STREAM_DEFAULT = false; /** * When true the {@code System.out/err} stream is cached on start up rather @@ -87,12 +123,18 @@ public class SimpleLoggerConfiguration { */ private boolean cacheOutputStream = CACHE_OUTPUT_STREAM_DEFAULT; - /** Default text used for the WARN level. */ + /** + * Default text used for the WARN level. + */ private static final String WARN_LEVELS_STRING_DEFAULT = "WARN"; - /** String to write for the warn level. Configured via {@code org.slf4j.simpleLogger.warnLevelString}. */ + /** + * String to write for the warn level. Configured via {@code org.slf4j.simpleLogger.warnLevelString}. + */ String warnLevelString = WARN_LEVELS_STRING_DEFAULT; - /** Properties loaded from {@link #CONFIGURATION_FILE}. */ + /** + * Properties loaded from {@link #CONFIGURATION_FILE}. + */ private final Properties properties = new Properties(); void init() { @@ -165,7 +207,7 @@ String getStringProperty(String name) { try { prop = System.getProperty(name); } catch (SecurityException e) { - ; // none // Ignore + ; // none // Ignore } return (prop == null) ? properties.getProperty(name) : prop; } diff --git a/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/ChronicleLoggerBehaviourTest.java b/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/ChronicleLoggerBehaviourTest.java new file mode 100644 index 000000000..889a27461 --- /dev/null +++ b/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/ChronicleLoggerBehaviourTest.java @@ -0,0 +1,146 @@ +package net.openhft.chronicle.logger.slf4j; + +import net.openhft.chronicle.logger.ChronicleLogLevel; +import net.openhft.chronicle.logger.ChronicleLogWriter; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.*; + +/** + * Verifies that {@link ChronicleLogger} filters levels correctly and forwards + * arguments plus throwables with the expected shape. + */ +public class ChronicleLoggerBehaviourTest { + + private String originalThreadName; + + @Before + public void captureThreadName() { + originalThreadName = Thread.currentThread().getName(); + Thread.currentThread().setName("slf4j-behaviour-test"); + } + + @After + public void restoreThreadName() { + Thread.currentThread().setName(originalThreadName); + } + + @Test + public void filtersLevelsAndPreservesArguments() { + RecordingWriter writer = new RecordingWriter(); + ChronicleLogger logger = new ChronicleLogger(writer, "behaviour-logger", ChronicleLogLevel.INFO); + + assertFalse("trace should be disabled at INFO threshold", logger.isTraceEnabled()); + assertFalse("debug should be disabled at INFO threshold", logger.isDebugEnabled()); + assertTrue(logger.isInfoEnabled()); + assertTrue(logger.isWarnEnabled()); + assertTrue(logger.isErrorEnabled()); + + // below threshold: no events + logger.trace("trace skipped"); + logger.debug("debug skipped"); + assertEquals(0, writer.events.size()); + + logger.info("info message"); + logger.info("info one arg {}", 42); + logger.info("info two args {} {}", "lhs", "rhs"); + IllegalStateException warningThrowable = new IllegalStateException("warn"); + logger.warn("warn with throwable", warningThrowable); + logger.warn("warn contextual {}", "ctx", new IllegalArgumentException("warn-ctx")); + logger.error("error varargs {} {}", 1, 2); + logger.error("error arg and throwable {}", "payload", new RuntimeException("kaboom")); + + List events = writer.events; + assertEquals("seven events expected", 7, events.size()); + + LoggedEvent first = events.get(0); + assertEquals(ChronicleLogLevel.INFO, first.level); + assertEquals("behaviour-logger", first.loggerName); + assertEquals("slf4j-behaviour-test", first.threadName); + assertEquals("info message", first.message); + assertNull(first.throwable); + assertEquals(0, first.args.length); + + LoggedEvent second = events.get(1); + assertEquals("info one arg {}", second.message); + assertArrayEquals(new Object[]{42}, second.args); + assertNull(second.throwable); + + LoggedEvent third = events.get(2); + assertEquals("info two args {} {}", third.message); + assertArrayEquals(new Object[]{"lhs", "rhs"}, third.args); + + LoggedEvent fourth = events.get(3); + assertEquals(ChronicleLogLevel.WARN, fourth.level); + assertEquals("warn with throwable", fourth.message); + assertEquals(warningThrowable, fourth.throwable); + assertEquals(0, fourth.args.length); + + LoggedEvent fifth = events.get(4); + assertEquals("warn contextual {}", fifth.message); + assertArrayEquals(new Object[]{"ctx"}, fifth.args); + assertTrue(fifth.throwable instanceof IllegalArgumentException); + assertEquals("warn-ctx", fifth.throwable.getMessage()); + + LoggedEvent sixth = events.get(5); + assertEquals(ChronicleLogLevel.ERROR, sixth.level); + assertEquals("error varargs {} {}", sixth.message); + assertArrayEquals(new Object[]{1, 2}, sixth.args); + assertNull(sixth.throwable); + + LoggedEvent seventh = events.get(6); + assertEquals("error arg and throwable {}", seventh.message); + assertArrayEquals(new Object[]{"payload"}, seventh.args); + assertTrue(seventh.throwable instanceof RuntimeException); + assertEquals("kaboom", seventh.throwable.getMessage()); + } + + private static final class RecordingWriter implements ChronicleLogWriter { + private final List events = new ArrayList<>(); + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message) { + events.add(new LoggedEvent(level, threadName, loggerName, message, null, new Object[0])); + } + + @Override + public void write(ChronicleLogLevel level, long timestamp, String threadName, String loggerName, String message, Throwable throwable, Object... args) { + Object[] safeArgs = args == null ? new Object[0] : Arrays.copyOf(args, args.length); + events.add(new LoggedEvent(level, threadName, loggerName, message, throwable, safeArgs)); + } + + @Override + public void close() { + // no-op + } + } + + private static final class LoggedEvent { + private final ChronicleLogLevel level; + private final String threadName; + private final String loggerName; + private final String message; + private final Throwable throwable; + private final Object[] args; + + LoggedEvent(ChronicleLogLevel level, + String threadName, + String loggerName, + String message, + Throwable throwable, + Object[] args) { + this.level = level; + this.threadName = threadName; + this.loggerName = loggerName; + this.message = message; + this.throwable = throwable; + this.args = args; + } + } +} diff --git a/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/Slf4jTestBase.java b/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/Slf4jTestBase.java index 604a7209a..4d7cc8bed 100644 --- a/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/Slf4jTestBase.java +++ b/logger-slf4j/src/test/java/net/openhft/chronicle/logger/slf4j/Slf4jTestBase.java @@ -15,8 +15,6 @@ */ package net.openhft.chronicle.logger.slf4j; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; import net.openhft.chronicle.logger.ChronicleLogLevel; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; diff --git a/logger-slf4j/src/test/resources/chronicle.logger.perf.properties b/logger-slf4j/src/test/resources/chronicle.logger.perf.properties index 7d48b381b..71117cbe5 100644 --- a/logger-slf4j/src/test/resources/chronicle.logger.perf.properties +++ b/logger-slf4j/src/test/resources/chronicle.logger.perf.properties @@ -15,9 +15,7 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - -chronicle.logger.base = ${java.io.tmpdir}/chronicle-slf4j - -chronicle.logger.root.path = ${chronicle.logger.base}/perf -chronicle.logger.root.level = debug -chronicle.logger.root.append = false +chronicle.logger.base=${java.io.tmpdir}/chronicle-slf4j +chronicle.logger.root.path=${chronicle.logger.base}/perf +chronicle.logger.root.level=debug +chronicle.logger.root.append=false diff --git a/logger-slf4j/src/test/resources/chronicle.logger.properties b/logger-slf4j/src/test/resources/chronicle.logger.properties index 43e00cea3..29a9e2c1a 100644 --- a/logger-slf4j/src/test/resources/chronicle.logger.properties +++ b/logger-slf4j/src/test/resources/chronicle.logger.properties @@ -15,23 +15,18 @@ # You should have received a copy of the GNU Lesser General Public License # along with this program. If not, see . # - ################################################################################ # Common ################################################################################ - -chronicle.logger.base = ${java.io.tmpdir}/chronicle-slf4j -chronicle.logger.root.path = ${chronicle.logger.base}/root -chronicle.logger.root.level = debug -chronicle.logger.root.append = false - +chronicle.logger.base=${java.io.tmpdir}/chronicle-slf4j +chronicle.logger.root.path=${chronicle.logger.base}/root +chronicle.logger.root.level=debug +chronicle.logger.root.append=false ################################################################################ # Loggers ################################################################################ - # logger : Logger1 -chronicle.logger.logger_1.path = ${chronicle.logger.base}/logger_1 -chronicle.logger.logger_1.level = info - +chronicle.logger.logger_1.path=${chronicle.logger.base}/logger_1 +chronicle.logger.logger_1.level=info # logger : impl -chronicle.logger.readwrite.path = ${chronicle.logger.base}/readwrite +chronicle.logger.readwrite.path=${chronicle.logger.base}/readwrite diff --git a/logger-tools/pom.xml b/logger-tools/pom.xml index bf27e57c9..2019efbc5 100644 --- a/logger-tools/pom.xml +++ b/logger-tools/pom.xml @@ -18,7 +18,8 @@ ~ --> - + 4.0.0 @@ -128,4 +129,156 @@ master + + + + code-review + + false + + + 0.7341 + 0.5769 + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + ${checkstyle.version} + + + com.puppycrawl.tools + checkstyle + ${puppycrawl.version} + + + net.openhft + chronicle-quality-rules + ${chronicle-quality-rules.version} + + + + + checkstyle + verify + + check + + + + + src/main/config/checkstyle.xml + true + true + warning + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + spotbugs + verify + + check + + + + + Max + Low + true + true + src/main/config/spotbugs-exclude.xml + + + com.h3xstream.findsecbugs + findsecbugs-plugin + ${findsecbugs.version} + + + + + + org.apache.maven.plugins + maven-pmd-plugin + ${maven-pmd-plugin.version} + + + pmd + verify + + check + + + + + true + true + true + ${project.basedir}/src/main/config/pmd-exclude.properties + + + + + org.jacoco + jacoco-maven-plugin + ${jacoco-maven-plugin.version} + + + prepare-agent + + prepare-agent + + + + report + verify + + report + + + + check + verify + + check + + + + + BUNDLE + + + LINE + COVEREDRATIO + ${jacoco.line.coverage} + + + BRANCH + COVEREDRATIO + ${jacoco.branch.coverage} + + + + + + + + + + + + + diff --git a/logger-tools/src/main/config/checkstyle.xml b/logger-tools/src/main/config/checkstyle.xml new file mode 100644 index 000000000..844dd904b --- /dev/null +++ b/logger-tools/src/main/config/checkstyle.xml @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/logger-tools/src/main/config/pmd-exclude.properties b/logger-tools/src/main/config/pmd-exclude.properties new file mode 100644 index 000000000..5e9ad5057 --- /dev/null +++ b/logger-tools/src/main/config/pmd-exclude.properties @@ -0,0 +1,2 @@ +# Chronicle Logger logger-tools module defaults to no PMD exclusions. +# Add entries in the form path=Rule1,Rule2 with requirement IDs when suppression is justified. diff --git a/logger-tools/src/main/config/spotbugs-exclude.xml b/logger-tools/src/main/config/spotbugs-exclude.xml new file mode 100644 index 000000000..77ed0210d --- /dev/null +++ b/logger-tools/src/main/config/spotbugs-exclude.xml @@ -0,0 +1,9 @@ + + + + + + diff --git a/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/ChronicleLogReader.java b/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/ChronicleLogReader.java index 564149f6e..983ae737b 100644 --- a/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/ChronicleLogReader.java +++ b/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/ChronicleLogReader.java @@ -19,6 +19,7 @@ import net.openhft.chronicle.queue.ChronicleQueue; import net.openhft.chronicle.queue.ExcerptTailer; import net.openhft.chronicle.wire.DocumentContext; +import net.openhft.chronicle.wire.ValueIn; import net.openhft.chronicle.wire.Wire; import net.openhft.chronicle.wire.WireType; import org.jetbrains.annotations.NotNull; @@ -121,17 +122,26 @@ public void processLogs(@NotNull ChronicleLogProcessor processor, boolean waitFo String threadName = wire.read("threadName").text(); String loggerName = wire.read("loggerName").text(); String message = wire.read("message").text(); - Throwable th = wire.hasMore() ? wire.read("throwable").throwable(false) : null; + Throwable throwable = null; List argsL = new ArrayList<>(); - if (wire.hasMore()) { - wire.read("args").sequence(argsL, (l, vi) -> { - while (vi.hasNextSequenceItem()) { - l.add(vi.object(Object.class)); - } - }); + StringBuilder field = new StringBuilder(); + while (wire.hasMore()) { + ValueIn valueIn = wire.readEventName(field); + String fieldName = field.toString(); + if ("throwable".equals(fieldName)) { + throwable = valueIn.throwable(false); + } else if ("args".equals(fieldName)) { + valueIn.sequence(argsL, (l, vi) -> { + while (vi.hasNextSequenceItem()) { + l.add(vi.object(Object.class)); + } + }); + } else { + valueIn.skipValue(); + } } - Object[] args = argsL.toArray(new Object[argsL.size()]); - processor.process(timestamp, level, threadName, loggerName, message, th, args); + Object[] args = argsL.toArray(new Object[0]); + processor.process(timestamp, level, loggerName, threadName, message, throwable, args); } } } diff --git a/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/internal/package-info.java b/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/internal/package-info.java index 15f9ea923..0d8231954 100644 --- a/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/internal/package-info.java +++ b/logger-tools/src/main/java/net/openhft/chronicle/logger/tools/internal/package-info.java @@ -2,8 +2,8 @@ * This package and any and all sub-packages contains strictly internal classes for this Chronicle library. * These utilities are not part of the public API and shall never be used directly. *

                  - * Specifically, the following actions (including, but not limited to) are not allowed - * on internal classes and packages: + * Specifically, the following actions (including, but not limited to) are not allowed + * on internal classes and packages: *

                    *
                  • Casting to
                    • *
                  • Reflection of any kind
                    • diff --git a/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleCliToolsTest.java b/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleCliToolsTest.java new file mode 100644 index 000000000..de80c6188 --- /dev/null +++ b/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleCliToolsTest.java @@ -0,0 +1,99 @@ +package net.openhft.chronicle.logger.tools; + +import net.openhft.chronicle.core.io.IOTools; +import net.openhft.chronicle.logger.ChronicleLogLevel; +import net.openhft.chronicle.logger.ChronicleLogWriter; +import net.openhft.chronicle.logger.DefaultChronicleLogWriter; +import net.openhft.chronicle.queue.ChronicleQueue; +import net.openhft.chronicle.wire.WireType; +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.Assert.assertTrue; + +/** + * CLI regression tests for {@link ChroniCat} and {@link ChroniTail}. + */ +public class ChronicleCliToolsTest { + + @Test + public void chroniCatPrintsUsageWhenNoArgumentsProvided() throws UnsupportedEncodingException { + PrintStream originalErr = System.err; + ByteArrayOutputStream err = new ByteArrayOutputStream(); + System.setErr(new PrintStream(err)); + try { + ChroniCat.main(new String[0]); + } finally { + System.setErr(originalErr); + } + String output = err.toString("UTF-8"); + assertTrue(output.contains("Usage: ChroniCat")); + } + + @Test + public void chroniCatPrintsRecordsFromQueue() throws IOException { + Path queuePath = Files.createTempDirectory("chroni-cat"); + try { + try (ChronicleQueue queue = ChronicleQueue.singleBuilder(queuePath).wireType(WireType.BINARY_LIGHT).build()) { + ChronicleLogWriter writer = new DefaultChronicleLogWriter(queue); + writer.write( + ChronicleLogLevel.INFO, + System.currentTimeMillis(), + "cli-thread", + "cli", + "test event {} {}", + null, + "lhs", + "rhs"); + writer.close(); + } + + PrintStream originalOut = System.out; + ByteArrayOutputStream out = new ByteArrayOutputStream(); + System.setOut(new PrintStream(out)); + try { + ChroniCat.main(new String[]{"-w", "binary_light", queuePath.toString()}); + } finally { + System.setOut(originalOut); + } + String output = out.toString("UTF-8"); + assertTrue("expected formatted message in ChroniCat output", output.contains("test event lhs rhs")); + } finally { + IOTools.deleteDirWithFiles(queuePath.toString()); + } + } + + @Test + public void chroniCatPrintsStackTraceWhenWireTypeMissing() throws UnsupportedEncodingException { + PrintStream originalErr = System.err; + ByteArrayOutputStream err = new ByteArrayOutputStream(); + System.setErr(new PrintStream(err)); + try { + ChroniCat.main(new String[]{"-w"}); + } finally { + System.setErr(originalErr); + } + String output = err.toString("UTF-8"); + assertTrue(output.contains("ArrayIndexOutOfBoundsException")); + } + + @Test + public void chroniTailPrintsUsageWhenNoArgumentsProvided() throws UnsupportedEncodingException { + PrintStream originalErr = System.err; + ByteArrayOutputStream err = new ByteArrayOutputStream(); + System.setErr(new PrintStream(err)); + try { + ChroniTail.main(new String[0]); + } finally { + System.setErr(originalErr); + } + String output = err.toString("UTF-8"); + assertTrue(output.contains("Usage: ChroniTail")); + } +} diff --git a/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleLogReaderTest.java b/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleLogReaderTest.java index cbf9167b1..f5e059ced 100644 --- a/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleLogReaderTest.java +++ b/logger-tools/src/test/java/net/openhft/chronicle/logger/tools/ChronicleLogReaderTest.java @@ -1,32 +1,172 @@ package net.openhft.chronicle.logger.tools; -import net.openhft.chronicle.core.OS; -import net.openhft.chronicle.core.util.Time; +import net.openhft.chronicle.core.io.IOTools; +import net.openhft.chronicle.logger.ChronicleLogLevel; +import net.openhft.chronicle.logger.ChronicleLogWriter; +import net.openhft.chronicle.logger.DefaultChronicleLogWriter; +import net.openhft.chronicle.queue.ChronicleQueue; +import net.openhft.chronicle.queue.ExcerptTailer; +import net.openhft.chronicle.wire.DocumentContext; +import net.openhft.chronicle.wire.Wire; import net.openhft.chronicle.wire.WireType; +import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import static java.lang.System.currentTimeMillis; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; public class ChronicleLogReaderTest { + private Path queuePath; + @Before - public void setup() { - System.setProperty( - "logback.configurationFile", - System.getProperty("resources.path") - + "/logback-chronicle-binary-appender.xml"); + public void setup() throws IOException { + queuePath = Files.createTempDirectory("chronicle-log-reader"); } @Test - public void readTest() { - final Logger logger = LoggerFactory.getLogger("binary-chronicle"); - logger.info("test {} {} {}", 1, 100L, 100.123D); - logger.info("test {} {} {}", 2, 100L, 100.123D); - logger.info("test {} {} {}", 3, 100L, 100.123D); + public void readTest() throws Exception { + try (ChronicleQueue cq = ChronicleQueue.singleBuilder(queuePath).wireType(WireType.BINARY_LIGHT).build()) { + ChronicleLogWriter writer = new DefaultChronicleLogWriter(cq); + writer.write( + ChronicleLogLevel.INFO, + currentTimeMillis(), + "thread-1", + "binary-chronicle", + "test {} {} {}", + null, + 1, + 100L, + 100.123D); + writer.write( + ChronicleLogLevel.INFO, + currentTimeMillis(), + "thread-1", + "binary-chronicle", + "test {} {} {}", + null, + 2, + 100L, + 100.123D); + writer.write( + ChronicleLogLevel.INFO, + currentTimeMillis(), + "thread-1", + "binary-chronicle", + "test {} {} {}", + null, + 3, + 100L, + 100.123D); + writer.close(); + } + + // sanity check: queue contains the expected number of entries + try (ChronicleQueue cq = ChronicleQueue.singleBuilder(queuePath).wireType(WireType.BINARY_LIGHT).build()) { + ExcerptTailer tailer = cq.createTailer(); + int count = 0; + while (true) { + try (DocumentContext dc = tailer.readingDocument()) { + Wire wire = dc.wire(); + if (wire == null) { + break; + } + count++; + } + } + assertEquals(3, count); + } + + ChronicleLogReader reader = new ChronicleLogReader(queuePath.toString(), WireType.BINARY_LIGHT); + CapturingProcessor processor = new CapturingProcessor(); + reader.processLogs(processor, false); + + List events = processor.events(); + assertEquals(3, events.size()); + for (int i = 0; i < events.size(); i++) { + LogEvent event = events.get(i); + assertEquals(net.openhft.chronicle.logger.ChronicleLogLevel.INFO, event.level()); + assertEquals("binary-chronicle", event.loggerName()); + assertEquals("thread-1", event.threadName()); + assertEquals("test {} {} {}", event.message()); + assertNull(event.throwable()); + assertEquals(3, event.args().length); + assertEquals(i + 1, ((Number) event.args()[0]).intValue()); + assertEquals(100L, ((Number) event.args()[1]).longValue()); + assertEquals(100.123D, ((Number) event.args()[2]).doubleValue(), 0.0D); + } + } + + @After + public void tearDown() { + if (queuePath != null) { + IOTools.deleteDirWithFiles(queuePath.toString()); + } + } + + private static final class CapturingProcessor implements ChronicleLogProcessor { + private final List captures = new java.util.ArrayList<>(); + + @Override + public void process(long timestamp, net.openhft.chronicle.logger.ChronicleLogLevel level, String loggerName, String threadName, String message, Throwable throwable, Object[] args) { + captures.add(new LogEvent(level, loggerName, threadName, message, throwable, args)); + } + + List events() { + return captures; + } + } + + private static final class LogEvent { + private final net.openhft.chronicle.logger.ChronicleLogLevel level; + private final String loggerName; + private final String threadName; + private final String message; + private final Throwable throwable; + private final Object[] args; + + LogEvent(net.openhft.chronicle.logger.ChronicleLogLevel level, + String loggerName, + String threadName, + String message, + Throwable throwable, + Object[] args) { + this.level = level; + this.loggerName = loggerName; + this.threadName = threadName; + this.message = message; + this.throwable = throwable; + this.args = args; + } + + net.openhft.chronicle.logger.ChronicleLogLevel level() { + return level; + } + + String loggerName() { + return loggerName; + } + + String threadName() { + return threadName; + } + + String message() { + return message; + } - ChronicleLogReader reader = new ChronicleLogReader(OS.getTarget() + "/chronicle-logback/binary-chronicle" + Time.uniqueId(), WireType.BINARY_LIGHT); - reader.processLogs(ChronicleLogReader::printf, false); + Throwable throwable() { + return throwable; + } - //ChroniCat.main(new String[] {OS.getTarget() + "/chronicle-logback/binary-chronicle"}); + Object[] args() { + return args; + } } } diff --git a/logger-tools/src/test/resources/logback-chronicle-binary-appender.xml b/logger-tools/src/test/resources/logback-chronicle-binary-appender.xml index ab32caa23..a28f72391 100644 --- a/logger-tools/src/test/resources/logback-chronicle-binary-appender.xml +++ b/logger-tools/src/test/resources/logback-chronicle-binary-appender.xml @@ -20,14 +20,14 @@ - + %d %contextName [%t] %level %logger{36} - %msg%n - + ${java.io.tmpdir}/chronicle-logback/binary-chronicle false false @@ -45,11 +45,11 @@ - + - + diff --git a/pom.xml b/pom.xml index acb72e3e6..0f60c40f1 100644 --- a/pom.xml +++ b/pom.xml @@ -17,20 +17,33 @@ ~ limitations under the License. ~ --> - + 4.0.0 net.openhft java-parent-pom 1.27ea1 - + chronicle-logger 4.27ea2-SNAPSHOT pom + + 3.6.0 + 8.45.1 + 4.8.6.6 + 1.14.0 + 3.28.0 + 0.8.14 + 0.80 + 0.70 + 1.23ea6 + + OpenHFT/Chronicle-Logger OpenHFT :: High Performance Logging @@ -136,4 +149,3 @@ - diff --git a/src/main/docs/architecture-overview.adoc b/src/main/docs/architecture-overview.adoc new file mode 100644 index 000000000..d3709ce17 --- /dev/null +++ b/src/main/docs/architecture-overview.adoc @@ -0,0 +1,55 @@ += Chronicle Logger Architecture Overview +:toc: +:lang: en-GB + +== Purpose + +This overview summarises how Chronicle Logger modules collaborate to capture, persist, and inspect log events. +It complements the requirement catalogue (`CLG-FN-001` through `CLG-NF-O-003`) and the decision log, offering a shared picture for maintainers and operators. + +== Module Topology + +The repository is split into focused modules that all depend on +`logger-core`: + +* `logger-core` supplies `ChronicleLogWriter`, `ChronicleLogManager`, and configuration helpers (see link:project-requirements.adoc#clg-fn-001[CLG-FN-001]). +* Binding modules (`logger-slf4j`, `logger-slf4j-2`, `logger-logback`, +`logger-log4j-1`, `logger-log4j-2`, `logger-jcl`, `logger-jul`) translate framework-specific APIs into Chronicle writes (link:project-requirements.adoc#clg-fn-003[CLG-FN-003]). +* `logger-tools` exposes reusable readers and CLI utilities (link:project-requirements.adoc#clg-fn-004[CLG-FN-004]). +* `benchmark` packages JMH scenarios that observe the performance envelopes (link:project-requirements.adoc#clg-fn-005[CLG-FN-005]). + +== Configuration Flow + +Configuration enters through properties or native framework settings: + +. Properties loaded by `ChronicleLogManager` via system property +`chronicle.logger.properties` or default class-path locations. +. Binding-specific configuration resolves logger names to queue paths, levels, and wire types. +. `ChronicleLogManager` caches writers per path; reload recreates queues safely (link:project-requirements.adoc#clg-nf-o-003[CLG-NF-O-003]). + +The design enforces explicit queue paths and fail-fast behaviour when storage is not writable (link:project-requirements.adoc#clg-nf-o-002[CLG-NF-O-002], +link:decision-log.adoc#clg-ops-002[CLG-OPS-002]). + +== Chronicle Queue Write Path + +1. Framework binding receives a log call and checks the requested level against its cached minimum. +2. When enabled, the binding forwards the event to `ChronicleLogWriter`, supplying timestamp, thread name, logger name, message pattern, optional throwable, and argument array (link:project-requirements.adoc#clg-fn-001[CLG-FN-001]). +3. `ChronicleLogWriter` serialises the data into a Chronicle Queue using the configured wire type, avoiding heap allocation on the hot path (link:project-requirements.adoc#clg-nf-p-001[CLG-NF-P-001]). + +Tailers and readers consume the queue either within the same JVM or from external processes. `ChronicleLogReader` provides a stable record schema and zero-garbage consumption (link:project-requirements.adoc#clg-nf-p-003[CLG-NF-P-003]). + +== Tooling and Observability + +`ChroniCat` and `ChroniTail` wrap `ChronicleLogReader` to offer one-shot and streaming inspection of log queues. +They surface warnings on unreadable paths and reuse the same decoding logic used by automated processors, including optional throwable decoding and argument preservation, ensuring identical semantics across CLI and programmatic use (link:project-requirements.adoc#clg-fn-004[CLG-FN-004]). + +Metrics for throughput, latency, and allocation are captured through the benchmark suite (link:project-requirements.adoc#clg-nf-p-001[CLG-NF-P-001]). +Future operational metrics follow the Ops obligation in +link:project-requirements.adoc#clg-doc-001[CLG-DOC-001]. + +== Operational Notes + +* Provision directories for each Chronicle Queue path during deployment and ensure permissions allow writer processes, satisfying +link:project-requirements.adoc#clg-nf-o-002[CLG-NF-O-002]. +* Coordinate configuration reloads with deployment tooling so that in-flight writers are recreated without leaks (link:project-requirements.adoc#clg-nf-o-003[CLG-NF-O-003]). +* Use the decision log to track future changes that alter queue layout, message schema, or failover strategy (link:decision-log.adoc#clg-doc-001[CLG-DOC-001]). diff --git a/src/main/docs/code-review-playbook.adoc b/src/main/docs/code-review-playbook.adoc new file mode 100644 index 000000000..82a54c679 --- /dev/null +++ b/src/main/docs/code-review-playbook.adoc @@ -0,0 +1,28 @@ += Chronicle Logger Code Review Playbook +:lang: en-GB + +== Purpose + +This playbook lists the static-analysis and coverage checks executed by the +`code-review` Maven profile so reviewers can reproduce the pipeline locally on the project’s supported JDKs. + +== Supported JDKs + +Chronicle Logger honours link:project-requirements.adoc#clg-nf-o-001[CLG-NF-O-001] +and the `code-review` profile now runs on Java 8, 11, and 17 without enforcing a Java 21 runtime. +Use the same JDK matrix that regular `mvn -q verify` builds exercise when validating a change. + +== How to Run + +. Use the module or aggregator root as the working directory. +. Execute `mvn -Pcode-review verify`. +. Address any failures reported by the plugins listed below before sharing the change for review. + +== Checks Included + +* `maven-checkstyle-plugin` with the Chronicle Quality Ruleset. +* `spotbugs-maven-plugin` with the FindSecBugs extensions. +* `maven-pmd-plugin` honouring the module-specific exclusion list. +* `jacoco-maven-plugin` to publish coverage reports and enforce agreed limits. + +The profile no longer enforces a minimum Java version, so reviewers can execute these checks in the same environments that production builds must support. diff --git a/src/main/docs/decision-log.adoc b/src/main/docs/decision-log.adoc new file mode 100644 index 000000000..f91ae71fe --- /dev/null +++ b/src/main/docs/decision-log.adoc @@ -0,0 +1,53 @@ +=== CLG-DOC-001 Adopt Nine-Box Requirements Catalogue + +Date:: 2025-10-17 +Context:: +* Peer Chronicle projects now maintain structured requirement catalogues with Nine-Box identifiers. +* Chronicle Logger previously relied on an unstructured bullet list, making cross-repo traceability difficult. +Decision Statement:: +* Chronicle Logger adopts the Nine-Box schema (FN, NF-P, NF-S, NF-O, DOC, OPS, TEST, RISK) with scope prefix `CLG` for all requirements and decisions. +Alternatives Considered:: +* Free-form numbering – low effort, but tooling and audits cannot map entries consistently. +* Per-module numbering – splits visibility across bindings and complicates shared obligations. +Rationale for Decision:: +* Aligns Chronicle Logger with company-wide guidance and enables cross-linking with test and benchmark artefacts. +* Simplifies audit checks for requirement coverage. +Impact & Consequences:: +* Requirements catalogue rewritten with tagged tables; future edits must preserve identifier uniqueness. +Notes/Links:: +* link:project-requirements.adoc[Requirements Catalogue] + +=== CLG-OPS-002 Enforce Explicit Queue Path Configuration + +Date:: 2025-10-17 +Context:: +* Legacy bindings silently created queue directories under the JVM working folder when configuration was missing. +* Operators requested fail-fast behaviour to avoid log loss and unpredictable disk usage. +Decision Statement:: +* All bindings must require an explicit Chronicle Queue path from configuration and fail fast with a warning if the directory is absent or unwritable. +Alternatives Considered:: +* Auto-create directories – masks configuration errors and may violate deployment policies. +* Fallback to in-memory logging – deviates from Chronicle Logger’s persistence contract. +Rationale for Decision:: +* Prevents silent data loss, aligns with operational expectations in regulated environments. +Impact & Consequences:: +* Tests updated to expect exceptions on missing paths. +* Deployment run-books must provision directories before service start. +Notes/Links:: +* link:project-requirements.adoc#clg-nf-o-002[CLG-NF-O-002] + +=== CLG-NF-P-003 Chronicle Log Reader Allocation Budget + +Date:: 2025-10-17 +Context:: +* Field users reported GC spikes when tailing high-volume logs using older reader implementations. +Decision Statement:: +* `ChronicleLogReader` must remain allocation-neutral during steady-state reads and expose benchmarks that enforce the constraint. +Alternatives Considered:: +* Accept sporadic allocations – risks latency regressions and contradicts Chronicle low-GC ethos. +Rationale for Decision:: +* Sustains deterministic behaviour for monitoring tools and aligns with performance requirements. +Impact & Consequences:: +* Reader implementation audited; benchmarks now gate memory usage. +Notes/Links:: +* link:project-requirements.adoc#clg-nf-p-003[CLG-NF-P-003] diff --git a/docs/images/Logger_line.png b/src/main/docs/images/Logger_line.png similarity index 100% rename from docs/images/Logger_line.png rename to src/main/docs/images/Logger_line.png diff --git a/src/main/docs/project-requirements.adoc b/src/main/docs/project-requirements.adoc new file mode 100644 index 000000000..5ef6d7e02 --- /dev/null +++ b/src/main/docs/project-requirements.adoc @@ -0,0 +1,118 @@ += Chronicle Logger Requirements Catalogue +:toc: +:lang: en-GB + +// ------------------------------------------------------------------- +// Legend +// ------------------------------------------------------------------- +// Requirement identifiers follow the Nine-Box taxonomy: +// --NNN, where Scope = CLG (Chronicle Logger) +// Tag = FN | NF-P | NF-S | NF-O | DOC | OPS | TEST | RISK +// NNN = zero-padded sequence per tag. +// Text must remain ASCII per AGENTS.md guidance. +// ------------------------------------------------------------------- + +== Scope + +Chronicle Logger provides Chronicle Queue backed appenders, bridges, and tools for popular Java logging frameworks (SLF4J 1.x and 2.x, Logback, Log4J 1/2, Commons Logging, JUL). +The requirements below define the contracts, quality targets, and operational obligations that all modules must meet. + +== Functional Requirements + +[cols="1,5,4",options="header"] +|=== +|ID |Requirement |Verification +|CLG-FN-001 |`ChronicleLogWriter` shall persist log events with timestamp, level, +thread name, logger name, message pattern, throwable, and arguments without +allocating on the steady-state hot path. |Unit tests assert field round trip via +`ChronicleLogReader`; allocation profiler runs in CI on representative workloads. +|CLG-FN-002 |`ChronicleLogManager` shall load configuration from the system +property `chronicle.logger.properties` or the documented class-path defaults, +supporting per-logger overrides for path, level, wire type, and append mode. |Integration +tests supply temp properties files and validate resolved settings per logger. +|CLG-FN-003 |Each binding (SLF4J 1.x, SLF4J 2.x, JUL, JCL, Logback, Log4J 1, Log4J 2) +shall honour its native configuration entry points while delegating writes +through `ChronicleLogWriter`. |Framework-specific tests assert that configuration +files enable Chronicle appenders and produce Chronicle Queue output. +|CLG-FN-004 |Logger tools (`ChroniCat`, `ChroniTail`, `ChronicleLogReader`) shall +read binary Chronicle logs, display tail output, and stream entries to custom +processors without data loss. |CLI smoke tests replay fixture queues; unit tests +verify processor callbacks receive ordered entries. +|CLG-FN-005 |Benchmark module shall provide repeatable JMH suites covering append +throughput and tail latency for key bindings with configurable message sizes. |CI +benchmark job executes weekly and publishes trend reports; spot checks confirm no +regressions before releases. +|=== + +== Non-Functional Requirements – Performance + +[cols="1,5,4",options="header"] +|=== +|ID |Requirement |Benchmark Target +|CLG-NF-P-001 |Steady-state append of INFO level messages through SLF4J 1.x binding +shall achieve ≥ 1.2 million events per second on a 3.2 GHz x86-64 host with +Chronicle Queue BINARY_LIGHT. |JMH `Throughput` benchmark in `benchmark` module; +flagged if throughput drops by > 10 percent relative to baseline. +|CLG-NF-P-002 |Tail latency (99.9 percentile) for append operations with async +queue roll shall remain ≤ 750 microseconds under mixed INFO/WARN traffic. |Benchmark +module scenario combining timed roll cycle and mixed levels. +|CLG-NF-P-003 |`ChronicleLogReader` shall consume existing queues at ≥ 500k events +per second while remaining allocation-neutral. |Micro benchmark comparing reader +performance to baseline sample queues. +|=== + +== Non-Functional Requirements – Security and Operability + +[cols="1,5,4",options="header"] +|=== +|ID |Requirement |Verification +|CLG-NF-S-001 |Bindings must never execute formatted message strings with user +input on configuration load; only explicit properties may influence Chronicle +Queue paths. |Static audit of configuration paths; integration tests confirm user +message content is not evaluated. +|CLG-NF-O-001 |All modules must support JDK 8 through the current LTS release, +with automated builds running `mvn -q verify` on each supported JDK. |CI matrix +build status; release checklist captures JDK versions. +|CLG-NF-O-002 |Components shall emit operational warnings via standard logging +channels when Chronicle Queue paths are missing or unwritable, and fail fast +rather than silently downgrading. |Negative-path tests create read-only +directories and assert warning plus exception semantics. +|CLG-NF-O-003 |Configuration reload shall be thread-safe: calling +`ChronicleLogManager.reload()` must close and recreate writers without leaking +resources. |Unit tests validate path switches and writer closure; stress tests +exercise concurrent reloads with leak detectors enabled. +|=== + +== Test Obligations + +[cols="1,5",options="header"] +|=== +|ID |Requirement +|CLG-TEST-001 |Maintain ≥ 80 percent line coverage for `logger-core` and each +binding package, excluding generated or vendor code. +Jacoco gate enforced in CI. +|CLG-TEST-002 |Provide regression fixtures per binding that append structured +events (with throwable and argument arrays) and replay them via `ChronicleLogReader` +to assert schema compatibility. +|CLG-TEST-003 |Nightly build runs `mvn -q verify` and the benchmark smoke profile, +failing the pipeline if throughput or latency deviates beyond documented thresholds. +|=== + +== Documentation and Operational Obligations + +[cols="1,5",options="header"] +|=== +|ID |Requirement +|CLG-DOC-001 |Keep `architecture-overview.adoc` aligned with current module +structure, configuration flow, and decision references. +|CLG-DOC-002 |Update this requirements catalogue and cross-link new entries when +decisions in `decision-log.adoc` change behaviour or support matrices. +|CLG-OPS-001 |Publish a run-book section describing log directory hygiene, +retention, and recovery steps; ensure tools document command-line flags. +|=== + +== References + +* link:architecture-overview.adoc[Architecture Overview] +* link:decision-log.adoc[Decision Log] +* link:code-review-playbook.adoc[Code Review Playbook]