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

Azure load tests #1665

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9c64c15
Locust load test works in Azure
halprin Dec 16, 2024
6ef84a2
Use an env var to tell whether running in Azure
halprin Dec 17, 2024
e90565c
Add some TODOs and update comment on why we avoid failing locust when…
halprin Dec 17, 2024
790abd7
Play with doing thread local registrations in the ApplicationContext
halprin Dec 17, 2024
0f55439
Make Inject work on the per thread injects
halprin Dec 19, 2024
34a783a
Merge branch 'main' into azure-load-tests
halprin Jan 3, 2025
54cfc1b
Add some comments and delete some commented out code
halprin Jan 3, 2025
7f8b16b
Pass in the Load-Test header in the load tests
halprin Jan 3, 2025
7896fd0
Check for load tests on the metadata endpoint too
halprin Jan 3, 2025
022e9b5
pass load-test header to metadata endpoint for load tests
halprin Jan 3, 2025
8be024f
Add additional code comments about the thread specific implementations
halprin Jan 3, 2025
fcd9c16
Remove old debug logging
halprin Jan 3, 2025
be3a898
Started Azure Load Test Action creation
jherrflexion Jan 3, 2025
9a013b0
Merge branch 'azure-load-tests' of https://github.com/CDCgov/trusted-…
jherrflexion Jan 3, 2025
565d2b6
add permissions block to workflow
JeremyIR Jan 4, 2025
b085490
Added secret definitions
jherrflexion Jan 6, 2025
d8ae1a4
try to fix az login with set oidc to true
JeremyIR Jan 6, 2025
c905fe7
specify environment in gh actions
JeremyIR Jan 6, 2025
467cc3b
Fixed parameters for az load test-run
jherrflexion Jan 6, 2025
fbdf77e
Added resource group
jherrflexion Jan 6, 2025
d3aa1da
Updated test name
jherrflexion Jan 6, 2025
283abd1
add inputs for workflow dispatch with conditional logic
JeremyIR Jan 6, 2025
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
77 changes: 77 additions & 0 deletions .github/workflows/azure-load-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
name: Azure Load Tests

on:
push:
branches:
- azure-load-tests
#schedule:

#- cron: "0 0 * * 1"

workflow_dispatch:
inputs:
load-test-name:
required: true
type: string
description: name of load test that is run
test-id:
required: true
type: string
description: test id for load test

workflow_call:
secrets:
AZURE_CLIENT_ID:
required: true
AZURE_TENANT_ID:
required: true
AZURE_SUBSCRIPTION_ID:
required: true

jobs:
loadtest:
name: Load Test
environment:
name: internal

runs-on: ubuntu-latest
permissions:
id-token: write
contents: read

steps:
# Checkout the repository
- name: Checkout Repository
uses: actions/checkout@v2

# Login to Azure using the CLI
- name: Login via Azure CLI
uses: azure/login@v2
with:
client-id: ${{ secrets.AZURE_CLIENT_ID }}
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}


# Run the Azure Load Test
- name: Run Load Test On Push
if: github.event_name == 'push'
run: |
az load test-run create \
--resource-group "csels-rsti-internal-moderate-rg" \
--load-test-resource "load-testing-internal" \
--test-id "9020b745-5fc4-4284-8803-04076ea09650" \
--test-run-id "run_"`date +"%Y%m%d%_H%M%S"`
# Run the Azure Load Test

- name: Trigger Load Test
if: github.event_name == 'workflow_dispatch'
run: |
az load test-run create \
--resource-group "csels-rsti-internal-moderate-rg" \
--load-test-resource "${{ github.event.inputs.load-test-name }}" \
--test-id "${{ github.event.inputs.test-id }}" \
--test-run-id "run_"`date +"%Y%m%d%_H%M%S"`



Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ static DomainConnector constructNewDomainConnector(Class<? extends DomainConnect
static Handler createHandler(
Function<DomainRequest, DomainResponse> handler, boolean isProtected) {
return (Context ctx) -> {
ApplicationContext
.clearThreadRegistrations(); // clear this thread's specific registrations from
// its previous use

LOGGER.logInfo(ctx.method().name() + " " + ctx.url());

var request = javalinContextToDomainRequest(ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ DomainResponse handleResults(DomainRequest request) {
}

DomainResponse handleMetadata(DomainRequest request) {
if (Boolean.parseBoolean(request.getHeaders().get("load-test"))
&& ApplicationContext.isPropertyPresent("REPORT_STREAM_URL_PREFIX")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Application Context is set when we build the app?

Copy link
Member

Choose a reason for hiding this comment

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

I'm having a hard time understanding your question here. Can you rephrase it?

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries, I think I answered my own silly question. I forgot the that the whole point of env variables was to allow them to be set differently for each environment lol.

// register the mock RS endpoint for this HTTP request because we don't want to call RS
// for real when doing a load test.
ApplicationContext.registerForThread(
RSEndpointClient.class, MockRSEndpointClient.getInstance());
}

try {
String metadataId = request.getPathParams().get("id");
Optional<PartnerMetadata> metadataOptional =
Expand Down Expand Up @@ -226,6 +234,14 @@ protected DomainResponse handleMessageRequest(
boolean markMetadataAsFailed = false;
String errorMessage = "";

if (Boolean.parseBoolean(request.getHeaders().get("load-test"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since any new places that call RS will also need to copy this code, we should document that (probably both in a comment here and the other location we evaluate this, plus in docs)

&& ApplicationContext.isPropertyPresent("REPORT_STREAM_URL_PREFIX")) {
// register the mock RS endpoint for this HTTP request because we don't want to call RS
// for real when doing a load test.
ApplicationContext.registerForThread(
RSEndpointClient.class, MockRSEndpointClient.getInstance());
}

try {
return requestHandler.handle(inboundReportId);
} catch (FhirParseException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package gov.hhs.cdc.trustedintermediary.etor.metadata.partner;

import gov.hhs.cdc.trustedintermediary.context.ApplicationContext;
import gov.hhs.cdc.trustedintermediary.etor.RSEndpointClient;
import gov.hhs.cdc.trustedintermediary.etor.messagelink.MessageLink;
import gov.hhs.cdc.trustedintermediary.etor.messagelink.MessageLinkException;
Expand Down Expand Up @@ -31,7 +32,6 @@ public class PartnerMetadataOrchestrator {

@Inject PartnerMetadataStorage partnerMetadataStorage;
@Inject MessageLinkStorage messageLinkStorage;
@Inject RSEndpointClient rsclient;
@Inject Formatter formatter;
@Inject Logger logger;

Expand All @@ -44,6 +44,9 @@ private PartnerMetadataOrchestrator() {}
public void updateMetadataForInboundMessage(PartnerMetadata partnerMetadata)
throws PartnerMetadataException {

// can't @Inject because the implementation can be different for this specific thread
RSEndpointClient rsclient = ApplicationContext.getImplementation(RSEndpointClient.class);

logger.logInfo(
"Looking up sender name and timeReceived from RS delivery API for inboundReportId: {}",
partnerMetadata.inboundReportId());
Expand Down Expand Up @@ -130,6 +133,11 @@ public Optional<PartnerMetadata> getMetadata(String inboundReportId)
PartnerMetadata partnerMetadata = optionalPartnerMetadata.get();
var outboundReportId = partnerMetadata.outboundReportId();
if (metadataIsStale(partnerMetadata) && outboundReportId != null) {

// can't @Inject because the implementation can be different for this specific thread
RSEndpointClient rsclient =
ApplicationContext.getImplementation(RSEndpointClient.class);

logger.logInfo(
"Receiver name not found in metadata or delivery status still pending, looking up {} from RS history API",
outboundReportId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
import gov.hhs.cdc.trustedintermediary.etor.RSEndpointClient;
import gov.hhs.cdc.trustedintermediary.wrappers.AuthEngine;
import gov.hhs.cdc.trustedintermediary.wrappers.Cache;
import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir;
import gov.hhs.cdc.trustedintermediary.wrappers.HttpClient;
import gov.hhs.cdc.trustedintermediary.wrappers.HttpClientException;
import gov.hhs.cdc.trustedintermediary.wrappers.Logger;
import gov.hhs.cdc.trustedintermediary.wrappers.MetricMetadata;
import gov.hhs.cdc.trustedintermediary.wrappers.SecretRetrievalException;
import gov.hhs.cdc.trustedintermediary.wrappers.Secrets;
import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter;
Expand Down Expand Up @@ -45,13 +43,10 @@ public class ReportStreamEndpointClient implements RSEndpointClient {
@Inject private HttpClient client;
@Inject private AuthEngine jwt;
@Inject private Formatter formatter;
@Inject private HapiFhir fhir;
@Inject private Logger logger;
@Inject private Secrets secrets;
@Inject private Cache cache;

@Inject MetricMetadata metadata;

private static final ReportStreamEndpointClient INSTANCE = new ReportStreamEndpointClient();

public static ReportStreamEndpointClient getInstance() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package gov.hhs.cdc.trustedintermediary.external.reportstream;

import gov.hhs.cdc.trustedintermediary.context.ApplicationContext;
import gov.hhs.cdc.trustedintermediary.etor.RSEndpointClient;
import gov.hhs.cdc.trustedintermediary.etor.messages.UnableToSendMessageException;
import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep;
Expand All @@ -17,7 +18,6 @@
public class ReportStreamSenderHelper {
private static final ReportStreamSenderHelper INSTANCE = new ReportStreamSenderHelper();

@Inject RSEndpointClient rsclient;
@Inject Formatter formatter;
@Inject Logger logger;
@Inject MetricMetadata metadata;
Expand All @@ -41,6 +41,10 @@ public Optional<String> sendResultToReportStream(String body, String fhirResourc
protected Optional<String> sendToReportStream(
String body, String fhirResourceId, PartnerMetadataMessageType messageType)
throws UnableToSendMessageException {

// can't @Inject because the implementation can be different for this specific thread
RSEndpointClient rsclient = ApplicationContext.getImplementation(RSEndpointClient.class);

String bearerToken;
String rsResponseBody;

Expand Down
41 changes: 34 additions & 7 deletions operations/locustfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import urllib.parse
import urllib.request
import uuid
import os

from locust import FastHttpUser, between, events, task
from locust.runners import MasterRunner
Expand All @@ -19,6 +20,8 @@
result_request_body = None
auth_request_body = None

in_azure = os.getenv('TEST_RUN_NAME') is not None


class SampleUser(FastHttpUser):
# Each task gets called randomly, but the number next to '@task' denotes
Expand Down Expand Up @@ -69,6 +72,7 @@ def post_message_request(self, endpoint, message):
headers={
"Authorization": self.access_token,
"RecordId": self.submission_id,
"Load-Test": "true",
},
data=message.replace("{{placer_order_id}}", poi),
)
Expand All @@ -88,7 +92,10 @@ def get_v1_etor_metadata(self):
if self.message_api_called:
self.client.get(
f"{METADATA_ENDPOINT}/{self.submission_id}",
headers={"Authorization": self.access_token},
headers={
"Authorization": self.access_token,
"Load-Test": "true",
},
name=f"{METADATA_ENDPOINT}/{{id}}",
)

Expand Down Expand Up @@ -118,6 +125,10 @@ def test_start(environment):

@events.quitting.add_listener
def assert_stats(environment):
if in_azure:

Choose a reason for hiding this comment

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

Ensure that the environment-specific paths and configurations, such as file paths and environment variables, are documented and validated to prevent runtime errors in different environments. [medium]

# don't evaluate this in Azure because we want the locust process to succeed and Azure does its own test criteria checking
return

if environment.stats.total.fail_ratio > 0.01:
logging.error("Test failed due to failure ratio > 1%")
environment.process_exit_code = 1
Expand All @@ -131,22 +142,38 @@ def assert_stats(environment):
def get_auth_request_body():
# set up the sample request body for the auth endpoint
# using a valid test token found in the mock_credentials directory
auth_scope = "report-stream"
with open("mock_credentials/report-stream-valid-token.jwt") as f:
auth_token = f.read()

# TODO - notes/clarification on 2 different creds, plus expiration date of jwt
# TODO - do we want to TF the tests? If yes which envs? In CDC envs, may need to adjust IP allow list on app. Also set as private endpoints in test config?
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good question. My assumption is that we don't want load test bombarding CDC environments unless they're okay with that. If they aren't, this could just be an internal running load test. However, I feel if we only run it in internal, it loses value since that would mean we're only running these until we pass the application off to the CDC.

# TODO - currently in Azure we're specifying a version for the key vault item (so if it gets updated, we'll be referencing an old version) - do we want to change this?
if in_azure:
auth_token = os.getenv("trusted-intermediary-valid-token-jwt")
else:
with open("mock_credentials/trusted-intermediary-valid-token.jwt") as f:
auth_token = f.read()

params = urllib.parse.urlencode(
{"scope": auth_scope, "client_assertion": auth_token.strip()}
{"scope": "trusted-intermediary", "client_assertion": auth_token.strip()}
)

return params.encode("utf-8")


def get_order_fhir_message():
# read the sample request body for the orders endpoint
with open("examples/Test/e2e/orders/002_ORM_O01_short.fhir", "r") as f:
file_path = "002_ORM_O01_short.fhir"
if not in_azure:
file_path = "examples/Test/e2e/orders/" + file_path

with open(file_path, "r") as f:
return f.read()


def get_result_fhir_message():
# read the sample request body for the results endpoint
with open("examples/Test/e2e/results/001_ORU_R01_short.fhir", "r") as f:
file_path = "001_ORU_R01_short.fhir"
if not in_azure:
file_path = "examples/Test/e2e/results/" + file_path

with open(file_path, "r") as f:
return f.read()
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.nio.file.attribute.PosixFilePermissions;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -28,6 +29,8 @@
public class ApplicationContext {

protected static final Map<Class<?>, Object> OBJECT_MAP = new ConcurrentHashMap<>();
protected static final InheritableThreadLocal<Map<Class<?>, Object>> THREAD_OBJECT_MAP =
new InheritableThreadLocal<>();
protected static final Map<String, String> TEST_ENV_VARS = new ConcurrentHashMap<>();
protected static final Set<Object> IMPLEMENTATIONS = new HashSet<>();

Expand All @@ -40,7 +43,39 @@ public static void register(Class<?> clazz, Object implementation) {
IMPLEMENTATIONS.add(implementation.getClass());
}

/**
* Registers an implementation for a class _only_ for the current executing thread (which
* currently is one-to-one with an HTTP request).
*/
public static void registerForThread(Class<?> clazz, Object implementation) {
Map<Class<?>, Object> threadObjectMap = THREAD_OBJECT_MAP.get();
if (threadObjectMap == null) {
threadObjectMap = new HashMap<>();
}

threadObjectMap.put(clazz, implementation);

THREAD_OBJECT_MAP.set(threadObjectMap);

// The implementation may never have had anything injected into it
// (e.g. it wasn't part of the bootstrapping implementations registered into the
// ApplicationContext),
// so inject into the implementation now.
injectIntoNonSingleton(implementation);
}

/** Removes the stored implementations for the current thread that calls this method. */
public static void clearThreadRegistrations() {

Choose a reason for hiding this comment

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

Consider using a more robust thread-local cleanup strategy. The current use of THREAD_OBJECT_MAP.remove() might lead to memory leaks if not all paths that add to the thread-local store also ensure to clear it after use. [important]

THREAD_OBJECT_MAP.remove();
}

public static <T> T getImplementation(Class<T> clazz) {
// check the thread local map first
Map<Class<?>, Object> threadObjectMap = THREAD_OBJECT_MAP.get();
if (threadObjectMap != null && threadObjectMap.containsKey(clazz)) {
return (T) threadObjectMap.get(clazz);
}

T object = (T) OBJECT_MAP.get(clazz);

if (object == null) {
Expand Down
Loading