[BE] 10-10EZ - Improvement: Update HCA::LogEmailDiffJob to remove the need for a redis key with a ttl of -1

[coope93]

Background

A member of the Platform Product team (DevOps) informed VFS teams of some Redis keys that are not defined in the redis.yml configuration file of the vets-api repo. They also have a TTL (Time to live) of -1, which makes them persist indefinitely in the Redis store.

One of the keys was removed here, but the HCA::LogEmailDiffJob is still using a redis key with a ttl of -1. This implemenation needs updated to still be able to increment the appropriate metric in the job without needing a redis key that does not expire. A database table or new column somewhere may be an approprate solution, but further investigation is needed.

Resources

Slack conversation - Expand to view

• From Kshitiz Shrestha
  We have noticed some Redis keys that are not defined in the redis.yml configuration file of the vets-api repository. Additionally, these keys have a TTL (Time to Live) of -1, meaning they are persisting indefinitely in the Redis store.
  We would like your assistance in identifying where these keys are being set or defined within the vets-api codebase. Moreover, we are trying to understand why the TTL for these keys is set to -1, as this could lead to unnecessary persistence of data in Redis.
  Could you please provide guidance on where these keys are located and the reasoning behind the TTL configuration?
  Your help in resolving this matter is much appreciated.

• Brandon Cooper
  Just to clarify on two of those keys, are they Form1010cg:* or is that from a search where it is a key that starts with Form1010cg: and has the wildcard at the end for other values? The same question for the HCA:* key. Additionally, what is the (2700) with the key? Is that the value? Just making sure I understand what all I should be looking for.

• Lindsey Hattamer
  I believe that's a wildcard character :this: but I will let Kshitiz confirm

• Brandon Cooper
  Thanks! If that is the case, my guess is those two keys come from here and here. Those look like a redis key is being set that starts with HCA and Form1010cg. I tested it locally on the spec, but it looks like when you call redis.set without defining a ttl it defaults to -1, which is probably why we have these keys with -1 set. It looks like we call redis.set a few places in vets-api so these may be where the others are coming from.

• Lindsey Hattamer
  Thank you for the link :this: This is really helpful because we assumed that most folks just followed the namespaced redis pattern

For the keys that belong to your team, is a -1 TTL appropriate? It seems like we would want a TTL for those keys

• Brandon Cooper
  Yeah, I agree. I'll have to do a little more digging into it, but I don't think -1 is necessary for these. They're both used in sidekiq jobs so they should expire at some point for sure.

So for those two keys, it looks like I should add them to the redis.yml file as per the documentation to avoid key collisions/this issue in the future?

• Lindsey Hattamer
  Yeah exactly. The namespacing (through the redis-namespace) gem helps us to avoid the key collisions

• Kshitiz Shrestha
  Form1010cg and HCA is namespace for the key. 2700 in the parenthesis is the number of keys with TTL -1.

Tasks

• Implement a way to update the statsD metrics api.1010ez.in_progress_form_email.same and api.1010ez.in_progress_form_email.different, following the existing logic in the HCA::LogEmailDiffJob by either using the existing job or building something new.
• If a redis key is required, the redis key should be implemented per the platform documentation, ensuring to set TTL's in redis.yml configuration file of the vets-api repo.

Acceptance Criteria

• Any redis keys used by the 10-10 team will utilize the redis-namespace gem and follow all expected implementation norms.
• No redis keys with a ttl of -1 should exist that are owned by the 1010 Health Apps team

[coope93]

@allanto-ah I turned this on in staging. To validate we'll need to compare the va profile email to an in progress form that has an email. The job gets called after saving the in progress form.

[coope93]

After the second fix for the job, I created a form for a logged in user and set the Veteran email to one different than the profile email for the user account. The metric showed up for them being different here with the toggle turned on.

[coope93]

Adding this comment for future reference if need be since I talked to a few people about it. Something that I did discover with both the new and old implementation for those metrics. If a Veteran saves an in progress form with an email, it will log the same/different metric, but if they go change the email value on the same in progress form a second time it won't log it again. With the old implementation it won't log more than one for the same user, and with the new it will only log for a unique user/in progress form combo. So if I go start a second form for the same user account, it will increment that metric again based on the new in progress form. I have to imagine creating multiple in progress 10-10EZ forms for the same user account is somewhat rare, but I'm sure it does happen.