Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter formatting is broken with TruffleLoggers #5801

Closed
hubertp opened this issue Mar 2, 2023 · 2 comments · Fixed by #5870
Closed

Parameter formatting is broken with TruffleLoggers #5801

hubertp opened this issue Mar 2, 2023 · 2 comments · Fixed by #5870
Assignees
Labels
-compiler p-medium Should be completed in the next few sprints

Comments

@hubertp
Copy link
Collaborator

hubertp commented Mar 2, 2023

Logs coming out of TruffleLogger don't use the provided arguments leading to incomplete messages:

[debug] [...] [epb.org.enso.interpreter.epb.EpbContext] Initializing languages {0}
[trace] [...] [epb.org.enso.interpreter.epb.EpbContext] Starting initialization thread
[trace] [...] [epb.org.enso.interpreter.epb.EpbContext] Entering initialization thread
[trace] [...] [epb.org.enso.interpreter.epb.EpbContext] Initializing language {0}
[trace] [...] [epb.org.enso.interpreter.epb.EpbContext] Initializing on background
[debug] [...] [enso] Executing commands in a separate command pool
[debug] [...] [epb.org.enso.interpreter.epb.EpbContext] Done initializing language {0} with {1} in {2} ms

Notice the {0} for parameter position which should have been replaced with the correct argument.

As per logs in https://github.com/enso-org/enso/blob/develop/engine/runtime-language-epb/src/main/java/org/enso/interpreter/epb/EpbContext.java#L66 or https://github.com/enso-org/enso/blob/develop/engine/runtime-language-epb/src/main/java/org/enso/interpreter/epb/EpbContext.java#L80.

Those logs were introduced in #5680 but I can see plenty of places where we could use parameter formatting except we can't so logs are full of String concatenation (also affecting performance).

@hubertp hubertp added p-medium Should be completed in the next few sprints -compiler labels Mar 2, 2023
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Mar 2, 2023
@JaroslavTulach JaroslavTulach moved this from ❓New to 📤 Backlog in Issues Board Mar 3, 2023
@JaroslavTulach
Copy link
Member

JaroslavTulach commented Mar 3, 2023

There is a Builder.logHandler(...) method. If we use it, we get LogRecord and we can format it using SimpleFomatter or our own formatter.

The implementation must be somewhere in PolyglotLoggers.

@jdunkerley jdunkerley added this to the Design Partners milestone Mar 7, 2023
@jdunkerley jdunkerley moved this from 📤 Backlog to ❓New in Issues Board Mar 7, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Mar 7, 2023
@JaroslavTulach
Copy link
Member

JaroslavTulach commented Mar 9, 2023

Running ./project-manager -vv --no-log-masking 2>&1 |grep "\{0" shows messages like:

[debug] [2023-03-09T09:22:58.525Z] [epb.org.enso.interpreter.epb.EpbContext] Initializing languages {0}
[trace] [2023-03-09T09:22:58.53Z] [epb.org.enso.interpreter.epb.EpbContext] Initializing language {0}
[debug] [2023-03-09T09:22:59.139Z] [epb.org.enso.interpreter.epb.EpbContext] Done initializing language {0} with {1} in {2} ms
[debug] [2023-03-09T09:51:17.29Z] [epb.org.enso.interpreter.epb.EpbContext] Initializing languages {0}
[trace] [2023-03-09T09:51:17.297Z] [epb.org.enso.interpreter.epb.EpbContext] Initializing language {0}
[debug] [2023-03-09T09:51:17.913Z] [epb.org.enso.interpreter.epb.EpbContext] Done initializing language {0} with {1} in {2} ms

they deserve to be properly formatted.

@JaroslavTulach JaroslavTulach moved this from 📤 Backlog to 👁️ Code review in Issues Board Mar 9, 2023
@mergify mergify bot closed this as completed in #5870 Mar 11, 2023
mergify bot pushed a commit that referenced this issue Mar 11, 2023
…rs (#5870)

Fixes #5801 to properly format Truffle log records before sending them for further processing.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-compiler p-medium Should be completed in the next few sprints
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants