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

[Manual][Backport 2.x] Improves connection pooling support for AWSSigV4 clients in data sources (#6135) #6176

Conversation

bandinib-amzn
Copy link
Member

@bandinib-amzn bandinib-amzn commented Mar 18, 2024

Overview

This is cherry pick of 1999a23 from #6135

Testing

Testing with Node 14.

  1. Create data source with Username and password authentication type.
Screenshot 2024-03-18 at 2 53 02 PM
  1. Create data source with AWS SigV4 authentication type.
Screenshot 2024-03-18 at 3 38 13 PM

After creating data sources, following scenarios tested:

  • Create index pattern
  • Dev tool
  • Discover
  • Add sample data
  • Visualizations
  • Dashboards

Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 77.68595% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 67.17%. Comparing base (98dfac8) to head (5edb124).
Report is 1 commits behind head on 2.x.

Files Patch % Lines
...data_source/server/legacy/http_aws_es/connector.js 64.40% 21 Missing ⚠️
...r/auth_registry/authentication_methods_registry.ts 33.33% 1 Missing and 1 partial ⚠️
...ins/data_source/server/util/credential_provider.ts 0.00% 2 Missing ⚠️
...gins/data_source/server/client/configure_client.ts 95.23% 0 Missing and 1 partial ⚠️
...ta_source/server/legacy/configure_legacy_client.ts 95.83% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              2.x    #6176   +/-   ##
=======================================
  Coverage   67.17%   67.17%           
=======================================
  Files        3330     3331    +1     
  Lines       64313    64387   +74     
  Branches    10273    10281    +8     
=======================================
+ Hits        43204    43254   +50     
- Misses      18608    18636   +28     
+ Partials     2501     2497    -4     
Flag Coverage Δ
Linux_1 35.07% <ø> (ø)
Linux_2 55.28% <ø> (ø)
Linux_3 44.71% <77.68%> (+0.10%) ⬆️
Linux_4 35.35% <ø> (ø)
Windows_1 35.10% <ø> (-0.04%) ⬇️
Windows_2 55.24% <ø> (ø)
Windows_3 44.71% <77.68%> (+0.08%) ⬆️
Windows_4 35.35% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bandinib-amzn
Copy link
Member Author

Before rebase all CIs have been passed except WhiteSource Security check (which is expected).

@bandinib-amzn bandinib-amzn force-pushed the backport/backport-6135-to-2.x branch from 5474104 to f363300 Compare March 18, 2024 21:13
@bandinib-amzn
Copy link
Member Author

As per @AMoo-Miki, did testing for Node 14. Working as expected. Adding more details in PR Overview.

…rces. (opensearch-project#6135)

* Upgrade @opensearch/opensearch@2.6.0 which inherits AWSSigv4 to .child

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

* Uses client.child for aws sigv4 connection

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

* Import http-aws-es connector class

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

* Refactor client caching mechanism

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

* Fix UT

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

* Added UT for client pool

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

* Revert client pool changes from authentication method

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>

---------

Signed-off-by: Bandini Bhopi <bandinib@amazon.com>
(cherry picked from commit 09f5563)
@bandinib-amzn bandinib-amzn force-pushed the backport/backport-6135-to-2.x branch from f363300 to 5edb124 Compare March 18, 2024 22:51
@ZilongX
Copy link
Collaborator

ZilongX commented Mar 18, 2024

bootstrap somehow failed on ciGroup1, maybe wait for retry

@bandinib-amzn
Copy link
Member Author

All checks are successful except WhiteSource Security checks (which is expected).

@bandinib-amzn bandinib-amzn merged commit ad5370e into opensearch-project:2.x Mar 19, 2024
65 of 66 checks passed
@bandinib-amzn bandinib-amzn deleted the backport/backport-6135-to-2.x branch March 19, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants