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

feat: Added ability to load spending plans from environment variable. #3153

Merged
merged 25 commits into from
Oct 31, 2024

Conversation

ebadiere
Copy link
Contributor

This enhancement adds the ability to load HBar Rate Limiter spending plans from an environment variable.
Related issue(s):

Fixes #3152

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@ebadiere ebadiere added the enhancement New feature or request label Oct 23, 2024
@ebadiere ebadiere added this to the 0.59.0 milestone Oct 23, 2024
@ebadiere ebadiere self-assigned this Oct 23, 2024
Copy link

github-actions bot commented Oct 23, 2024

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled validates enforcement of request id. This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: MarkSweepCompact
Cost: 30,459.7 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.46 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: decreased with 348.16 KB
  • Total Available Size: decreased with 7.63 MB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 64.00 bytes
  • Used Heap Size: decreased with 3.44 MB
  • Heap Size Limit: no changes
  • Malloced Memory: no changes
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • Old Space:

    • Space Size: increased with 1.84 MB
    • Space Used Size: increased with 2.07 MB
    • Space Available Size: decreased with 10.39 MB
    • Physical Space Size: increased with 1.84 MB
  • Large Object Space:

    • Space Size: increased with 835.58 KB
    • Space Used Size: increased with 813.50 KB
    • Space Available Size: no changes
    • Physical Space Size: increased with 835.58 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

Copy link

github-actions bot commented Oct 23, 2024

Test Results

 20 files   -   3  265 suites   - 23   33m 6s ⏱️ - 3m 31s
605 tests  -   3  596 ✅ +  3  4 💤 ±0  5 ❌  - 6 
663 runs   - 126  654 ✅  - 118  4 💤  - 2  5 ❌  - 6 

For more details on these failures, see this check.

Results for commit c48d26f. ± Comparison against base commit 6606c38.

This pull request removes 4 and adds 1 tests. Note that renamed tests count towards both.
"before all" hook for "@release should execute "eth_getTransactionCount" primary" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_getTransactionCount "before all" hook for "@release should execute "eth_getTransactionCount" primary"
"before all" hook for "Function calling HederaTokenService.isToken(token)" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call with contract that calls precompiles "before all" hook for "Function calling HederaTokenService.isToken(token)"
"before all" hook in "Debug API Test Suite" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite "before all" hook in "Debug API Test Suite"
"before all" hook in "RPC Server Acceptance Tests" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-1 RPC Server Acceptance Tests RPC Server Acceptance Tests "before all" hook in "RPC Server Acceptance Tests"
"before all" hook in "@precompile-calls Tests for eth_call with HTS" ‑ RPC Server Acceptance Tests Acceptance tests @precompile-calls Tests for eth_call with HTS "before all" hook in "@precompile-calls Tests for eth_call with HTS"

♻️ This comment has been updated with latest results.

@ebadiere ebadiere changed the title feat: Added ability to load spening plans from environment variable. feat: Added ability to load spending plans from environment variable. Oct 23, 2024
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Config suggestion, other than that looks good.

docs/configuration.md Outdated Show resolved Hide resolved
Copy link
Contributor

@victor-yanev victor-yanev left a comment

Choose a reason for hiding this comment

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

Looks good, just a few suggestions:

Nana-EC
Nana-EC previously approved these changes Oct 24, 2024
Copy link
Collaborator

@Nana-EC Nana-EC 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, just a non-blocking clean up suggestion on the logic.

…tent.

Signed-off-by: ebadiere <[email protected]>

fix: removed .only

Signed-off-by: ebadiere <[email protected]>

fix: Cleaned up file and env var evaluation.

Signed-off-by: ebadiere <[email protected]>

fix: Flaky unit test fix.

Signed-off-by: ebadiere <[email protected]>

feat: Refactored implementation and updated tests.

Signed-off-by: ebadiere <[email protected]>
Copy link

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled should execute "eth_accounts". This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: MarkSweepCompact
Cost: 25,192.6 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.05 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 274.43 KB
  • Total Available Size: decreased with 1.48 MB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 96.00 bytes
  • Used Heap Size: decreased with 946.81 KB
  • Heap Size Limit: no changes
  • Malloced Memory: no changes
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • New Space:
    • Space Size: increased with 1.05 MB
    • Space Used Size: decreased with 750.30 KB
    • Space Available Size: increased with 750.30 KB
    • Physical Space Size: increased with 274.43 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

Copy link

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled should execute "eth_getTransactionByHash with missing transaction". This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: Scavenge
Cost: 3,870.55 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 262.14 KB
  • Total Heap Size Executable: no changes
  • Total Physical Size: decreased with 253.95 KB
  • Total Available Size: increased with 267.24 KB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: no changes
  • Used Heap Size: decreased with 273.39 KB
  • Heap Size Limit: no changes
  • Malloced Memory: no changes
  • External Memory: decreased with 165.00 bytes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • Old Space:
    • Space Size: increased with 262.14 KB
    • Space Used Size: increased with 335.87 KB
    • Space Available Size: decreased with 79.88 KB
    • Physical Space Size: increased with 262.14 KB

GC Type: Scavenge
Cost: 2,468.03 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.31 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 794.62 KB
  • Total Available Size: decreased with 799.01 KB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: no changes
  • Used Heap Size: decreased with 255.66 KB
  • Heap Size Limit: no changes
  • Malloced Memory: increased with 32.78 KB
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • New Space:

    • Space Size: increased with 1.05 MB
    • Space Used Size: decreased with 591.07 KB
    • Space Available Size: increased with 591.07 KB
    • Physical Space Size: increased with 532.48 KB
  • Old Space:

    • Space Size: increased with 262.14 KB
    • Space Used Size: increased with 335.41 KB
    • Space Available Size: decreased with 79.36 KB
    • Physical Space Size: increased with 262.14 KB

GC Type: MarkSweepCompact
Cost: 34,529.5 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.05 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 274.43 KB
  • Total Available Size: decreased with 1.41 MB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 96.00 bytes
  • Used Heap Size: decreased with 1.30 MB
  • Heap Size Limit: no changes
  • Malloced Memory: no changes
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • New Space:
    • Space Size: increased with 1.05 MB
    • Space Used Size: decreased with 424.45 KB
    • Space Available Size: increased with 424.45 KB
    • Physical Space Size: increased with 274.43 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

Copy link

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled should execute "net_peerCount". This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: MarkSweepCompact
Cost: 30,671.3 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.05 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 274.43 KB
  • Total Available Size: decreased with 1.86 MB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 64.00 bytes
  • Used Heap Size: decreased with 920.43 KB
  • Heap Size Limit: no changes
  • Malloced Memory: increased with 40.98 KB
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • New Space:
    • Space Size: increased with 1.05 MB
    • Space Used Size: decreased with 576.59 KB
    • Space Available Size: increased with 576.59 KB
    • Physical Space Size: increased with 274.43 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

Copy link

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled should execute "eth_sign". This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: MarkSweepCompact
Cost: 37,788.9 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 2.10 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 32.77 KB
  • Total Available Size: decreased with 128.46 KB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 64.00 bytes
  • Used Heap Size: decreased with 947.09 KB
  • Heap Size Limit: no changes
  • Malloced Memory: increased with 50.41 KB
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • New Space:
    • Space Size: increased with 2.10 MB
    • Space Used Size: decreased with 579.02 KB
    • Space Available Size: increased with 1.61 MB
    • Physical Space Size: increased with 32.77 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

Copy link

🚨 Memory Leak Detected 🚨

A potential memory leak has been detected in the test titled should hit batch request limit. This may impact the application's performance and stability.

Details

📊 Memory Leak Detection Report 📊

GC Type: MarkSweepCompact
Cost: 36,867.5 ms

Heap Statistics (before vs after executing the test):

  • Total Heap Size: increased with 1.05 MB
  • Total Heap Size Executable: no changes
  • Total Physical Size: increased with 274.43 KB
  • Total Available Size: decreased with 425.51 KB
  • Total Global Handles Size: no changes
  • Used Global Handles Size: decreased with 64.00 bytes
  • Used Heap Size: decreased with 633.02 KB
  • Heap Size Limit: no changes
  • Malloced Memory: increased with 106.53 KB
  • External Memory: no changes
  • Peak Malloced Memory: no changes

Heap Space Statistics (before vs after executing the test):

  • New Space:
    • Space Size: increased with 1.05 MB
    • Space Used Size: decreased with 470.91 KB
    • Space Available Size: increased with 470.91 KB
    • Physical Space Size: increased with 274.43 KB

Recommendations

Please investigate the memory allocations in this test, focusing on objects that are not being properly deallocated.

Copy link
Contributor

@victor-yanev victor-yanev left a comment

Choose a reason for hiding this comment

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

LGTM, one last thing

@ebadiere ebadiere modified the milestones: 0.59.0, 0.60.0 Oct 30, 2024
quiet-node and others added 2 commits October 30, 2024 12:02
…erent-batches' into 3152-spendingplan-as-env-var

Signed-off-by: ebadiere <[email protected]>
ebadiere and others added 6 commits October 30, 2024 18:09
* chore: divided hbar limtier tests into different batches

Signed-off-by: Logan Nguyen <[email protected]>

* fix: fixed acceptance.yml

Signed-off-by: Logan Nguyen <[email protected]>

---------

Signed-off-by: Logan Nguyen <[email protected]>
Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
tomzhenghedera
tomzhenghedera previously approved these changes Oct 31, 2024
Copy link

sonarcloud bot commented Oct 31, 2024

@ebadiere ebadiere merged commit f189801 into main Oct 31, 2024
44 of 45 checks passed
@ebadiere ebadiere deleted the 3152-spendingplan-as-env-var branch October 31, 2024 20:17
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.35%. Comparing base (6606c38) to head (c48d26f).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...ay/src/lib/config/hbarSpendingPlanConfigService.ts 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3153      +/-   ##
==========================================
- Coverage   83.39%   83.35%   -0.05%     
==========================================
  Files          66       66              
  Lines        4283     4290       +7     
  Branches      835      837       +2     
==========================================
+ Hits         3572     3576       +4     
- Misses        471      473       +2     
- Partials      240      241       +1     
Flag Coverage Δ
config-service 98.14% <ø> (ø)
relay 85.54% <85.71%> (-0.06%) ⬇️
server 83.52% <ø> (ø)
ws-server 36.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ckages/config-service/src/services/globalConfig.ts 100.00% <ø> (ø)
...kages/relay/src/lib/clients/cache/localLRUCache.ts 93.84% <100.00%> (+0.09%) ⬆️
...ay/src/lib/config/hbarSpendingPlanConfigService.ts 95.49% <84.61%> (-2.60%) ⬇️

ebadiere added a commit that referenced this pull request Oct 31, 2024
…#3153)

* feat: Added ability to load spening plans from environment variable.

Signed-off-by: ebadiere <[email protected]>

* feat: Now uses one property for either spending plan file or JSON content.

Signed-off-by: ebadiere <[email protected]>

fix: removed .only

Signed-off-by: ebadiere <[email protected]>

fix: Cleaned up file and env var evaluation.

Signed-off-by: ebadiere <[email protected]>

fix: Flaky unit test fix.

Signed-off-by: ebadiere <[email protected]>

feat: Refactored implementation and updated tests.

Signed-off-by: ebadiere <[email protected]>

* fix: Adding updated package-lock.json

Signed-off-by: ebadiere <[email protected]>

* fix: Updated global variable reference to the new HBAR_SPENDING_PLANS_CONFIG

Signed-off-by: ebadiere <[email protected]>

fix: Test fix.

Signed-off-by: ebadiere <[email protected]>

fix: Removed irrelevant test since we now use either env var or file for spending plan.

Signed-off-by: ebadiere <[email protected]>

fix: Updated HBAR_SPENDING_PLANS_CONFIG from HBAR_SPENDING_PLANS_CONFIG_FILE

Signed-off-by: ebadiere <[email protected]>

* fix: Clean up. Updated logging and appropriate tests.

Signed-off-by: ebadiere <[email protected]>

* fix: Replaced the useInMemoryRedisServer with the start and stop redis in the
before and after mocha functions.

Signed-off-by: ebadiere <[email protected]>

* fix: Test fix.  Added the envName back to the loggerService test.

Signed-off-by: ebadiere <[email protected]>

* Update docs/configuration.md

Co-authored-by: Victor Yanev <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>

* fix: Added back file not found tests.

Signed-off-by: ebadiere <[email protected]>

* fix: Updated file name in `withOverriddenEnvsInMochaTest` with existing file.

Signed-off-by: ebadiere <[email protected]>

* fix: Clear the spending plan repository in a test as in CI it seems to already
be populated.

Signed-off-by: ebadiere <[email protected]>

fix: Cleanup.

Signed-off-by: ebadiere <[email protected]>

* fix: Added more time for the HBar Rate Limiter to update expenses in the background.

Signed-off-by: ebadiere <[email protected]>

* chore: divided hbar limtier tests into different batches

Signed-off-by: Logan Nguyen <[email protected]>

* fix: Removed the clearing of the spending plans.

Signed-off-by: ebadiere <[email protected]>

* chore: divided hbar limtier tests into different batches (#3181)

* chore: divided hbar limtier tests into different batches

Signed-off-by: Logan Nguyen <[email protected]>

* fix: fixed acceptance.yml

Signed-off-by: Logan Nguyen <[email protected]>

---------

Signed-off-by: Logan Nguyen <[email protected]>

* Update packages/relay/src/lib/config/hbarSpendingPlanConfigService.ts

Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>

* fix: Restored class comments and corrected HBAR_SPENDING_PLAN_CONFIG.

Signed-off-by: ebadiere <[email protected]>

* test: fix hbarSpendingPlanConfigService.spec.ts

Signed-off-by: Victor Yanev <[email protected]>

* test: remove `.only` from `describe`

Signed-off-by: Victor Yanev <[email protected]>

* test: disconnect redis client after tests with shared cache

Signed-off-by: Victor Yanev <[email protected]>

---------

Signed-off-by: ebadiere <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
Co-authored-by: Victor Yanev <[email protected]>
Co-authored-by: Logan Nguyen <[email protected]>
Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Co-authored-by: Victor Yanev <[email protected]>
ebadiere added a commit that referenced this pull request Oct 31, 2024
#3201)

feat: Added ability to load spending plans from environment variable. (#3153)

* feat: Added ability to load spening plans from environment variable.



* feat: Now uses one property for either spending plan file or JSON content.



fix: removed .only



fix: Cleaned up file and env var evaluation.



fix: Flaky unit test fix.



feat: Refactored implementation and updated tests.



* fix: Adding updated package-lock.json



* fix: Updated global variable reference to the new HBAR_SPENDING_PLANS_CONFIG



fix: Test fix.



fix: Removed irrelevant test since we now use either env var or file for spending plan.



fix: Updated HBAR_SPENDING_PLANS_CONFIG from HBAR_SPENDING_PLANS_CONFIG_FILE



* fix: Clean up. Updated logging and appropriate tests.



* fix: Replaced the useInMemoryRedisServer with the start and stop redis in the
before and after mocha functions.



* fix: Test fix.  Added the envName back to the loggerService test.



* Update docs/configuration.md




* fix: Added back file not found tests.



* fix: Updated file name in `withOverriddenEnvsInMochaTest` with existing file.



* fix: Clear the spending plan repository in a test as in CI it seems to already
be populated.



fix: Cleanup.



* fix: Added more time for the HBar Rate Limiter to update expenses in the background.



* chore: divided hbar limtier tests into different batches



* fix: Removed the clearing of the spending plans.



* chore: divided hbar limtier tests into different batches (#3181)

* chore: divided hbar limtier tests into different batches



* fix: fixed acceptance.yml



---------



* Update packages/relay/src/lib/config/hbarSpendingPlanConfigService.ts




* fix: Restored class comments and corrected HBAR_SPENDING_PLAN_CONFIG.



* test: fix hbarSpendingPlanConfigService.spec.ts



* test: remove `.only` from `describe`



* test: disconnect redis client after tests with shared cache



---------

Signed-off-by: ebadiere <[email protected]>
Signed-off-by: Eric Badiere <[email protected]>
Signed-off-by: Logan Nguyen <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
Co-authored-by: Victor Yanev <[email protected]>
Co-authored-by: Logan Nguyen <[email protected]>
Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Co-authored-by: Victor Yanev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add the ability for spending plans to be defined using environment variables.
6 participants