Update log configurations [ECR-3139] #854

dmitry-timofeev · 2019-04-22T17:38:55Z

Overview

Update log configurations:

Configure SLF4J->Log4j binding
Update log configurations:
- For Java tests — to print to stdout our debug messages and warnings from 3rd-party libraries;
- For native tests — to print only fatal errors. That reduces the number of log messages in build output.
Redirect any output of unit and integration Java tests to files.
Move the log4j configuration for native ITs to native ITs
so that it is (a) easily discoverable; (b) can be changed with no
recompilation of Java fakes module.
Also, pass the path to the Log4j configuration file to the
service runtime bootstrap test to remove the error message from logs.

See: https://jira.bf.local/browse/ECR-3139?focusedCommentId=43246&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-43246

Definition of Done

There are no TODOs left in the code
Change is covered by automated tests
The coding guidelines are followed
Public API has Javadoc
Method preconditions are checked and documented in the Javadoc of the method
Changelog is updated if needed (in case of notable or breaking changes)
The continuous integration build passes

coveralls · 2019-04-22T18:04:15Z

Coverage remained the same at 85.388% when pulling d05e1c5 on dmitry-timofeev:tap-slf4j-logs-ECR-3139 into aeba822 on exonum:master.

Use 'debug' for our code only; 'warn' — for anything else.

Send any warnings to stdout; our debug entries — to stdout and the list appender.

That reduces the number of log messages in build output.

Move the log4j configuration for native ITs to native ITs so that it is (a) easily discoverable; (b) can be changed with no recompilation of Java `fakes` module. Also, pass the path to the Log4j configuration file to the service runtime bootstrap test to remove the error message from logs.

dmitry-timofeev · 2019-04-23T13:01:06Z

exonum-java-binding/core/rust/integration_tests/src/vm.rs

+}
+
+/// The path to `integration_tests` root directory.
+fn project_root_dir() -> &'static Path {


Does it have to return a reference? If so when is the object destroyed?

I guess we need to return PathBuf, not &Path.

exonum-java-binding/CHANGELOG.md

bullet-tooth · 2019-04-23T13:10:09Z

exonum-java-binding/CHANGELOG.md

@@ -19,7 +19,9 @@ The release is based on Exonum 0.11

 ### Added
 - `toOptional()` method to `EntryIndexProxy`. (#790)
- `getPoolTransactionHashes()` method to `Blockchain`. (#850)
+- `getTransactionPool()` method to `Blockchain`. (#850)
+- SLF4J to Log4j binding to enable libraries coded to the SLF4J API to use Log4j 2, 


I believe that "- SLF4J support" is enough in the changelog. Also, I would mention in docs that EJB provides log4j 2 as a logging implementation

bullet-tooth · 2019-04-23T13:23:45Z

exonum-java-binding/pom.xml

@@ -354,6 +354,7 @@
          <artifactId>maven-surefire-plugin</artifactId>
          <version>2.22.1</version>
          <configuration>
+            <redirectTestOutputToFile>true</redirectTestOutputToFile>


What does it bring to us? We do not use stdout in tests so often.

We don't, the logger prints to stdout (see the docs)

Co-Authored-By: dmitry-timofeev <[email protected]>

vitvakatu · 2019-04-24T07:05:28Z

exonum-java-binding/core/rust/integration_tests/src/vm.rs

+/// Java logs are needed for debugging purposes.
+///
+/// It requires the log4j-core library to be present on the classpath, which is the case with fakes.
+fn get_log4j_path_option() -> String {


nit: why we even use these get_ prefixes? https://rust-lang-nursery.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter

vitvakatu · 2019-04-24T07:07:55Z

exonum-java-binding/core/rust/integration_tests/src/vm.rs

+}
+
+/// The path to `integration_tests` root directory.
+fn project_root_dir() -> &'static Path {


I guess we need to return PathBuf, not &Path.

dmitry-timofeev added the work-in-progress 👷‍♂️ Do not expect reviewers to provide any feedback on WIP PRs — please ask for it explicitly! label Apr 22, 2019

dmitry-timofeev and others added 8 commits April 23, 2019 15:46

Update the log configuration for native ITs

05daed4

Use 'debug' for our code only; 'warn' — for anything else.

Configure SLF4J->Log4j binding

79bab21

Update Log4j test configurations:

f090597

Send any warnings to stdout; our debug entries — to stdout and the list appender.

Redirect test output to file

e26ca46

Use FATAL as the minimum log level for native ITs

4034849

That reduces the number of log messages in build output.

Add changelog entry

fba08eb

fix description

c670c73

dmitry-timofeev force-pushed the tap-slf4j-logs-ECR-3139 branch from 6a51a75 to c670c73 Compare April 23, 2019 12:48

Fix changelog

eba995d

dmitry-timofeev removed the work-in-progress 👷‍♂️ Do not expect reviewers to provide any feedback on WIP PRs — please ask for it explicitly! label Apr 23, 2019

dmitry-timofeev changed the title ~~Tap slf4j logs ecr 3139~~ Update log configurations [ECR-3139] Apr 23, 2019

Fix rustfmt

8b9b965

dmitry-timofeev requested review from bullet-tooth, MakarovS, skletsun and vitvakatu and removed request for bullet-tooth and MakarovS April 23, 2019 12:59

dmitry-timofeev commented Apr 23, 2019

View reviewed changes

bullet-tooth reviewed Apr 23, 2019

View reviewed changes

Update exonum-java-binding/CHANGELOG.md

79b8fbd

Co-Authored-By: dmitry-timofeev <[email protected]>

vitvakatu reviewed Apr 24, 2019

View reviewed changes

dmitry-timofeev added 3 commits April 24, 2019 13:23

Use PathBuf instead of Path

de2db4b

Drop get_ prefix from vm functions

44c68a8

rm unused imports

0caa7c0

Merge branch 'master' into tap-slf4j-logs-ECR-3139

bdcba77

MakarovS approved these changes Apr 25, 2019

View reviewed changes

Merge branch 'master' into tap-slf4j-logs-ECR-3139

d05e1c5

dmitry-timofeev added the Next to merge 🛬 The next PR to merge (that is up-to-date with master) label Apr 25, 2019

dmitry-timofeev merged commit a7be6e8 into exonum:master Apr 25, 2019

dmitry-timofeev deleted the tap-slf4j-logs-ECR-3139 branch April 25, 2019 12:52

skletsun approved these changes Apr 25, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update log configurations [ECR-3139] #854

Update log configurations [ECR-3139] #854

dmitry-timofeev commented Apr 22, 2019 •

edited

Loading

coveralls commented Apr 22, 2019 •

edited

Loading

dmitry-timofeev Apr 23, 2019

vitvakatu Apr 24, 2019

bullet-tooth Apr 23, 2019

bullet-tooth Apr 23, 2019

dmitry-timofeev Apr 23, 2019

vitvakatu Apr 24, 2019

vitvakatu Apr 24, 2019

Update log configurations [ECR-3139] #854

Update log configurations [ECR-3139] #854

Conversation

dmitry-timofeev commented Apr 22, 2019 • edited Loading

Overview

Definition of Done

coveralls commented Apr 22, 2019 • edited Loading

dmitry-timofeev Apr 23, 2019

Choose a reason for hiding this comment

vitvakatu Apr 24, 2019

Choose a reason for hiding this comment

bullet-tooth Apr 23, 2019

Choose a reason for hiding this comment

bullet-tooth Apr 23, 2019

Choose a reason for hiding this comment

dmitry-timofeev Apr 23, 2019

Choose a reason for hiding this comment

vitvakatu Apr 24, 2019

Choose a reason for hiding this comment

vitvakatu Apr 24, 2019

Choose a reason for hiding this comment

dmitry-timofeev commented Apr 22, 2019 •

edited

Loading

coveralls commented Apr 22, 2019 •

edited

Loading