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-3905 scope message about searching for database config #6790

Merged
merged 3 commits into from
May 1, 2024

Conversation

torcolvin
Copy link
Collaborator

@torcolvin torcolvin commented Apr 24, 2024

On startup, this will log as follows, which is confusing at best for what it is doing.

2024-04-19T17:57:22.896+02:00 [INF] Config: GetDatabaseConfigs starting (attempt 1/5)
2024-04-19T17:57:32.894+02:00 [INF] Config: GetDatabaseConfigs starting (attempt 1/5)
2024-04-19T17:57:32.896+02:00 [INF] Config: GetDatabaseConfigs starting (attempt 1/5)
2024-04-19T17:57:32.897+02:00 [INF] Config: GetDatabaseConfigs starting (attempt 1/5)

I'm not sure if INFO logging is already too aggressive here, this was added in #6720 and will log every 10s.

Change the code to log the bucket name everywhere in this function by context and drop the logging for this message to trace if this is the first attempt, and info on attempt > 1. I think warn is too much for logging attempts 2/5, because on a simultaneous update to the config the bootstrap config could fail, and we will log warning after attempt 5 fails.

I think the best thing a reviewer could do is to run Sync Gateway with and without some buckets and see if the logging verbosity makes sense.

…mpt 1/5)

This will log once per bucket without the bucket name. This adds the
bucket name in the context.
@torcolvin torcolvin changed the title scope message about searching for database config CBG-3905 scope message about searching for database config May 1, 2024
@torcolvin torcolvin requested a review from bbrks May 1, 2024 00:25
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 good with these changes but would like to see the trace log moved to debug.

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>
@bbrks bbrks enabled auto-merge (squash) May 1, 2024 14:23
@bbrks bbrks merged commit 04f7ba2 into main May 1, 2024
16 of 17 checks passed
@bbrks bbrks deleted the improve-logging branch May 1, 2024 14:32
torcolvin added a commit that referenced this pull request May 1, 2024
* clarify message about [INF] Config: GetDatabaseConfigs starting (attempt 1/5)

This will log once per bucket without the bucket name. This adds the
bucket name in the context.

* Log first attempt on trace and subsequent attempts on info

* Update rest/config_manager.go

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>

---------

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>
bbrks added a commit that referenced this pull request May 1, 2024
…config (#6790) (#6800)

* clarify message about [INF] Config: GetDatabaseConfigs starting (attempt 1/5)

This will log once per bucket without the bucket name. This adds the
bucket name in the context.

* Log first attempt on trace and subsequent attempts on info

* Update rest/config_manager.go



---------

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>
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