search

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

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
99 stars 70 forks source link

Resolve non-Facilities team uses of the VADOTGOV_FacilityLocator lighthouse API consumer key #18351

Open jilladams opened 1 month ago

jilladams commented 1 month ago

Problem Statement

VADOTGOV_FacilityLocator is our Lighthouse Facilities API consumer. This user is used in 2 known cases:

  1. facilities-api - which we own and operate (with endpoints like /facilities_api/v2/va)
  2. https://api.va.gov/v1/facilities/va ~which we do NOT own/operate.~ wrong. turns out we own this, too.

What: We would like #2 to stop using this API user / key. Why: Because then we end up owning finding / resolving traffic to deprecating endpoints, as happened with th V0 > V1 cutover

Context

This API originates here: https://github.com/department-of-veterans-affairs/vets-api/tree/master/lib/lighthouse/facilities The API consumer's key is set here: https://github.com/department-of-veterans-affairs/vets-api/blob/master/lib/lighthouse/facilities/configuration.rb#L19 (lighthouse.facilities.api_key, which belongs to the Facilities team's VADOTGOV_FacilityLocator API consumer)

We don't know who owns this API. We have asked many places:

Contributors include:

ACs

jilladams commented 1 month ago

Convo with prior contributors about what is going on here: https://dsva.slack.com/archives/C0FQSS30V/p1718747443195809

tl;dr: Patrick Vinograd can help us understand, but not right now. We'll come back to this when he has more time later.

eselkin commented 1 month ago

This delineation seems correct though.

jilladams commented 1 month ago

Facilities owns it: https://github.com/department-of-veterans-affairs/platform-atlas/blob/6647cda5fff9fc8a344952f3d5ad88b0c65c6ba7/vets-api-ASSIGNED-CODEOWNERS#L1406

So: we can decide to prune this tech debt at any point (with appropriate comms, scheduling, etc). We also now know that users include VA Mobile app who updated it to V1 LH, bless them.

jilladams commented 3 weeks ago

Discussion with IIR today about /v0/facilities (the legacy API client): https://dsva.slack.com/archives/C0FQSS30V/p1719428540258619. Salient points:

@mmiddaugh I think we should try to chase down whether we should have ownership of this sooner than later if we can, since it's still getting traffic. Let me know if there's some way I can help?

patrickvinograd commented 2 weeks 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)

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

patrickvinograd commented 2 weeks ago 
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;
patrickvinograd commented 2 weeks ago

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 2 weeks ago

@FranECross @Agile6MSkinner @mmiddaugh we need to provide Eli some time in Sprint 8 to review these notes from Patrick, and sort out next steps with the 4 of us, probably. @eselkin let's make sure to discuss in planning tomorrow. I've queued the ticket accordingly.

eselkin commented 2 weeks ago

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

But basically:

  1. Should decommission and remove not the library but the endpoint for /v1/facilities/va
  2. 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.
  3. Should talk to Mobile about decommissioning and removing the main app API (not V1) library after CovidVaccine is removed.
  4. 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.
eselkin commented 2 weeks ago

The use of our API Key is not necessarily the most important thing, because we are nowhere near our rate limit. We get up to like 400/minute and the limit is 10x that.

eselkin commented 2 weeks ago

2 Sources of datadog traffic for /v1/facilties/va

  1. CHAMPVA%20Launcher/3 CFNetwork/1496.0.7 Darwin/23.5.0 and
  2. Referrers like: https://www.va.gov/find-locations/facility/vha_671GS
    1. but if you go to that page it's calling FacilitiesApi V2 so it must be in some wayback machine or something else.
jilladams commented 1 week ago

Deprecation of the V1 endpoint is ticketed: https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18487

jilladams commented 1 week ago

Adding the remainder of Eli's notes to ACs.