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-3585 log bucket and groupID during config search #6564

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Conversation

torcolvin
Copy link
Collaborator

This makes it a bit more clear which bucket a config error might come from. I added groupID to these logs as well, since they are logged by some lines in these functions but not all.

@adamcfraser
Copy link
Collaborator

Generally speaking logging groupID is redundant information, since a given node (and it's logs) is only associated with a single groupID per startup, and the groupID is logged at startup. Given the particular relevance of groupID to the registry I'm fine leaving it here for ease of use, but as a rule I still think we should omit.

@torcolvin
Copy link
Collaborator Author

Generally speaking logging groupID is redundant information, since a given node (and it's logs) is only associated with a single groupID per startup, and the groupID is logged at startup. Given the particular relevance of groupID to the registry I'm fine leaving it here for ease of use, but as a rule I still think we should omit.

I alway felt groupID was redundant but it was present in other log lines in the same function. The important thing that was left out was the bucket. OTOH, if you are trying to debug issues here having groupID again is helpful.

@torcolvin torcolvin merged commit 2fe011c into master Nov 7, 2023
26 checks passed
@torcolvin torcolvin deleted the CBG-3585 branch November 7, 2023 14:35
gregns1 added a commit that referenced this pull request Feb 6, 2024
#6667)

* CBG-3585 log bucket and groupID during config search (#6564)

* Replace `%w` with `%s` inside log functions. Only `fmt.Errorf` supports `%w` (#6494)

---------

Co-authored-by: Tor Colvin <[email protected]>
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.

3 participants