Deprecate legacy Facilities API client

jilladams commented 21 hours ago

Description

In https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18351 we determined that the mobile team are the last users of the legacy facilities API client. They plan to migrate to the modern facilities-api starting in December 2024. When their migration is complete, we can deprecate the legacy client.

Mobile team migration

Mobile team must complete migration before this work begins.

Slack thread with all relevant points of contact: https://dsva.slack.com/archives/C018V2JCWRJ/p1732212235321749?thread_ts=1732111200.531149&cid=C018V2JCWRJ

Engineering notes / background

Included in comments on #18351 and copy/pasted to comments on this ticket. Patrick VInograd was the subject matter expert, and he is no longer with VA.

Testing & QA

Acceptance criteria

[ ] Deprecation timeline is announced in Team of Teams at least 1 month before merge
[ ] Deprecation timeline is announced weekly until merge
[ ] Legacy API client is removed from vets-api and no longer available for use or adoption
[ ] Swagger is updated
[ ] Documentation is updated: https://github.com/department-of-veterans-affairs/va.gov-team/tree/master/products/facilities/facilities-api#legacy-facilities-api-client

jilladams commented 21 hours ago

Facility APIs: Controller: app/controllers/v1/facilities/va_controller.rb Route: api.va.gov/v1/facilities/va Calls methods on Lighthouse::Facilities::Client.new aka uses LH v0 API

Controller: modules/facilities_api/app/controllers/facilities_api/v1/va_controller.rb Route: api.va.gov/facilities_api/v1/va Instantiates: FacilitiesApi::V1::Lighthouse::Client.new

Controller: modules/facilities_api/app/controllers/facilities_api/v2/va_controller.rb Route: api.va.gov/facilities_api/v2/va Instantiates: FacilitiesApi::V2::Lighthouse::Client.new

API traffic: Metrics Explorer | Datadog (ddog-gov.com)

Other Client Uses: MedicalCopays::ZeroBalanceStatements -> Lighthouse::Facilities::Client CovidVaccine::V0::FacilityLookupService -> Lighthouse::Facilities::Client CovidVaccine::V0::FacilitySuggestionService -> Lighthouse::Facilities::Client

Mobile::FacilitiesHelper -> Lighthouse::Facilities::Client (per feature flag) Mobile::FacilitiesHelper -> Lighthouse::Facilities::V1::Client (per feature flag)

https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18351#issuecomment-2211459157

V0::HealthcareApplicationController -> Lighthouse::Facilities::V1::Client

Non-Uses: VAOS::V2::FacilitiesController - uses a facility service in the MAP platform, not LH

API Clients:

lib/lighthouse/facilities/client.rb Lighthouse::Facilities::Client Invokes: /services/va_facilities/v0/facilities (Lighthouse v0 API)

lib/lighthouse/facilities/v1/client.rb Lighthouse::Facilities::V1::Client Invokes: /services/va_facilities/v1/facilities (Lighthouse v1 API)

Both main app clients rely on the same Lighthouse::Facilities::Configuration class • Same base_path = Settings.lighthouse.facilities.url • Same apiKey = Settings.lighthouse.facilities.api_key

modules/facilities_api/app/services/facilities_api/v1/lighthouse/client.rb FacilitiesApi::V1::Lighthouse::Client Invokes: /services/va_facilities/v0/facilities (Lighthouse v0 API) Configuration = V1::Lighthouse::Configuration • base_path = Settings.lighthouse.facilities_url • api_key = Settings.lighthouse.facilities.api_key

modules/facilities_api/app/services/facilities_api/v2/lighthouse/client.rb FacilitiesApi::V2::Lighthouse::Client Invokes: /services/va_facilities/v1/facilities (Lighthouse v1 API) Configuration = V2::Lighthouse::Configuration • base_path = Settings.lighthouse.facilities_url api_key = Settings.lighthouse.facilities.api_key

jilladams commented 21 hours ago

https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18351#issuecomment-2211477163

flowchart LR
  LHv0[Lighthouse v0 API\n/services/va_facilities/v0/facilities]
  LHv1[Lighthouse v1 API\n/services/va_facilities/v1/facilities]
  mainAppClient[Lighthouse::Facilities::Client]
  mainAppClientv1[Lighthouse::Facilities::V1::Client]
  moduleClientv1[FacilitiesApi::V1::Lighthouse::Client]
  moduleClientv2[FacilitiesApi::V2::Lighthouse::Client]
  appApiv1(VA.gov API: api.va.gov/v1/facilities/va\nV1::Facilities::VaController)
  moduleApiv1(VA.gov API: api.va.gov/facilities_api/v1/va\nFacilitiesApi::V1::VaController)
  moduleApiv2(VA.gov API: api.va.gov/facilities_api/v2/va\nFacilitiesApi::V2::VaController)
  appConfig[Lighthouse::Facilities::Configuration]
  appConfigAlias[Lighthouse::Facilities::Configuration]
  moduleConfigv1[FacilitiesApi::V1::Lighthouse::Configuration]
  moduleConfigv2[FacilitiesApi::V2::Lighthouse::Configuration]
  classDef sitewide fill:#90EE90
  class appApiv1,moduleApiv1,moduleApiv2 sitewide;

  copays[MedicalCopays::ZeroBalanceStatements]
  covidVax[CovidVaccine::V0::FacilityLookupService\nCovidVaccine::V0::FacilitySuggestionService]
  mobileA[Mobile::FacilitiesHelper\n*Per Feature Flag]
  mobileB[Mobile::FacilitiesHelper\n*Per Feature Flag]
  healthcareApp[V0::HealthcareApplicationController]

  subgraph appcli [main app API client]
    direction TB
    mainAppClient-->appConfig
  end
  subgraph modcliv1 [module API client v1]
    direction TB
    moduleClientv1-->moduleConfigv1;
  end
  subgraph appcliv1 [main app API client v1]
    direction TB
    mainAppClientv1-->appConfigAlias
  end
  subgraph modcliv2 [module API client v2]
    direction TB
    moduleClientv2-->moduleConfigv2;
  end
  appApiv1-->appcli;
  moduleApiv1-->modcliv1;
  moduleApiv2-->modcliv2;
  appcli-->LHv0;
  modcliv1-->LHv0;
  appcliv1-->LHv1;
  modcliv2-->LHv1;
  copays-->appcli;
  covidVax-->appcli;
  healthcareApp-->appcliv1;
  mobileA-->appcli;
  mobileB-->appcliv1;

jilladams commented 21 hours ago

https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18351#issuecomment-2211490286 The above encapsulates all the uses of LH facilities APIs in vets-api.

In terms of API key usage - there are 4 distinct API clients (the yellow-ish boxes) with 3 distinct configuration classes (both of the "main app" clients use the same configuration class. But all of those configuration classes (and hence all the API clients) refer to the same API key setting: Settings.lighthouse.facilities.api_key. So if there's a desire to split things up, the first step would be to provision a new API key, get it configured in the parameter store and create a new entry for it in the settings file, and then reference it from whichever API client/configuration classes is appropriate.

The two APIs and API clients in the facilities_api module seem clearly to be owned by the sitewide/facilities team, to power the VA.gov facility locator, so if the main goal is to isolate "facility locator usage" vs. "all other usage", then those two Configuration classes in the module should be updated to point to a new Settings entry with the facility-locator specific key.

That leaves the uses of the two clients in the main app:

The V1::Facilities::VaController is being presented at (api.va.gov/v1/facilities/va), and in the Datadog graph I posted two comments ago, is receiving more than zero traffic. If that's not from the facility locator, then it would behoove the sitewide/facilities team to figure out where that API traffic is coming from.

Then there are the non-facilities-API uses - the medical copay, covid vaccine, mobile app, and healthcare application. The three that are pointing to LH v0 API should be confirmed whether they are still active - I believe covid vaccine is slated for decomissioning, not sure about medical copays, and the mobile app may have switched their feature flag to use v1. The two currently pointing to LH v1 API (mobile, healthcare application) are legit uses of the LH facilities API so as above, if that client/configuration can be issued a separate API key from the facility locator uses, then things should be more clear. Do they need separate API keys from one another? Unclear.

Finally, who owns the main app API clients? Unfortunately this is a long-standing problem in vets-api. Ownership of apps/APIs is clear, but the code under /lib frequently slips into shared library territory. The facility locator was the first team to need this integration, but once built, it made sense for other teams to use it. The sitewide team could throw up their hands and say "our app doesn't need that anymore, we're not supporting shared libraries for other VFS teams." and retreat to its module.

However, my opinionated take is that this re-use of upstream integrations constitutes one of the main strategic advantages of vets-api and therefore should be encouraged and accounted for. Otherwise every team will build a private integration with Lighthouse, costing us more time and money and reducing consistency of implementation. So my take is that the facilities team should not only build the facility locator that veterans use, it should be the steward of this facilities client API code as a shared benefit to allow all parts of VA.gov to access this data domain. And you should make it clear to your product owners that that is on your plate and requires resources to support.

jilladams commented 21 hours ago

https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18351#issuecomment-2218951623

Tried to follow the diagram and mark it up... Screenshot 2024-07-09 at 3 50 01 PM

But basically:

Should decommission and remove not the library but the endpoint for /v1/facilities/va
Should talk to Medical Copays and see if they are using it still. If still using it, talk to them about using V1 that Mobile updated.
Should talk to Mobile about decommissioning and removing the main app API (not V1) library after CovidVaccine is removed.
Despite the original idea of a shared library being good, V1 of Facilities API was not built on it years ago and we modeled V2 on V1. Facilities API also has CCP client/endpoint in addition to VA client/endpoint. Also the library was not versioned at the time when FacilitiesApi V1 was created. We should ask Mobile if they want to take ownership of "Main App V1 library"? If they do not want to, we can keep ownership of it and make V3 of facilities API use V2 of Main App in the future.

jilladams commented 20 hours ago

@Agile6MSkinner @mmiddaugh (cc @eselkin) FYI that today the Mobile team agreed to migrate to the modern facilities API. Once their migration is complete, we can take on this update to deprecate the legacy API client that they have been using.

I've updated the Q4 priority list with this update, and this is still blocked, but something we could consider for Q1.

department-of-veterans-affairs / va.gov-cms