Skip to content

Comments

Use slf4j in instrument non-guest code#12627

Merged
mergify[bot] merged 6 commits intodevelopfrom
wip/jtulach/slf4j12500
Mar 25, 2025
Merged

Use slf4j in instrument non-guest code#12627
mergify[bot] merged 6 commits intodevelopfrom
wip/jtulach/slf4j12500

Conversation

@JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Mar 25, 2025

Pull Request Description

  • based on experiments made in Reschedule guest code execution into dedicated thread pool #12613 that are by themselves motivated by Upgrading to Truffle libraries 24.2.0 #12500
  • TruffleLogger shall only be used in the interpreter running guest code
  • most of the code in runtime-instrument-xyz projects is not running a guest code
  • most of the code in these instruments schedules runs of guest code
  • thus using TruffleLogger isn't appropriate and it yields exception when it cannot find EnsoContext around
  • let's make the code ready for context not being around
  • let's use SLF4J instead of TruffleLogger

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
  • Unit tests updated to continue to pass

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 25, 2025
@JaroslavTulach JaroslavTulach self-assigned this Mar 25, 2025
Copy link
Collaborator

@hubertp hubertp left a comment

Choose a reason for hiding this comment

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

Welcome to the dark side

JaroslavTulach and others added 2 commits March 25, 2025 17:24
Co-authored-by: Hubert Plociniczak <hubert.plociniczak@gmail.com>
@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Mar 25, 2025
@mergify mergify bot merged commit f954fca into develop Mar 25, 2025
72 checks passed
@mergify mergify bot deleted the wip/jtulach/slf4j12500 branch March 25, 2025 19:14
@enso-bot enso-bot bot mentioned this pull request Mar 26, 2025
6 tasks
@jdunkerley jdunkerley added this to the 2025-Q2 Release milestone Mar 26, 2025
@hubertp
Copy link
Collaborator

hubertp commented Mar 26, 2025

All our logs are now broken:

[DEBUG] [2025-03-26T17:07:33.631] [org.enso.languageserver.requesthandler.text.ApplyEditHandler] stopped
[TRACE] [2025-03-26T17:07:33.631] [org.enso.interpreter.instrument.execution.ReentrantLocking] Waited [[Ljava.lang.Object;@48e19ca2] {}ms for the {}
[TRACE] [2025-03-26T17:07:33.631] [org.enso.interpreter.instrument.execution.ReentrantLocking] Waited [[Ljava.lang.Object;@ea44313] {}ms for the {}
[TRACE] [2025-03-26T17:07:33.631] [org.enso.interpreter.instrument.command.Command] Adding pending file [[Ljava.lang.Object;@7d8d303e] edits [{}] and IdMap of length {}
[TRACE] [2025-03-26T17:07:33.631] [org.enso.interpreter.instrument.execution.ReentrantLocking] Kept pending edits lock [[Ljava.lang.Object;@405a9f84] for {}ms
[DEBUG] [2025-03-26T17:07:33.631] [org.enso.interpreter.instrument.execution.ReentrantLocking] Kept file lock [[Ljava.lang.Object;@d2247] for {1}ms

@Akirathan
Copy link
Contributor

All our logs are now broken:

Right. Because slf4j logger has different API than java.util.logging logger. slf4j logger accepts vararg for arguments, wherease java.util.logging accepts Object[]. This means that every logging method call that passes new Object[]{...} should be rewritten to the vararg variant.

} else {
logger.log(
Level.FINE,
"Requested environment '{}' is the same as the current one. Request has no"
Copy link
Member Author

Choose a reason for hiding this comment

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

This must have been broken as java.util.logging.Logger doesn't recognize {} as a variable placeholder.

@JaroslavTulach
Copy link
Member Author

All our logs are now broken:
slf4j logger accepts vararg for arguments, wherease java.util.logging accepts Object[]

I see. I'll fix it:

  • having two logging systems/formats along each other may be confusing
  • mistakes had already been present
  • now we have the vararg mistakes

How to detect the errors?

  • only errors f954fca will be addressed
  • e.g. only added lines in the commit
  • but we need to take a look at surrounding lines as well as new Object[] is often placed on a separate line
git show f954fca22fca9158b3e8126c1ccc6673e10c9bd9 | grep ^+ -C2 | grep new.*Object -C20

yields few candidates. But then we also log from scala and following command gives plenty:

git show f954fca22fca9158b3e8126c1ccc6673e10c9bd9 | grep ^+ -C2 | grep Array -C20

mergify bot pushed a commit that referenced this pull request Mar 27, 2025
Fine tunes #12627 by using variable arguments when passing multiple arguments to log messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants