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

CBG-3933 allow console logs to be rotated #7058

Merged
merged 6 commits into from
Aug 21, 2024
Merged

CBG-3933 allow console logs to be rotated #7058

merged 6 commits into from
Aug 21, 2024

Conversation

torcolvin
Copy link
Collaborator

@torcolvin torcolvin commented Aug 12, 2024

  • create initLumberjackLogger to initialize a lumberjack logging for files
  • fix a latent bug where we leaked logCollationWorker since we would never exit the go routine. In order to close this gap, I added FileLogger.closed channel, since the following events have to happen in order:
    1. exit log rotation worker goroutine
    2. wait for log rotation worker to be done
    3. close the file handle
    4. exit logCollationWorker goroutine
  • fixup docs for logging config. I noticed that https://docs.couchbase.com/sync-gateway/current/configuration-schema-bootstrap.html is missing a lot of the default values, so I added them in. Use allOf to allow for composition of data structures to override the max age parameters.

Notes:

  • I don't know that max_age=6 is right for the stats logger. It probably doesn't matter since we never hit it.
  • I kept the console logger max_age=0 because I wanted to conform to the API. That said, as above our API isn't documented. I don't know what it worse, losing console logs or filing up disks.

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Integration Tests

Copy link

github-actions bot commented Aug 12, 2024

Redocly previews

Copy link
Member

@bbrks bbrks left a comment

Choose a reason for hiding this comment

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

Haven't reviewed the API doc changes yet, but have done a full pass on the code changes.

- change log deletion to use regex to match all patterns of possible log
  paths
- disable compression for console logs
- restore stderr log collation
@torcolvin torcolvin requested a review from bbrks August 15, 2024 14:37
@torcolvin torcolvin assigned bbrks and unassigned torcolvin Aug 15, 2024
Copy link
Member

@bbrks bbrks left a comment

Choose a reason for hiding this comment

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

I'm OK with this approach but just have a few comments around getDeletionDirAndRegexp

@bbrks bbrks assigned torcolvin and unassigned bbrks Aug 19, 2024
@bbrks bbrks merged commit b24e02a into main Aug 21, 2024
41 checks passed
@bbrks bbrks deleted the CBG-3933 branch August 21, 2024 11:59
torcolvin added a commit that referenced this pull request Aug 23, 2024
* CBG-3933 allow console logs to be rotated

* update log deletion tests

- change log deletion to use regex to match all patterns of possible log
  paths
- disable compression for console logs
- restore stderr log collation

* windows requires no slashes in path names

* Apply suggestions from code review

Co-authored-by: Ben Brooks <[email protected]>

* Update base/logger_console_test.go

Co-authored-by: Ben Brooks <[email protected]>

* add missing import

---------

Co-authored-by: Ben Brooks <[email protected]>
torcolvin added a commit that referenced this pull request Aug 23, 2024
* CBG-3933 allow console logs to be rotated

* update log deletion tests

- change log deletion to use regex to match all patterns of possible log
  paths
- disable compression for console logs
- restore stderr log collation

* windows requires no slashes in path names

* Apply suggestions from code review

Co-authored-by: Ben Brooks <[email protected]>

* Update base/logger_console_test.go

Co-authored-by: Ben Brooks <[email protected]>

* add missing import

---------

Co-authored-by: Ben Brooks <[email protected]>
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