Log "unknown feature flag" as a warning instead of info

[nc-wittj]

Requirements

• I have added test coverage for new or changed functionality
• I have followed the repository's pull request submission guidelines
• I have validated my changes against all supported platform versions

Related issues
We've been running LaunchDarkly logs with the info log level in all environments, so we can capture instances of "unknown feature flag" logs. The annoyance of working around the async event logs in our Rails console has reached the point that we'd really like to swap to the warning log level. However, that would mean we would not see instances of "unknown feature flag" logs. To us, those logs really make more sense as a warning (if not an error), thus, I'd like to propose changing it to warn.

I also noticed a few logs that are missing the [LDClient] prefix, so I added that. (I'm sure there are more missing.)

Describe the solution you've provided

Log "unknown feature flag" as a warning instead of info.

Describe alternatives you've considered

An alternative would be to use get_flag_state_relaxed, and handle our own default fallback, but that does not seem ideal for this sort of thing.

Additional context

[nc-wittj]

@eli-darkly @bwoskow-ld Just wanted to make sure this doesn't get lost. I'm looking forward to hearing your thoughts! Thanks!

[eli-darkly]

Hi - sorry for the delay, we received a bunch of SDK-related issues and have been working through them slowly.

There are a few issues here. One is what log levels are most appropriate for what conditions. We try to apply that decision across all SDKs for consistency rather than on a per-SDK basis, so if we were going to change this it would not be only for the Ruby SDK.

We also have to be very careful about making such changes in general, because even though the logging behavior is not technically an API that has to be kept consistent by semantic versioning rules, changing it can have significant impact on existing customers depending on how they've set up their monitoring. So when you say "those logs really make more sense as a warning (if not an error)"— we would definitely not want to change the level to ERROR, because for many customers that would mean they are suddenly getting unwanted alerts they didn't previously get from their monitoring service. And since web applications normally evaluate the same feature flags very frequently, that could be a lot of alerts; for that reason, we try to avoid logging anything at ERROR level that will always happen the same way for any given flag key.

Whether WARN is appropriate is another question. We understand the argument that this would make the log more useful for spotting code mistakes (such as a misspelled flag key) or operational mistakes (such as forgetting to create the flag). On the other hand, some customers have a habit of deliberately deploying new code before the corresponding flag is created, relying on the application default value parameter to provide the desired behavior until the flag is in place. So it's unclear whether this should be considered an abnormal condition or not. The same issue applies here as above in terms of this being a condition that could happen very frequently— we usually try to avoid using WARN level for something that is dependent only on an invariant parameter that's passed by the application, at least in flag evaluations, because any such condition is likely to happen over and over and over again.

[nc-wittj]

No worries! Thanks for your response.

I see what you're getting at. Though, looking at the Go SDK, it seems like it logs at the WARN level? When a feature flag is not found, it appears to call evalErrorResult, which seems to log at the WARN level. Or am I missing something?

I do understand why you might want to avoid lots of WARN logs, but at this point, we are getting lots more INFO logs, which are super annoying. It does seem useful from a debugging standpoint to be able to look in the logs to see "feature flag not found" warnings, since that would save time if the flag has a typo. But then again, I'm not sure our team has mistyped a flag name yet. 🤞

If the consensus on your side is to keep this an INFO log in the Ruby SDK, then I'll discuss with my team about either not having these in the logs (move to WARN level), or to implement custom logging by calling get_flag_state_relaxed (but that doesn't seem super ideal, as we'd need to implement fallback logic).

[eli-darkly]

Though, looking at the Go SDK, it seems like it logs at the WARN level

Yes, this is one area where the Go SDK is currently inconsistent with the other server-side SDKs. By default it does not log anything at all for evaluation errors, because applications can detect those by looking at the error return value from the Variation call— in other languages there's no such thing as an error return value, they would have to throw an exception which is much more disruptive. The logEvaluationErrors option, if enabled, causes these messages to be logged at WARN level but it is not enabled by default.

or to implement custom logging by calling get_flag_state_relaxed

What is get_flag_state_relaxed?

[nc-wittj]

What is get_flag_state_relaxed?

Sorry, I accidentally copied an internal method... 😅
I meant variation_detail, which looking at it again, it actually looks like it does all the fallback logic anyways. So it wouldn't be too bad just to check errorKind if it was FLAG_NOT_FOUND and do our own logging.

Is that what you'd recommend?

[eli-darkly]

Yes, variation_detail would give you a way to detect that condition, and it behaves exactly the same as variation in terms of how the flag value is calculated.

[nc-wittj]

Ok, it sounds like at least for now, we should go that route. Feel free to close this PR. Thank you!