Skip to content
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

10432: Performance Measurement - TO TEST #5254

Closed
wants to merge 62 commits into from
Closed
Show file tree
Hide file tree
Changes from 59 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
d0ab8ac
10432: Add middleware around our sequences to measure how long the se…
Jul 31, 2024
dd5e41e
Merge branch 'staging' into 10432-story
cruzjone-flexion Jul 31, 2024
b6f37f2
10432: Type the arguments parameter;
Aug 1, 2024
c5eb1e3
Merge branch '10432-story' of github.com:flexion/ef-cms into 10432-story
Aug 1, 2024
07380f6
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 1, 2024
3a38e1f
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 2, 2024
a39dbbd
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 2, 2024
87eb967
10432: Create scripts to save sequence performance minimum time const…
Aug 6, 2024
380c5c5
10432: Wrap every action with logic to time how long each action take…
Aug 6, 2024
eff97a4
10432: Construct object with info we want to log;
Aug 6, 2024
82848dc
10432: Add typing in action;
Aug 6, 2024
941dbf5
10432: Create new endpoint to get system perormance data in back end;
Aug 6, 2024
ddd365a
10432: Use luxon instead of Date.now();
Aug 8, 2024
57c3356
10432: Update script to fetch "info" cluster from AWS;
Aug 8, 2024
c89d7fe
10432: Setup env with info cluster URL; create method to get info clu…
Aug 8, 2024
d7e4767
10432: Fetch Info Cluster Url for setting up env;
Aug 8, 2024
a3b4380
10432: Export variable;
Aug 8, 2024
4866e20
10432: Create index mappings for system performance logs;
Aug 8, 2024
3fddf93
10432: Made script to create index for system performance logs in Inf…
Aug 8, 2024
e78f332
10432: Create new permission to allow user to save system performance…
Aug 8, 2024
7da2c74
10432: Pass user to interactor;
Aug 8, 2024
ead4cf1
10432: Improve local system performance log;
Aug 8, 2024
6851bf3
10432: Fix reference issues in the action middleware;
Aug 8, 2024
6ffe153
10432: Remove new permission;
Aug 8, 2024
fcbab17
10432: Create new elastic search persistence method;
Aug 8, 2024
58e4f2c
10432: Call new elastic search method in interactor;
Aug 8, 2024
21fc761
Merge branch 'staging' of https://github.com/ustaxcourt/ef-cms into 1…
Aug 8, 2024
ee3ceb1
10432: Add default feature flags;
Aug 8, 2024
2191761
Merge remote-tracking branch 'taxCourt/staging' into 10432-story
Aug 9, 2024
ce38cfa
10432: Change endpoint;
Aug 9, 2024
9fc4074
10432: Check user is authorized to log performance data;
Aug 9, 2024
4be1711
10432: Add tests around new interactor;
Aug 9, 2024
6bf5405
10432: Add retry mechanism in cypress test;
Aug 9, 2024
bef8820
10432: Provide env variable for lambdas;
Aug 9, 2024
fd7109d
10432: Remove console.log from action;
Aug 9, 2024
e1bca79
10432: Add tests around "performanceMeasurementEndAction";
Aug 9, 2024
2acf15e
10432: Add tests around "performanceMeasurementStartAction";
Aug 9, 2024
0aee681
10432: Refactor "performanceMeasurementStartAction";
Aug 9, 2024
b3c03e9
10432: Add permission to lmabda to execute "es:ESHttpPost" to info cl…
Aug 9, 2024
4438256
10432: Format script file
Aug 12, 2024
a464e1d
10432: Fix Index mappings and changed from integers to float;
Aug 12, 2024
c8e07c0
10432: Keep function names when minimizing
Aug 12, 2024
b991050
10432: Remove policy from the INFO cluster;
Aug 13, 2024
9379100
10432: Remove aws_caller_identity;
Aug 13, 2024
6085387
10432: Remove cerebral sequence wrapper. Extract timing wrapper funct…
Aug 13, 2024
51e7282
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 13, 2024
03c6104
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 14, 2024
5263328
10432: Fix white spaces
Aug 14, 2024
53d6ff7
10432: Undo removing cluster policy;
Aug 14, 2024
e65b8f1
10432: Use CloudWatch instead of SE Cluster;
Aug 14, 2024
5fbcc6b
10432: Fix typing of sequence;
Aug 14, 2024
64322d8
10432: Allow lambda to put CloudWatch Metric data;
Aug 14, 2024
7db5777
10432: Fix json;
Aug 14, 2024
9219991
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 14, 2024
add497f
Merge branch 'staging' into 10432-story
cruzjone-flexion Aug 14, 2024
fbbc610
10432: Clean up info cluster setup;
Aug 14, 2024
6abdc8f
Merge branch '10432-story' of github.com:flexion/ef-cms into 10432-story
Aug 14, 2024
e99fb8a
Merge remote-tracking branch 'taxCourt/staging' into 10432-story
Aug 15, 2024
c9ccb80
Merge remote-tracking branch 'taxCourt/test' into 10432-test
Aug 15, 2024
8025ed9
10432: Remove passing the email from the front end; Implement a black…
Aug 15, 2024
93331f5
10432: Rename test;
Aug 15, 2024
12de0a6
Merge branch '10432-story' into 10432-test
Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,14 @@ describe('Document QC Complete', () => {
});

cy.visit('/document-qc/my/inbox');
cy.get(
`[data-testid="message-queue-docket-number-${unservedDocketNumber}"]`,
).should('be.visible');

retry(() => {
cy.reload(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the reload? (Just making sure I understand!)

cy.get('[data-testid="my-work-queue-inbox"]').should('be.visible');
return assertExists(
`[data-testid="message-queue-docket-number-${unservedDocketNumber}"]`,
);
});
});
});
});
Expand Down
1 change: 1 addition & 0 deletions esbuildHelper.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export default async function ({
entryNames: '[name].[hash]',
entryPoints: [`web-client/src/${entryPoint}`],
format: 'esm',
keepNames: true,
loader: {
'.html': 'text',
'.pdf': 'file',
Expand Down
31 changes: 31 additions & 0 deletions scripts/dynamo/setup-sequence-performance-minimum-time.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash

# creates the entry for minimum time for sequence performance logging in the dynamo deploy table

# Usage
# ENV=dev ./setup-sequence-performance-minimum-time.sh

./check-env-variables.sh \
"ENV" \
"AWS_SECRET_ACCESS_KEY" \
"AWS_ACCESS_KEY_ID"

ITEM=$(cat <<-END
{
"pk": {
"S": "sequence-performance-minimum-time"
},
"sk":{
"S": "sequence-performance-minimum-time"
},
"current": {
"N": "5"
}
}
END
)

aws dynamodb put-item \
--region us-east-1 \
--table-name "efcms-deploy-${ENV}" \
--item "${ITEM}"
1 change: 1 addition & 0 deletions scripts/env/environments/00-common
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ IRS_CLIENT_ID=$(aws cognito-idp list-user-pool-clients \
echo "DAWSON_ENV='${ENV}'"
echo "DYNAMODB_TABLE_NAME='${SOURCE_TABLE}'"
echo "ELASTICSEARCH_ENDPOINT='${ELASTICSEARCH_ENDPOINT}'"
echo "ELASTICSEARCH_INFO_ENDPOINT='${ELASTICSEARCH_INFO_ENDPOINT}'"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, and probably not worth doing if it involves changing infrastructure, but I wonder if we should give this a clearer name? Info can mean just about anything.

echo "EMAIL_CHANGE_VERIFICATION_TEMPLATE='email_change_verification_${ENV}'"
echo "EMAIL_DOCUMENT_SERVED_TEMPLATE='document_served_${ENV}'"
echo "EMAIL_SERVED_PETITION_TEMPLATE='petition_served_${ENV}'"
Expand Down
3 changes: 3 additions & 0 deletions shared/src/business/entities/EntityConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ export const ALLOWLIST_FEATURE_FLAGS = {
ENTITY_LOCKING_FEATURE_FLAG: {
key: 'entity-locking-feature-flag',
},
SEQUENCE_PERFORMANCE_MINIMUM_TIME: {
key: 'sequence-performance-minimum-time',
},
UPDATED_PETITION_FLOW: {
key: 'updated-petition-flow',
},
Expand Down
17 changes: 17 additions & 0 deletions shared/src/proxies/system/logUserPerformanceDataProxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { post } from '@shared/proxies/requests';

export const logUserPerformanceDataInteractor = (
applicationContext,
performanceData: {
sequenceName: string;
duration: number;
actionPerformanceArray: { actionName: string; duration: number }[];
email: string;
},
) => {
return post({
applicationContext,
body: { performanceData },
endpoint: '/log/performance-data',
});
};
11 changes: 11 additions & 0 deletions web-api/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ import { getUsersPendingEmailLambda } from './lambdas/users/getUsersPendingEmail
import { getWorkItemLambda } from './lambdas/workitems/getWorkItemLambda';
import { ipLimiter } from './middleware/ipLimiter';
import { lambdaWrapper } from './lambdaWrapper';
import { logUserPerformanceDataLambda } from '@web-api/lambdas/system/logUserPerformanceDataLambda';
import { logger } from './logger';
import { loginLambda } from '@web-api/lambdas/auth/loginLambda';
import { opinionAdvancedSearchLambda } from './lambdas/documents/opinionAdvancedSearchLambda';
Expand Down Expand Up @@ -1072,6 +1073,16 @@ app.delete(
app.put('/work-items', lambdaWrapper(assignWorkItemsLambda));
}

/**
* logging
*/
{
app.post(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this contribute to high server load if we end up logging a lot?

'/log/performance-data',
lambdaWrapper(logUserPerformanceDataLambda),
);
}

/**
* system
*/
Expand Down
17 changes: 17 additions & 0 deletions web-api/src/applicationContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { Case } from '../../shared/src/business/entities/cases/Case';
import { CaseDeadline } from '../../shared/src/business/entities/CaseDeadline';
import { Client } from '@opensearch-project/opensearch';
import { CloudWatchClient } from '@aws-sdk/client-cloudwatch';
import { CognitoIdentityProvider } from '@aws-sdk/client-cognito-identity-provider';
import { Correspondence } from '../../shared/src/business/entities/Correspondence';
import { DocketEntry } from '../../shared/src/business/entities/DocketEntry';
Expand Down Expand Up @@ -79,6 +80,7 @@ import sass from 'sass';

let sqsCache: SQSClient;
let searchClientCache: Client;
let cloudWatchClientCache: CloudWatchClient;

const entitiesByName = {
Case,
Expand Down Expand Up @@ -125,6 +127,21 @@ export const createApplicationContext = (
return await getChromiumBrowserAWS();
}
},
getCloudWatchClient: () => {
if (cloudWatchClientCache) return cloudWatchClientCache;

if (environment.stage === 'local') {
cloudWatchClientCache = {
send: (...args) => console.log('** SYSTEM PERFORMANCE LOGS', ...args),
} as CloudWatchClient;
} else {
cloudWatchClientCache = new CloudWatchClient({
region: environment.region,
});
}

return cloudWatchClientCache;
},
getCognito: (): CognitoIdentityProvider => {
if (environment.stage === 'local') {
return getLocalCognito();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { ROLES } from '@shared/business/entities/EntityConstants';
import { UnknownAuthUser } from '@shared/business/entities/authUser/AuthUser';
import { applicationContext } from '@shared/business/test/createTestApplicationContext';
import { logUserPerformanceDataInteractor } from '@web-api/business/useCases/system/logUserPerformanceDataInteractor';

describe('logUserPerformanceDataInteractor', () => {
const TEST_EMAIL = 'TEST_EMAIL@EXAMPLE.COM';

const PERFORMANCE_DATA: {
actionPerformanceArray: { actionName: string; duration: number }[];
duration: number;
email: string;
sequenceName: string;
} = {
actionPerformanceArray: [
{ actionName: 'TEST_ACTION_NAME_1', duration: 25 },
{ actionName: 'TEST_ACTION_NAME_2', duration: 25 },
{ actionName: 'TEST_ACTION_NAME_3', duration: 25 },
{ actionName: 'TEST_ACTION_NAME_4', duration: 25 },
],
duration: 100,
email: TEST_EMAIL,
sequenceName: 'TEST_SEQUENCE_NAME',
};

const TEST_USER: UnknownAuthUser = {
email: TEST_EMAIL,
name: 'TEST_NAME',
role: ROLES.docketClerk,
userId: 'SOME_TEST_USER_ID',
};

beforeEach(() => {
applicationContext
.getPersistenceGateway()
.saveSystemPerformanceData.mockImplementation(() => {});
});

it('should call persistence method with correct data', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it('should not allow unauthorized call to log data')

await expect(
logUserPerformanceDataInteractor(
applicationContext,
PERFORMANCE_DATA,
undefined,
),
).rejects.toThrow('Unauthorized to log performance data');
});

it('should call persistence method with correct data', async () => {
await logUserPerformanceDataInteractor(
applicationContext,
PERFORMANCE_DATA,
TEST_USER,
);

const saveSystemPerformanceDataCalls =
applicationContext.getPersistenceGateway().saveSystemPerformanceData.mock
.calls;

expect(saveSystemPerformanceDataCalls.length).toEqual(1);
expect(saveSystemPerformanceDataCalls[0][0].performanceData).toEqual(
PERFORMANCE_DATA,
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { ServerApplicationContext } from '@web-api/applicationContext';
import { UnauthorizedError } from '@web-api/errors/errors';
import { UnknownAuthUser } from '@shared/business/entities/authUser/AuthUser';

export const logUserPerformanceDataInteractor = async (
applicationContext: ServerApplicationContext,
performanceData: {
sequenceName: string;
duration: number;
actionPerformanceArray: { actionName: string; duration: number }[];
email: string;
},
authorizedUser: UnknownAuthUser,
): Promise<void> => {
if (!authorizedUser || !authorizedUser.userId) {
throw new UnauthorizedError('Unauthorized to log performance data');
}

await applicationContext
.getPersistenceGateway()
.saveSystemPerformanceData({ applicationContext, performanceData });
};
2 changes: 2 additions & 0 deletions web-api/src/getPersistenceGateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ import {
} from './persistence/dynamo/cases/removePractitionerOnCase';
import { saveDispatchNotification } from './persistence/dynamo/notifications/saveDispatchNotification';
import { saveDocumentFromLambda } from './persistence/s3/saveDocumentFromLambda';
import { saveSystemPerformanceData } from '@web-api/persistence/elasticsearch/system/saveSystemPerformanceData';
import { saveUserConnection } from './persistence/dynamo/notifications/saveUserConnection';
import { saveWorkItem } from './persistence/dynamo/workitems/saveWorkItem';
import { saveWorkItemForDocketClerkFilingExternalDocument } from './persistence/dynamo/workitems/saveWorkItemForDocketClerkFilingExternalDocument';
Expand Down Expand Up @@ -388,6 +389,7 @@ const gatewayMethods = {
removeIrsPractitionerOnCase,
removeLock,
removePrivatePractitionerOnCase,
saveSystemPerformanceData,
setChangeOfAddressCaseAsDone,
setStoredApplicationHealth,
uploadDocument,
Expand Down
2 changes: 2 additions & 0 deletions web-api/src/getUseCases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ import { getUsersInSectionInteractor } from './business/useCases/user/getUsersIn
import { getUsersPendingEmailInteractor } from './business/useCases/user/getUsersPendingEmailInteractor';
import { getWorkItemInteractor } from './business/useCases/workItems/getWorkItemInteractor';
import { handleBounceNotificationInteractor } from './business/useCases/email/handleBounceNotificationInteractor';
import { logUserPerformanceDataInteractor } from '@web-api/business/useCases/system/logUserPerformanceDataInteractor';
import { loginInteractor } from '@web-api/business/useCases/auth/loginInteractor';
import { onConnectInteractor } from './business/useCases/notifications/onConnectInteractor';
import { onDisconnectInteractor } from './business/useCases/notifications/onDisconnectInteractor';
Expand Down Expand Up @@ -354,6 +355,7 @@ const useCases = {
getUsersPendingEmailInteractor,
getWorkItemInteractor,
handleBounceNotificationInteractor,
logUserPerformanceDataInteractor,
loginInteractor,
onConnectInteractor,
onDisconnectInteractor,
Expand Down
18 changes: 18 additions & 0 deletions web-api/src/lambdas/system/logUserPerformanceDataLambda.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { UnknownAuthUser } from '@shared/business/entities/authUser/AuthUser';
import { genericHandler } from '@web-api/genericHandler';

export const logUserPerformanceDataLambda = (
event,
authorizedUser: UnknownAuthUser,
) =>
genericHandler(event, async ({ applicationContext }) => {
const { performanceData } = JSON.parse(event.body);

return await applicationContext
.getUseCases()
.logUserPerformanceDataInteractor(
applicationContext,
performanceData,
authorizedUser,
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import {
MetricDatum,
PutMetricDataCommand,
PutMetricDataCommandInput,
StandardUnit,
} from '@aws-sdk/client-cloudwatch';
import { ServerApplicationContext } from '@web-api/applicationContext';

export const saveSystemPerformanceData = async ({
applicationContext,
performanceData,
}: {
applicationContext: ServerApplicationContext;
performanceData: {
sequenceName: string;
duration: number;
actionPerformanceArray: { actionName: string; duration: number }[];
email: string;
};
}) => {
const { stage } = applicationContext.getEnvironment();

const cloudwatchClient = applicationContext.getCloudWatchClient();
const metricData: MetricDatum[] = [
{
Dimensions: [
{ Name: 'SequenceName', Value: performanceData.sequenceName },
],
MetricName: 'SequenceDuration',
Unit: 'Seconds',
Value: performanceData.duration,
},
...performanceData.actionPerformanceArray.map(action => ({
Dimensions: [
{ Name: 'SequenceName', Value: performanceData.sequenceName },
{ Name: 'ActionName', Value: action.actionName },
],
MetricName: 'ActionPerformance',
Unit: 'Seconds' as StandardUnit,
Value: action.duration,
})),
];

const params: PutMetricDataCommandInput = {
MetricData: metricData,
Namespace: `System-Performance-Log-${stage}`,
};

const command = new PutMetricDataCommand(params);

await cloudwatchClient.send(command);
};
5 changes: 5 additions & 0 deletions web-api/storage/fixtures/seed/efcms-local.json
Original file line number Diff line number Diff line change
Expand Up @@ -40527,6 +40527,11 @@
"current": "2023-05-01",
"pk": "document-visibility-policy-change-date"
},
{
"sk": "sequence-performance-minimum-time",
"current": "5",
"pk": "sequence-performance-minimum-time"
},
{
"sk": "e-consent-fields-enabled-feature-flag",
"current": true,
Expand Down
12 changes: 11 additions & 1 deletion web-api/terraform/modules/lambda-role/lambda-role.tf
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ resource "aws_iam_role_policy" "lambda_policy" {
],
"Effect": "Allow"
},
{
"Action": [
"cloudwatch:PutMetricData"
],
"Resource": [
"*"
],
"Effect": "Allow"
},
{
"Action": [
"lambda:InvokeFunction"
Expand Down Expand Up @@ -160,7 +169,8 @@ resource "aws_iam_role_policy" "lambda_policy" {
"es:ESHttpPut"
],
"Resource": [
"arn:aws:es:us-east-1:${data.aws_caller_identity.current.account_id}:domain/efcms-search-${var.environment}-*"
"arn:aws:es:us-east-1:${data.aws_caller_identity.current.account_id}:domain/efcms-search-${var.environment}-*",
"arn:aws:es:us-east-1:${data.aws_caller_identity.current.account_id}:domain/info"
],
"Effect": "Allow"
},
Expand Down
Loading
Loading