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

Ability to disable log with level ERROR if key not exists #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

roman-struchev
Copy link

@roman-struchev roman-struchev commented Feb 13, 2024

Context
We use feature hub to manage feature toggles

Also, we have 2 additional point

  • not all feature toggles exist in feature hub, the responsible person often creates it when feature toggle is needed
  • we have several feature toggle sources with priority, for example, feature hub, application properties, environment, etc

Issue

public FeatureState getFeatureState(String key) {
return features.computeIfAbsent(
key,
key1 -> {
if (hasReceivedInitialState) {
log.error(
"FeatureHub error: application requesting use of invalid key after initialization: `{}`",
key1);
}
return new FeatureStateBase(null, this, key);
});
}

When we try to get feature value in java (io.featurehub.client.ClientFeatureRepository#getFeatureState) first time, then we have such log for our applications FeatureHub error: application requesting use of invalid key after initialization: XXX.

This happens only for the first request for the feature toggle (not existing in feature hub), after instance of application was restarted (because of java.util.Map#computeIfAbsent used)

So in monitoring of our applications we have error cases by logs

Fix
I try to provide an ability to skip this log by configuration, because looks like that is usual case when feature toggle not exists in feature hub

@rvowles
Copy link
Contributor

rvowles commented Feb 14, 2024

Hey, probably simpler - it should really allow the feature to exist all the time even if it never comes from the server. I have another change to make so I will simplify this logic and write a test or two to ensure its working properly. Thanks for bringing this to our attention!

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