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

Add Source to Logging #1367

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Add Source to Logging #1367

merged 2 commits into from
Jan 5, 2023

Conversation

lkiesow
Copy link
Contributor

@lkiesow lkiesow commented Jan 5, 2023

The Audiobookshelf logs sometimes contain information about the source of the log statement, but sometimes they don't This really depends on developers adding these information to the log messages.

But even then, the information is usually just a hint about the module logging this, like [Db] or [Watcher]`, and finding the exact line can be hard.

This patch automatically adds the source of the log statement to the logs. This means if someone calls Logger.info(…) in line 22 of foo.js, the log statement will contain this file and line:

[2023-01-05 19:04:12[ (LogManager.js:85:18) DEBUG: Daily Log file found 2023-01-05.txt
[2023-01-05 19:04:12] (LogManager.js:59:12)  INFO: [LogManager] Init current daily log filename: 2023-01-05.txt

This should make it much easier to identify the code where the log statement originated from.

Long-term, this also means that we can probably remove the manually set identifiers contained in the log messages, like the [LogManager] in the example above.

lkiesow and others added 2 commits January 5, 2023 19:13
The Audiobookshelf logs sometimes contain information about the source
of the log statement, but sometimes they don't This really depends on
developers adding these information to the log messages.

But even then, the information is usually just a hint about the module
logging this, like `[Db]` or [Watcher]`, and finding the exact line can
be hard.

This patch automatically adds the source of the log statement to the
logs. This means if someone calls `Logger.info(…)` in line `22` of
`foo.js`, the log statement will contain this file and line:

```
[2023-01-05 19:04:12[ (LogManager.js:85:18) DEBUG: Daily Log file found 2023-01-05.txt
[2023-01-05 19:04:12] (LogManager.js:59:12)  INFO: [LogManager] Init current daily log filename: 2023-01-05.txt
```

This should make it much easier to identify the code where the log
statement originated from.

Long-term, this also means that we can probably remove the manually set
identifiers contained in the log messages, like the `[LogManager]` in
the example above.
@advplyr
Copy link
Owner

advplyr commented Jan 5, 2023

I like it. I didn't think to catch the stack trace like that. I do prefer some updates to it and fixed it so it works on Windows.

[2023-01-05 16:44:46] INFO: [DB] 2346 Library Items Loaded
[2023-01-05 16:44:46] DEBUG: [BackupManager] Backup found "2022-12-21T0730" (BackupManager.js:177)

Putting it at the end of the log line and not including it for info logs.
Also, I updated the regex so that it only includes the line number since the line position is irrelevant.

I set a variable to detect if this is windows global.isWin so we can use that to resolve the other issue you mentioned with posix.

@advplyr advplyr merged commit f76f9c7 into advplyr:master Jan 5, 2023
@lkiesow
Copy link
Contributor Author

lkiesow commented Jan 5, 2023

Sorry about Windows. I forgot about the file path being different there. And I don't have a Windows at hand to test…

[…] and not including it for info logs.

Do you have strong feelings about not including this for info and trace? I find something like this really helpful when investigating issues and not every issues comes with an actual error.

@advplyr
Copy link
Owner

advplyr commented Jan 5, 2023

We could include it for info in development mode.
trace isn't being used and console.trace will already dump a stack trace.

@lkiesow
Copy link
Contributor Author

lkiesow commented Jan 6, 2023

Let's not do that. I suspect that in dev mode it's less often that you work with logs and it's always somewhat confusing if an application behaves differently if different environments. I'd rather avoid that if there is no good reason. But I guess, I can always fall back to the log files where the info is included after all.

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