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

OpenHIM Platform performance testing #321

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

OpenHIM Platform performance testing #321

wants to merge 13 commits into from

Conversation

drizzentic
Copy link
Collaborator

@drizzentic drizzentic commented Aug 12, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a comprehensive Grafana dashboard for monitoring performance metrics from k6 load testing.
    • Developed scripts for various performance testing scenarios: load, smoke, and volume testing for FHIR API endpoints.
    • Enhanced testing capabilities with a JSON bundle for simulating healthcare-related data.
    • Improved Prometheus and Grafana configurations for enhanced monitoring infrastructure.
  • Documentation

    • Added detailed instructions for performance test setup and execution in the README.

Copy link
Contributor

coderabbitai bot commented Aug 12, 2024

Walkthrough

The recent changes introduce a comprehensive suite of K6 performance testing scripts and a new Grafana dashboard for monitoring application performance. Enhancements include various testing scenarios—smoke, volume, load, stress, and soak tests—alongside a script for generating healthcare data in JSON format. Additionally, detailed README files provide essential guidelines for conducting performance tests, ensuring users can thoroughly evaluate their systems under realistic conditions.

Changes

Files Change Summary
monitoring/grafana/dashboards/performance/k6.json New Grafana dashboard for k6 performance metrics, featuring panels for HTTP request rates, response times, and error rates, integrated with Prometheus.
test/performance/README.md New file offering performance testing guidelines with K6, detailing setup, various test types, and example commands for effective implementation.
test/performance/resources/ips-bundle-transaction.js, test/performance/resources/ips-bundle-transaction.json Introduces a script for generating a JSON bundle simulating healthcare data and a JSON configuration detailing various healthcare resources and their relationships.
test/performance/scripts/*.js Multiple new performance test scripts (load.js, smoke.js, volume.js), each simulating different load scenarios on a FHIR API endpoint.
monitoring/docker-compose.yml Updates to enhance Grafana and Prometheus setup, including a new dashboard and improved data ingestion capabilities for Prometheus.
documentation/guides/performance/README.md New file providing comprehensive guidance on performance testing using K6, including setup, execution, and monitoring of performance metrics.

Poem

In the meadow, swift and bright,
K6 hops with all its might.
Scripts to test and monitor,
Load and soak, we’ll explore!
With each request, we’ll see our way,
Performance blooms, come what may! 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@drizzentic drizzentic marked this pull request as ready for review August 13, 2024 09:55
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (7)
test/performance/scripts/soak.js (1)

5-5: Consider externalizing the default BASE_URL.

The default BASE_URL is hardcoded to http://localhost:5001. Consider externalizing this value to a configuration file or environment variable to improve flexibility and maintainability.

test/performance/scripts/load.js (1)

5-5: Consider externalizing the default BASE_URL.

The default BASE_URL is hardcoded to http://localhost:5001. Consider externalizing this value to a configuration file or environment variable to improve flexibility and maintainability.

test/performance/scripts/volume.js (1)

5-5: Consider externalizing the default BASE_URL.

The default BASE_URL is hardcoded to http://localhost:5001. Consider externalizing this value to a configuration file or environment variable to improve flexibility and maintainability.

test/performance/README.md (2)

25-25: Fix heading level increment.

The heading level should increment by one level at a time. Change this to an H3 heading.

-#### Volume test
+### Volume test
Tools
Markdownlint

25-25: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


17-17: Specify language for fenced code blocks.

Add the language identifier to the fenced code blocks for better syntax highlighting.

-```
+```bash

Also applies to: 29-29, 37-37, 45-45, 53-53

Tools
Markdownlint

17-17: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

test/performance/resources/ips-bundle-transaction.js (1)

7-10: Update the JSON file path in the code.

The JSON file ips-bundle-transaction.json exists in the repository at test/performance/resources/ips-bundle-transaction.json. Update the path in the code to reflect this location.

  • Update the path in the code snippet to test/performance/resources/ips-bundle-transaction.json.
Analysis chain

Ensure the JSON file path is correct.

The JSON file path ./ips-bundle-transaction.json should be verified to ensure it exists and is accessible.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence of the JSON file

if [ -f "./ips-bundle-transaction.json" ]; then
  echo "JSON file exists."
else
  echo "JSON file does not exist."
fi

Length of output: 103


Script:

#!/bin/bash
# Search for the JSON file in the entire repository

fd --type f --name "ips-bundle-transaction.json"

Length of output: 299


Script:

#!/bin/bash
# Corrected search for the JSON file in the entire repository

fd "ips-bundle-transaction.json" --type f

Length of output: 96

monitoring/grafana/dashboards/performance/k6.json (1)

1929-1940: Review dashboard instructions.

The text panel provides instructions for visualizing additional k6 results. Ensure that these instructions are clear and accurate, guiding users effectively on how to add new visualizations.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0882027 and a4879c4.

Files selected for processing (8)
  • monitoring/grafana/dashboards/performance/k6.json (1 hunks)
  • test/performance/README.md (1 hunks)
  • test/performance/resources/ips-bundle-transaction.js (1 hunks)
  • test/performance/resources/ips-bundle-transaction.json (1 hunks)
  • test/performance/scripts/load.js (1 hunks)
  • test/performance/scripts/soak.js (1 hunks)
  • test/performance/scripts/stress.js (1 hunks)
  • test/performance/scripts/volume.js (1 hunks)
Additional context used
Markdownlint
test/performance/README.md

25-25: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


17-17: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


29-29: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


37-37: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


45-45: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


53-53: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Additional comments not posted (18)
test/performance/scripts/soak.js (2)

7-23: Review the soak test configuration.

The soak test is configured with 50 virtual users for 24 hours. Ensure this configuration aligns with your testing goals and infrastructure capacity.


25-41: Ensure generateBundle produces valid data.

The generateBundle function is used to generate data for the POST request. Verify that it produces valid and expected data for the FHIR endpoint.

test/performance/scripts/load.js (2)

7-26: Review the load test configuration.

The load test is configured with a rate of 50 requests per second for 5 minutes. Ensure this configuration aligns with your testing goals and infrastructure capacity.


28-44: Ensure generateBundle produces valid data.

The generateBundle function is used to generate data for the POST request. Verify that it produces valid and expected data for the FHIR endpoint.

Verification successful

generateBundle Produces Valid FHIR Data

The generateBundle function is defined in test/performance/resources/ips-bundle-transaction.js. It constructs a JSON string by replacing placeholders with dynamically generated data using the faker library. This includes UUIDs, names, dates, and other identifiers, ensuring the data is randomized and unique for each call. The final JSON string is parsed and returned as a JavaScript object, which should be valid for the FHIR endpoint as it adheres to expected data structures.

  • File: test/performance/resources/ips-bundle-transaction.js
  • Lines: 12-356

The function appears to be correctly generating valid data for the FHIR endpoint. No further issues were found with the implementation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the output of `generateBundle` function.

# Test: Log the output of `generateBundle` to ensure it is valid.
ast-grep --lang javascript --pattern 'function generateBundle() { $$$ }' --json | jq '.[0].content' | node -e "console.log(eval(require('fs').readFileSync('/dev/stdin', 'utf8'))())"

Length of output: 273


Script:

#!/bin/bash
# Locate and display the implementation of the `generateBundle` function to verify its output.
ast-grep --lang javascript --pattern 'function generateBundle() { $$$ }'

Length of output: 74


Script:

#!/bin/bash
# Search for occurrences of `generateBundle` to find its definition or import.
rg 'generateBundle'

Length of output: 806


Script:

#!/bin/bash
# Display the implementation of `generateBundle` from its source file to verify its output.
ast-grep --lang javascript --pattern 'export const generateBundle = () => { $$$ }'

Length of output: 27475

test/performance/scripts/volume.js (2)

7-30: Review the volume test configuration.

The volume test is configured with a ramping arrival rate. Ensure this configuration aligns with your testing goals and infrastructure capacity.


32-48: Ensure generateBundle produces valid data.

The generateBundle function is used to generate data for the POST request. Verify that it produces valid and expected data for the FHIR endpoint.

Verification successful

The generateBundle function appears to produce valid data for FHIR testing.

The generateBundle function generates a JSON object by replacing placeholders with realistic data using the faker library. This includes UUIDs, names, dates, and other identifiers, which are essential for simulating FHIR bundle data. The function seems well-suited for generating test data for FHIR endpoints.

  • The function reads a JSON template from ips-bundle-transaction.json.
  • It uses faker to generate realistic data for various fields like UUIDs, names, and dates.
  • The placeholders in the JSON are replaced with these generated values using replacePlaceholder.

Given this setup, the function should produce valid and expected data for the FHIR endpoint.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the output of `generateBundle` function.

# Test: Log the output of `generateBundle` to ensure it is valid.
ast-grep --lang javascript --pattern 'function generateBundle() { $$$ }' --json | jq '.[0].content' | node -e "console.log(eval(require('fs').readFileSync('/dev/stdin', 'utf8'))())"

Length of output: 273


Script:

#!/bin/bash
# Description: Locate and display the implementation of `generateBundle` function.

# Locate the file containing `generateBundle` and display its implementation.
ast-grep --lang javascript --pattern 'function generateBundle() { $$$ }'

Length of output: 74


Script:

#!/bin/bash
# Description: Search for occurrences of `generateBundle` in the codebase.

# Use ripgrep to find any mention of `generateBundle`.
rg 'generateBundle'

Length of output: 806


Script:

#!/bin/bash
# Description: Display the implementation of `generateBundle` from the source file.

# Extract and display the implementation of `generateBundle`.
ast-grep --lang javascript --pattern 'export const generateBundle = () => { $$$ }' --json | jq '.[0].content'

Length of output: 116


Script:

#!/bin/bash
# Description: Display the content of the file containing `generateBundle`.

# Display the content of `ips-bundle-transaction.js`.
cat test/performance/resources/ips-bundle-transaction.js

Length of output: 8469

test/performance/scripts/stress.js (3)

8-32: Review K6 options configuration.

The K6 options are configured for a stress test scenario using ramping-arrival-rate. Ensure that the stages and thresholds are appropriate for the intended stress testing goals.


34-50: Ensure the generateBundle function is correctly implemented.

The generateBundle function is used to create data for the POST request. Verify that this function generates the expected data structure.

Verification successful

The generateBundle function is correctly implemented. The function generates a JSON object with various fields populated by dynamically generated values using the faker library, suitable for testing purposes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the output of the generateBundle function

ast-grep --lang javascript --pattern $'export const generateBundle = () => {
  $$$
  return JSON.parse(jsonString);
}'

Length of output: 27510


4-4: Consider making the BASE_URL configurable.

The BASE_URL is set to a default value of http://localhost:5001. Ensure that this environment variable is set correctly in different environments.

Verification successful

Ensure BASE_URL is set in deployment environments

The code already supports configuring BASE_URL via an environment variable, defaulting to http://localhost:5001 if not set. This aligns with the review comment's suggestion. Ensure that deployment environments set this variable appropriately to avoid using the default value unintentionally.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if BASE_URL is set in the environment

if [ -z "$BASE_URL" ]; then
  echo "BASE_URL is not set. Using default: http://localhost:5001"
else
  echo "BASE_URL is set to: $BASE_URL"
fi

Length of output: 140

test/performance/resources/ips-bundle-transaction.js (1)

358-361: Check the replacePlaceholder function.

The replacePlaceholder function uses a regular expression to replace placeholders in a JSON string. Ensure that the function works as intended and handles edge cases.

test/performance/resources/ips-bundle-transaction.json (5)

3-11: Ensure uniqueness of identifiers.

The id and identifier fields use placeholders like {{bundleUUID}} and {{bundleIdentifierUUID}}. Ensure that these values are unique across different bundles to prevent data conflicts.


35-47: Check for duplicate identifiers.

The identifier array contains multiple entries with similar types, such as PHN, PPN, DL, etc. Ensure that these identifiers are unique within the Patient resource to avoid data integrity issues.


175-176: Verify placeholder values.

Ensure that placeholders like {{patientGender}} and {{patientDOB}} are replaced with valid values during data generation, as these fields are critical for patient data accuracy.


373-422: Validate AllergyIntolerance resource.

Check that the AllergyIntolerance resource includes all necessary fields, such as clinicalStatus, verificationStatus, type, code, and patient. Ensure that placeholders are correctly replaced with valid data.


901-936: Ensure correct URL structure.

The fullUrl fields use a base URL like http://hapi-fhir:8080/fhir/. Confirm that this URL is correct and accessible in the testing environment to ensure successful data transactions.

monitoring/grafana/dashboards/performance/k6.json (3)

4-10: Verify Prometheus data source configuration.

Ensure that the Prometheus data source (DS_PROMETHEUS) is correctly configured and accessible from Grafana to avoid issues with data visualization.


266-329: Review PromQL expressions for accuracy.

The PromQL expressions used in the targets should be verified to ensure they accurately capture the intended metrics. For example, ensure that the expressions for k6_vus, k6_http_req_duration, and k6_http_reqs_total are correct and return the expected data.


542-709: Check panel configurations for completeness.

Ensure that all panels are configured with the necessary options, such as legend settings and tooltip modes, to provide clear and informative visualizations.

Copy link
Member

@rcrichton rcrichton left a comment

Choose a reason for hiding this comment

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

Looking good so far, some comments added.

Also, I think it would be a good idea to include some gitbook docs on how to run these both against a local env and a remote env.

test/performance/README.md Show resolved Hide resolved
test/performance/README.md Show resolved Hide resolved
@drizzentic
Copy link
Collaborator Author

@rcrichton I have addressed the feedback and hooked up the result output to grafana. This is the sample output of the dashboard
image

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (10)
documentation/packages/README.md (2)

16-16: Clarify dependency installation instructions.

Specify the exact dependencies required, especially for k6, to avoid confusion for users unfamiliar with the tool.

- Make sure you have the necessary dependencies installed.More importantly the `k6` binary.
+ Ensure all necessary dependencies are installed, particularly the `k6` binary. See [k6 installation guide](https://k6.io/docs/getting-started/installation/) for details.

22-22: Correct abbreviation punctuation.

The abbreviation "e.g." should have periods after each letter.

- where the scripts are located e.g [`load.js`]("/media/platform/test/performance/scripts/load.js")
+ where the scripts are located, e.g., [`load.js`]("/media/platform/test/performance/scripts/load.js")
Tools
LanguageTool

[uncategorized] ~22-~22: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...directory where the scripts are located e.g [load.js]("/media/platform/test/perfo...

(E_G)

test/performance/README.md (8)

21-21: Specify language for code blocks.

Add a language specifier to the code block for syntax highlighting.

- ```
+ ```bash
Tools
Markdownlint

21-21: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


33-33: Specify language for code blocks.

Add a language specifier to the code block for syntax highlighting.

- ```
+ ```bash
Tools
Markdownlint

33-33: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


41-41: Specify language for code blocks.

Add a language specifier to the code block for syntax highlighting.

- ```
+ ```bash
Tools
Markdownlint

41-41: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


49-49: Specify language for code blocks.

Add a language specifier to the code block for syntax highlighting.

- ```
+ ```bash
Tools
Markdownlint

49-49: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


57-57: Specify language for code blocks.

Add a language specifier to the code block for syntax highlighting.

- ```
+ ```bash
Tools
Markdownlint

57-57: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


67-67: Specify language for code blocks.

Add a language specifier to the code block for syntax highlighting.

- ```
+ ```bash
Tools
Markdownlint

67-67: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


63-63: Add a comma for clarity.

Insert a comma to improve readability.

- The example k6 binary comes with a prometheus remote write extension which allows you to specify the prometheus endpoint where the load testing results will be pushed to.
+ The example k6 binary comes with a prometheus remote write extension, which allows you to specify the prometheus endpoint where the load testing results will be pushed to.
Tools
LanguageTool

[uncategorized] ~63-~63: Possible missing comma found.
Context: ...ry comes with a prometheus remote write extension which allows you to specify the prometh...

(AI_HYDRA_LEO_MISSING_COMMA)


73-73: Add a comma for clarity.

Insert a comma to improve readability.

- For more environment variables follow this link [here](https://k6.io/docs/results-output/real-time/prometheus-remote-write/#time-series-visualization)
+ For more environment variables, follow this link [here](https://k6.io/docs/results-output/real-time/prometheus-remote-write/#time-series-visualization)
Tools
LanguageTool

[uncategorized] ~73-~73: Possible missing comma found.
Context: ...pts/volume.js ``` For more environment variables follow this link [here](https://k6.io/d...

(AI_HYDRA_LEO_MISSING_COMMA)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a4879c4 and ec4d149.

Files selected for processing (3)
  • documentation/packages/README.md (1 hunks)
  • monitoring/docker-compose.yml (6 hunks)
  • test/performance/README.md (1 hunks)
Additional context used
LanguageTool
documentation/packages/README.md

[uncategorized] ~22-~22: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...directory where the scripts are located e.g [load.js]("/media/platform/test/perfo...

(E_G)

test/performance/README.md

[uncategorized] ~63-~63: Possible missing comma found.
Context: ...ry comes with a prometheus remote write extension which allows you to specify the prometh...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~73-~73: Possible missing comma found.
Context: ...pts/volume.js ``` For more environment variables follow this link [here](https://k6.io/d...

(AI_HYDRA_LEO_MISSING_COMMA)

Markdownlint
test/performance/README.md

29-29: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


21-21: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


33-33: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


41-41: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


49-49: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


57-57: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


67-67: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Additional comments not posted (3)
monitoring/docker-compose.yml (3)

160-160: Formatting improvement in healthcheck command.

The formatting change improves readability but does not affect functionality.


69-70: Ensure correct dashboard configuration.

Verify that the k6.json dashboard file exists and is correctly configured in Grafana.

Verification successful

Dashboard File Exists and Is Correctly Referenced

The k6.json dashboard file exists at the expected location in the repository, confirming that the configuration in docker-compose.yml is correct. No further action is needed.

  • File location: monitoring/grafana/dashboards/performance/k6.json
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence and configuration of the k6.json dashboard file.

# Test: Check if the k6.json file exists in the specified directory.
fd 'k6.json' ./grafana/dashboards/performance

Length of output: 165


Script:

#!/bin/bash
# Search for the k6.json file throughout the repository to verify its existence and location.
fd 'k6.json'

Length of output: 62


94-94: Verify Prometheus remote write configuration.

Ensure that the --web.enable-remote-write-receiver option is correctly configured and supported by the Prometheus version in use.

Copy link
Member

@rcrichton rcrichton left a comment

Choose a reason for hiding this comment

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

Thanks, some more comments.

test/performance/README.md Outdated Show resolved Hide resolved
@@ -8,3 +8,25 @@ description: >-
# 📦 Packages

Package can be stood up individually using the `instant package init -n <package_name>` command, or they can be included in your own recipes. This can be accomplished by [creating a profile](https://app.gitbook.com/s/TwrbQZir3ZdvejunAFia/getting-started/config#launching-a-profile) that includes the necessary packages and any custom configuration packages.

## Performance Testing
Copy link
Member

Choose a reason for hiding this comment

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

I think this might better fit under guides rather.

Copy link
Member

Choose a reason for hiding this comment

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

In addition I'd like you to include the results of a run to give an indication of the current capabilities of platform. This should include the system tested on and the results achieved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we have a guides section? Also should we provide only sample results for a specific performance test?

Copy link
Member

Choose a reason for hiding this comment

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

I think we need both. A guide on how to reproduce and the an example run using specific hardware to give people an idea of the current performance. So both.

The guides section is here: https://github.com/jembi/platform/tree/main/documentation/guides

Login to the gitbook app to get a better idea of how the docs are structured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have made re-arrangements and added the test results in the guide. @rcrichton

test/performance/scripts/load.js Outdated Show resolved Hide resolved
documentation/packages/README.md Outdated Show resolved Hide resolved
test/performance/README.md Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ec4d149 and 062c1bf.

Files selected for processing (5)
  • documentation/packages/README.md (1 hunks)
  • test/performance/README.md (1 hunks)
  • test/performance/scripts/load.js (1 hunks)
  • test/performance/scripts/smoke.js (1 hunks)
  • test/performance/scripts/volume.js (1 hunks)
Additional context used
LanguageTool
documentation/packages/README.md

[uncategorized] ~16-~16: A comma might be missing here.
Context: ...e necessary dependencies installed.More importantly the k6 binary. 2. Set the [`BASE_URL...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[uncategorized] ~22-~22: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...directory where the scripts are located e.g [load.js]("/media/platform/test/perfo...

(E_G)

test/performance/README.md

[grammar] ~31-~31: The verb form ‘help’ does not appear to fit in this context.
Context: ...n example guide. #### Smoke Test This test help to verify that the system works well un...

(SINGULAR_NOUN_VERB_AGREEMENT)


[uncategorized] ~73-~73: A comma might be missing here.
Context: ... results will be pushed to. To achieve this ensure the following variable is set co...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[uncategorized] ~81-~81: Possible missing comma found.
Context: ...pts/volume.js ``` For more environment variables follow this link [here](https://k6.io/d...

(AI_HYDRA_LEO_MISSING_COMMA)

Markdownlint
test/performance/README.md

29-29: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


21-21: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


33-33: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


41-41: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


49-49: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


57-57: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


65-65: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


75-75: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Additional comments not posted (10)
test/performance/scripts/load.js (3)

1-6: Imports and constants are well-defined.

The use of standard imports and configurable constants is appropriate for flexible performance testing environments.


7-26: Load testing configuration is appropriate.

The scenario and threshold settings in the options object are well-suited for load testing. Ensure these settings align with the expected performance metrics of the OpenHIM platform.

Consider verifying these settings with actual performance data to ensure they are realistic and achievable.


46-48: Default export function is straightforward and correct.

The function correctly encapsulates the operation to be tested, maintaining simplicity and focus in the test script.

test/performance/scripts/smoke.js (4)

1-5: Imports and constants are correctly defined.

The import statements and the BASE_URL constant are appropriately set up for a K6 script.


7-26: Options configuration is well-defined for smoke testing.

The scenarios and thresholds are appropriately configured to ensure the test is meaningful and aligns with performance expectations.


28-46: Function makePostRequest is correctly implemented.

The function properly constructs the request with appropriate headers and checks the response effectively.


48-50: Default function is correctly set up.

The default exported function properly invokes makePostRequest to perform the test.

test/performance/scripts/volume.js (3)

1-5: Imports and constants are correctly defined.

The import statements and the BASE_URL constant are appropriately set up for a K6 script.


7-30: Options configuration is well-defined for volume testing.

The scenarios and thresholds are appropriately configured to ensure the test is meaningful and aligns with performance expectations.


50-52: Default function is correctly set up.

The default exported function properly invokes makeGetRequest to perform the test.

test/performance/scripts/load.js Show resolved Hide resolved
documentation/packages/README.md Outdated Show resolved Hide resolved
documentation/packages/README.md Outdated Show resolved Hide resolved
test/performance/README.md Show resolved Hide resolved
test/performance/README.md Outdated Show resolved Hide resolved
test/performance/README.md Outdated Show resolved Hide resolved
test/performance/README.md Outdated Show resolved Hide resolved
test/performance/scripts/volume.js Outdated Show resolved Hide resolved
drizzentic and others added 7 commits August 28, 2024 13:25
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 062c1bf and 65cb46e.

Files selected for processing (3)
  • documentation/guides/performance/README.md (1 hunks)
  • test/performance/README.md (1 hunks)
  • test/performance/scripts/volume.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • test/performance/scripts/volume.js
Additional context used
Markdownlint
test/performance/README.md

29-29: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


21-21: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


33-33: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


41-41: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


49-49: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


57-57: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


65-65: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


75-75: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

LanguageTool
documentation/guides/performance/README.md

[uncategorized] ~13-~13: Possible missing comma found.
Context: ... to the directory where the scripts are located e.g. [load.js]("/media/platform/test/...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~23-~23: Possible missing comma found.
Context: ...ccurred. 9. To visualize the output in grafana run the k6 scripts with the following...

(AI_HYDRA_LEO_MISSING_COMMA)

Additional comments not posted (11)
test/performance/README.md (9)

19-19: Move binary build detail to a different section.

The binary build detail should be moved to a different section, as suggested in the previous review.


17-23: LGTM!

The instructions for building a k6 binary are clear and well-detailed.

The code changes are approved.

Tools
Markdownlint

21-21: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


29-29: Specify language for code blocks.

To adhere to markdown best practices, specify the language for all fenced code blocks.

Apply this diff to specify the language for the code blocks:

- ```
+ ```bash

Also applies to: 33-33, 41-41, 49-49, 57-57, 65-65, 75-75

Tools
Markdownlint

29-29: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


25-25: Include a smoke test.

Include a smoke test which runs just 1 transaction and ensures it succeeds. Suggest in the docs that this is always run first as some services need to be warmed up before they perform well.


75-75: Specify language for code blocks.

To adhere to markdown best practices, specify the language for all fenced code blocks.

Apply this diff to specify the language for the code blocks:

- ```
+ ```bash
Tools
Markdownlint

75-75: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


73-73: Add a comma for clarity.

Consider adding a comma after "this" to improve the readability of the sentence.

Apply this diff to fix the punctuation issue:

- To achieve this ensure the following variable is set correctly and the output flag enabled
+ To achieve this, ensure the following variable is set correctly and the output flag enabled

81-81: Add a comma for clarity.

Consider adding a comma after "variables" to improve the readability of the sentence.

Apply this diff to fix the punctuation issue:

- For more environment variables follow this link [here](https://k6.io/docs/results-output/real-time/prometheus-remote-write/#time-series-visualization)
+ For more environment variables, follow this link [here](https://k6.io/docs/results-output/real-time/prometheus-remote-write/#time-series-visualization)

25-47: LGTM!

The sample load test results are clear and provide useful information.

The code changes are approved.

Tools
Markdownlint

29-29: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


33-33: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


41-41: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


49-69: LGTM!

The sample volume test results are clear and provide useful information.

The code changes are approved.

Tools
Markdownlint

49-49: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


57-57: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


65-65: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

documentation/guides/performance/README.md (2)

25-47: LGTM!

The sample load test results are clear and provide useful information.

The code changes are approved.


49-69: LGTM!

The sample volume test results are clear and provide useful information.

The code changes are approved.

documentation/guides/performance/README.md Outdated Show resolved Hide resolved
documentation/guides/performance/README.md Outdated Show resolved Hide resolved
drizzentic and others added 2 commits August 30, 2024 10:05
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 65cb46e and f341baf.

Files selected for processing (1)
  • documentation/guides/performance/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • documentation/guides/performance/README.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants