-
Notifications
You must be signed in to change notification settings - Fork 70
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
Resolve non-Facilities team uses of the VADOTGOV_FacilityLocator lighthouse API consumer key #18351
Comments
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. |
This delineation seems correct though. |
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. |
Discussion with IIR today about
@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? |
Facility APIs: Controller: modules/facilities_api/app/controllers/facilities_api/v1/va_controller.rb Controller: modules/facilities_api/app/controllers/facilities_api/v2/va_controller.rb API traffic: Other Client Uses: Mobile::FacilitiesHelper -> Lighthouse::Facilities::Client (per feature flag) V0::HealthcareApplicationController -> Lighthouse::Facilities::V1::Client Non-Uses: API Clients: lib/lighthouse/facilities/client.rb lib/lighthouse/facilities/v1/client.rb Both main app clients rely on the same Lighthouse::Facilities::Configuration class modules/facilities_api/app/services/facilities_api/v1/lighthouse/client.rb modules/facilities_api/app/services/facilities_api/v2/lighthouse/client.rb |
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;
|
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: 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. |
@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. |
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. |
2 Sources of datadog traffic for /v1/facilties/va
|
Deprecation of the V1 endpoint is ticketed: #18487 |
Adding the remainder of Eli's notes to ACs. |
Per Donald McCaughey: Don to have someone on his backend team take a look at scope in order to use the facilities-api, rather than the legacy API client. Their team will need to understand if the data is equivalent between the two, in order to determine if migration has a clear path OR what the effort / timing might be. |
Sent follow up email to Don to check in on any progress. cced Michael / Michelle. |
WOO! So: after they make that change, we will be able to deprecate any additional portions of the legacy API client. I'm ticketing that work separately and closing this epic. |
Problem Statement
VADOTGOV_FacilityLocator
is our Lighthouse Facilities API consumer. This user is used in 2 known cases:facilities-api
- which we own and operate (with endpoints like/facilities_api/v2/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'sVADOTGOV_FacilityLocator
API consumer)We don't know who owns this API. We have asked many places:
Contributors include:
ACs
If not, ask owners to get their own API consumer from Lighthouse, to use in vets-apiseems like the wrong direction to go, since we own it.The text was updated successfully, but these errors were encountered: