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

Only warn once when a flag is missing #148

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Conversation

erbridge
Copy link
Member

It's totally legitimate to reference a feature flag that doesn't exist, yet, but before this change, doing so would spam our logs with warnings.

@erbridge erbridge requested a review from a team as a code owner October 18, 2024 11:18
@philandstuff
Copy link
Contributor

Hmm, I think it might be a useful signal that a flag has been typoed? It does mean we need to remember to create the flag before deploying though, otherwise we get said spam.

@erbridge
Copy link
Member Author

erbridge commented Oct 18, 2024

Has that ever happened for us? Because I know we do get it right, but don't create the flag in LaunchDarkly, resulting in said spam.

@nickstenning
Copy link
Member

IMO most ergonomic thing to do here would be to keep a package private map[string]bool which says whether or not we've emitted the warning, and emit it once before setting the key value to true in that map.

@erbridge
Copy link
Member Author

Nice. I like that idea.

@erbridge erbridge force-pushed the no-missing-flag-warnings branch from 55f79fc to 28c0fd2 Compare October 18, 2024 12:22
@erbridge erbridge changed the title Don't warn when a flag is missing Only warn once when a flag is missing Oct 18, 2024
@philandstuff
Copy link
Contributor

as long as said map is thread-safe. A sync.Map, say.

@erbridge erbridge force-pushed the no-missing-flag-warnings branch from 28c0fd2 to 22358c8 Compare October 18, 2024 12:25
It's totally legitimate to reference a feature flag that doesn't exist,
yet, but before this change, doing so would spam our logs with warnings.
@erbridge erbridge force-pushed the no-missing-flag-warnings branch from 22358c8 to a52ef03 Compare October 18, 2024 12:25
@erbridge erbridge merged commit 3dec3aa into main Oct 23, 2024
2 checks passed
@erbridge erbridge deleted the no-missing-flag-warnings branch October 23, 2024 10:53
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