IHE Gateway V2 Monitoring #2200

jonahkaye · 2024-06-04T04:54:11Z

Ticket: #1667

TODO

also add to report analysis of DQs and DRs

Description

scheduled lambda for evaluating ihe v2 success rates.
removing noisy captures/sentry alerts + adding logging
introduces infra pattern of using read only endpoint
added an index on created_at for patient_discovery_result, document_query_result, document_retrieval_result

Testing

Local
- ran reporting script with test utis script
- performed sql migration and ran a speed test. 140x speed improvement in local env with index on the querries in this PR
Staging
- branch to staging - logs of it working on a 10 min increment schedule
- test on staging

Release Plan

Merge this

packages/infra/lib/api-stack.ts

…ces added on created_at Refs: #1667 Signed-off-by: Jonah Kaye <[email protected]>

jonahkaye · 2024-06-05T21:33:34Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

rename this file and folder monitor

leite08

I think we should avoid adding more code that uses Sentry on lambdas. We have this ticket to move to pg so we're lighter when hitting the DB from lambdas: https://github.com/metriport/metriport-internal/issues/1561

WRT to the reports themselves, I'd try to store more raw results in a .csv and send/log a link to it. That's because by putting it on a Spreadsheet we have more flexibility to work the data. Example: we're sending percentage of failure/success, but we might want to see the amount over time as well.

But NABD, just trying to avoid back-and-forth with code updates, PRs, releases, just to change how we see the data.

...rc/sequelize/migrations/2024-06-04_00_alter-patient-discovery-result-add-created-at-index.ts

...lize/migrations/2024-06-04_01_alter-document-query-retrieval-results-add-created-at-index.ts

packages/core/src/external/carequality/ihe-gateway-v2/ihe-gateway-v2-logic.ts

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

packages/infra/lib/api-stack.ts

leite08 · 2024-06-06T00:16:06Z

packages/infra/lib/ihe-gateway-v2-stack.ts

+ dbCredsSecret.grantRead(scheduledReportLambda);
+
+ const rule = new events.Rule(this, "ScheduledReportRule", {
+ schedule: events.Schedule.rate(Duration.hours(12)),


Will this run every 12h from the time its deployed? If so, wouldn't it be better to have specific times on the day so we have more predictable Ops?

I changed it to run at 12am and 2pm UTC (7am PST and 5pm PST

jonahkaye

Made requested changes, especially switching from sequelize to pg.
WRT to csv, I think I am gonna hold off on doing that for now.
The format this is in works for me and I dont see the value in overcomplicating things with csvs since this is really just for us to track things and isnt for outside reporting or analytics.

Refs: #1667 Signed-off-by: Jonah Kaye <[email protected]>

leite08 · 2024-07-02T23:39:11Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ SELECT request_id
+ FROM ${tableName}
+ WHERE created_at >= $1
+ ORDER BY created_at DESC


We should be referencing the TableColumns enum, right?

As for the Query, wouldn't be better to random on the request IDs filtered by created at?

leite08 · 2024-07-02T23:40:17Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ `;
+ const values = [since, limit];
+ const { rows } = await pool.query(query, values);
+ return rows.map(row => row.request_id);


ditto: row is probably any, right? We could use this instead, to at least make sure we're accessing the correct property:

... row[TableColumns.requestId]);

leite08 · 2024-07-02T23:40:50Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ request_id = "request_id",
+ created_at = "created_at",


NABD, but the prop for the column name could follow camelCase

leite08 · 2024-07-02T23:41:32Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ PatientDiscoveryResult = "patient_discovery_result",
+ DocumentQueryResult = "document_query_result",
+ DocumentRetrievalResult = "document_retrieval_result",


nit: props/enum items should be camelCase; PascalCase is for Classes/Types only.

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

leite08

I dont see the value in overcomplicating things with csvs

I actually think this code is more complicated than appending a line at the end of 3-6 files. This PR calculates percentages and summaries, while we could just add lines w/ raw data to files on S3.

Second, this assumes we're not changing how we assess the result/success/usage of the IHE GW. As we all know, things tend to change after we ship them. 😄

Lastly, how to you plan to look at data over time? Looking at logs and calculating in your head, or copy/pasting and parsing the results? If we have a CSV where we add a line for each new run, we already have data over time there.

Its not a blocker for me, but my experience tells me that we'd be better served with a handful of CSVs instead of logs on CloudWatch.

leite08 · 2024-07-02T23:49:29Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ result.patientMatch === true ||
+ result.operationOutcome?.issue?.some(issue => issue.code === "not-found")
+ );
+
+ const failures = results.filter(
+ (result: OutboundPatientDiscoveryResp) =>
+ !(
+ result.patientMatch === true ||
+ result.operationOutcome?.issue?.some(issue => issue.code === "not-found")


Since we're replicating the condition in verbatim, might as well write a small function isSuccess or something that we can use in both places, to avoid updating one and forgetting the other one.

leite08 · 2024-07-02T23:50:50Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ );
+
+ const errorOids = failures.map((error: OutboundPatientDiscoveryResp) => ({
+ oid: (error.gateway as XCPDGateway).oid,


No need to cast here, right?

leite08 · 2024-07-02T23:51:46Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ );
+
+ const errorOids = failures.map((error: OutboundDocumentQueryResp) => ({
+ oid: (error.gateway as XCAGateway).homeCommunityId,


ditto: no need to cast

leite08 · 2024-07-02T23:52:08Z

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

+ );
+
+ const errorOids = failures.map((error: OutboundDocumentRetrievalResp) => ({
+ oid: (error.gateway as XCAGateway).homeCommunityId,


ditto: cast

leite08 · 2024-07-03T00:02:46Z

packages/core/src/external/carequality/ihe-gateway-v2/ihe-gateway-v2-logic.ts

+ const errorString = errorToString(error);
+ log(errorString);
 capture.error("Failed to send PD response to Internal Carequality Endpoint", {


Can we also send the message to the log? Just the error w/o context is not as helpful.

We have a "pattern" for this:

const msg = "Failed to send PD response to Internal Carequality Endpoint"; log(`${msg} - ${errorToString(error)}`); capture.error(msg, {

leite08 · 2024-07-03T00:13:40Z

packages/utils/src/carequality/report.ts

+ ]);
+
+ fs.writeFileSync(
+ "./runs/carequality-report/patient-discovery-report.json",


NABD, but carequality-report is tied to a specific HIE, but this is IHE GW, which in theory could be other HIEs in the mid-term.

Also, no date on the file name?

leite08 · 2024-07-03T00:18:15Z

packages/utils/src/carequality/report.ts

+async function main() {
+ const sqlDBCreds = getEnvVarOrFail("DB_CREDS");
+ const readReplicaEndpoint = getEnvVarOrFail("DB_READ_REPLICA_ENDPOINT");


Missing initRunsFolder() - see other scripts that use the "runs" folder.

leite08 · 2024-07-03T00:19:23Z

packages/lambdas/src/ihe-gateway-v2-scheduled-report.ts

+const dbReadOnlyEndpoint = getEnvVarOrFail("DB_READ_REPLICA_ENDPOINT");
+const region = getEnvVarOrFail("AWS_REGION");
+
+capture.setExtra({ lambdaName: "scheduled-report-lambda" });


There's an env var set by AWS for this, see other lambdas - example:

// Automatically set by AWS const lambdaName = getEnvOrFail("AWS_LAMBDA_FUNCTION_NAME"); const region = getEnvOrFail("AWS_REGION"); // Set by us ...

leite08 · 2024-07-03T00:20:37Z

packages/core/src/util/pg.ts

+import { DbCreds, dbCredsSchema, dbReadReplicaEndpointSchema } from "./sequelize";
+// Initialize the main DB pool


supernit: new line

leite08 · 2024-07-03T00:23:30Z

packages/core/src/util/pg.ts

+ const pool = new Pool({
+ user: parsedDbCreds.username,
+ host: parsedDbCreds.host,
+ database: parsedDbCreds.dbname,
+ password: parsedDbCreds.password,
+ port: parsedDbCreds.port,
+ });


It would be good to have a const and set the pool size (property max). Even though the default 10 connections is fine for this use case, we're building something that might be used afterwards, so being explicit about this, with a link to the docs, is helpful - I had to search it up online to make sure there was more than 1 connection by default.

jonahkaye temporarily deployed to staging June 4, 2024 13:14 — with GitHub Actions Inactive

jonahkaye self-assigned this Jun 4, 2024

jonahkaye commented Jun 4, 2024

View reviewed changes

packages/infra/lib/api-stack.ts Show resolved Hide resolved

jonahkaye force-pushed the 1667-report-monitoring branch from 85c4f70 to 14122f5 Compare June 4, 2024 17:16

jonahkaye marked this pull request as ready for review June 4, 2024 17:17

jonahkaye requested review from Goncharo, leite08, Orta21 and RamilGaripov as code owners June 4, 2024 17:17

jonahkaye changed the title ~~1667 report monitoring~~ IHE Gateway V2 Monitoring Jun 4, 2024

feat(ihev2): ihe v2 reports, scheduled lambda with ro db access, indi…

17c7a2a

…ces added on created_at Refs: #1667 Signed-off-by: Jonah Kaye <[email protected]>

jonahkaye force-pushed the 1667-report-monitoring branch from 14122f5 to 17c7a2a Compare June 4, 2024 17:34

jonahkaye commented Jun 5, 2024

View reviewed changes

packages/core/src/external/carequality/ihe-gateway-v2/report/report.ts

Copy link

Member Author

jonahkaye Jun 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename this file and folder monitor

leite08 reviewed Jun 6, 2024

View reviewed changes

jonahkaye commented Jun 6, 2024

View reviewed changes

jonahkaye added 2 commits June 5, 2024 23:29

feat(ihev2): sequelize to pg and other changes

c48a62b

Refs: #1667 Signed-off-by: Jonah Kaye <[email protected]>

feat(ihe): develop to branch

93949b9

Refs: #1667 Signed-off-by: Jonah Kaye <[email protected]>

jonahkaye requested a review from thomasyopes as a code owner June 16, 2024 20:36

leite08 reviewed Jul 2, 2024

View reviewed changes

leite08 reviewed Jul 3, 2024

View reviewed changes

jonahkaye marked this pull request as draft July 3, 2024 14:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IHE Gateway V2 Monitoring #2200

IHE Gateway V2 Monitoring #2200

jonahkaye commented Jun 4, 2024 •

edited

Loading

jonahkaye Jun 5, 2024

leite08 left a comment

leite08 Jun 6, 2024

jonahkaye Jun 6, 2024

jonahkaye left a comment

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 left a comment

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 Jul 2, 2024

leite08 Jul 3, 2024

leite08 Jul 3, 2024

leite08 Jul 3, 2024

leite08 Jul 3, 2024

leite08 Jul 3, 2024

leite08 Jul 3, 2024

leite08 Jul 3, 2024

		import { DbCreds, dbCredsSchema, dbReadReplicaEndpointSchema } from "./sequelize";
		// Initialize the main DB pool

IHE Gateway V2 Monitoring #2200

Are you sure you want to change the base?

IHE Gateway V2 Monitoring #2200

Conversation

jonahkaye commented Jun 4, 2024 • edited Loading

TODO

Description

Testing

Release Plan

Choose a reason for hiding this comment

leite08 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonahkaye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leite08 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonahkaye commented Jun 4, 2024 •

edited

Loading