Skip to content

Conversation

@ehigham
Copy link
Member

@ehigham ehigham commented Nov 6, 2025

Readers are encouraged to read the log4j documentation on performance considerations. This change is motivated by a number of the issues discussed therein.

We do a lot of string interpolation in logs which isn't great as these logging APIs use call by ref instead of call by name.

log4j-api-scala uses macros to transform string-interpolated log statements into efficient java versions, making sure we're not building strings needlessly.

Their doc suggests inheriting from Logging​ and using the protected logger​ field. One benefit of doing things this way is that logs contain the name of the enclosing class without analysing the call stack. I find it much easier to find where a log statement was generated within the source code as a result.

The trouble with this Logging​ trait is that the loggeris not reconfigurable for reasons I'm yet to fully understand. I think it has something to do with specifying the class loader to log4j's LoggingFactory​..? Anyway, I repurposed our Logging​ trait to provide the protected logger​ member that is reconfigurable.

This change cannot impact the Hail Batch instance deployed by Broad Institute in GCP

Copy link
Member Author

ehigham commented Nov 6, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ehigham ehigham mentioned this pull request Nov 6, 2025
@ehigham ehigham force-pushed the ehigham/log4j-socker-appender branch from fb70546 to 48f939c Compare November 6, 2025 19:01
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch 2 times, most recently from 6412c86 to 6b96148 Compare November 6, 2025 19:33
@ehigham ehigham force-pushed the ehigham/log4j-socker-appender branch 2 times, most recently from 6d112eb to 928f7e9 Compare November 6, 2025 19:40
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch 2 times, most recently from 51de23d to ab95499 Compare November 6, 2025 20:48
@ehigham ehigham force-pushed the ehigham/log4j-socker-appender branch from 928f7e9 to f5248ee Compare November 6, 2025 20:48
@ehigham ehigham changed the base branch from ehigham/log4j-socker-appender to graphite-base/15181 November 6, 2025 21:21
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch from ab95499 to ae40002 Compare November 6, 2025 21:21
@ehigham ehigham force-pushed the graphite-base/15181 branch from f5248ee to aa4f644 Compare November 6, 2025 21:21
@ehigham ehigham changed the base branch from graphite-base/15181 to ehigham/experimental-init November 6, 2025 21:22
@ehigham ehigham force-pushed the ehigham/experimental-init branch from aa4f644 to 7b5f5aa Compare November 7, 2025 15:29
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch 3 times, most recently from 70430b3 to 1e2c06e Compare November 7, 2025 17:36
@ehigham ehigham force-pushed the ehigham/experimental-init branch 2 times, most recently from 7df0197 to a5a2ce2 Compare November 7, 2025 17:53
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch from 1e2c06e to 92e19fc Compare November 7, 2025 17:53
@ehigham ehigham force-pushed the ehigham/experimental-init branch from a5a2ce2 to 1688481 Compare November 7, 2025 20:07
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch from 92e19fc to 53a9a24 Compare November 7, 2025 20:07
@ehigham ehigham marked this pull request as ready for review November 7, 2025 22:29
@ehigham ehigham force-pushed the ehigham/experimental-init branch from 1688481 to 892768c Compare November 13, 2025 16:20
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch 2 times, most recently from 00e81ae to 68c71a3 Compare November 13, 2025 16:29
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch 3 times, most recently from 68c71a3 to b05bdc9 Compare November 13, 2025 16:40
@ehigham ehigham force-pushed the ehigham/experimental-init branch from 892768c to efca829 Compare November 13, 2025 16:52
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch from b05bdc9 to bf6e025 Compare November 13, 2025 16:52
@ehigham ehigham force-pushed the ehigham/experimental-init branch from efca829 to 6fdd548 Compare November 13, 2025 20:57
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch from bf6e025 to a6c8816 Compare November 13, 2025 20:57
@ehigham ehigham force-pushed the ehigham/log4j-api-scala branch from a6c8816 to f70eee1 Compare November 14, 2025 03:14
@ehigham ehigham force-pushed the ehigham/experimental-init branch from 6fdd548 to c3f9ff5 Compare November 14, 2025 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants