Skip to content

Conversation

@finnegancarroll
Copy link
Contributor

@finnegancarroll finnegancarroll commented Jun 5, 2025

Description

Adds a few changes with the goal of making AuxTransport settings more easily configured/extended.

  • Moves AuxTransport out of NetworkPlugin into org.opensearch.transport.AuxTransport.
    Minor refactor. All other transport base classes are in this package and AuxTransport a bit out of place in NetworkPlugin.
  • Adds a settingKey accessor to AuxTransport.
    Forward looking change for plugins which want to identify the namespace of AuxTransports.
    (TLS support for auxiliary transports security#5375)
  • Bugfix for FlightStreamPlugin which registers the FlightService AuxTransport under key aux.transport.types.
    Moved this to experimental-transport-arrow-flight-rpc which users will need to include under aux.transport.types setting to enable this transport.

Related Issues

N/A

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Finn Carroll <carrofin@amazon.com>
Plugins integrating with AuxTransports need a way to identify transport types
with a unique name. Specifically thinking in terms of security plugin
where security settings for a transport will be configured under a namespace
matching that transport. However since security plugin does have an
authoritative list of aux transports it needs a way to fetch this "namespace
key" from an AuxTransport isntance.

Signed-off-by: Finn Carroll <carrofin@amazon.com>
Signed-off-by: Finn Carroll <carrofin@amazon.com>
Signed-off-by: Finn Carroll <carrofin@amazon.com>
Signed-off-by: Finn Carroll <carrofin@amazon.com>
@finnegancarroll finnegancarroll marked this pull request as ready for review June 5, 2025 21:44
@finnegancarroll finnegancarroll requested review from a team and peternied as code owners June 5, 2025 21:44
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

❌ Gradle check result for ca46db5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Finn Carroll <carrofin@amazon.com>
@finnegancarroll
Copy link
Contributor Author

@rishabhmaurya can you take a look when you have a chance?
Trying to standardize identifiers for registering aux transports a little to give better visibility to other plugins.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

✅ Gradle check result for 61c996c: SUCCESS

@codecov
Copy link

codecov bot commented Jun 5, 2025

Codecov Report

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

Project coverage is 73.07%. Comparing base (bee0749) to head (7641533).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
...a/org/opensearch/common/network/NetworkModule.java 57.14% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #18452      +/-   ##
============================================
+ Coverage     72.74%   73.07%   +0.33%     
- Complexity    67864    68162     +298     
============================================
  Files          5507     5508       +1     
  Lines        312253   312256       +3     
  Branches      45348    45348              
============================================
+ Hits         227137   228190    +1053     
+ Misses        66618    65766     -852     
+ Partials      18498    18300     -198     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Performance Roadmap Jun 5, 2025
@rishabhmaurya
Copy link
Contributor

@finnegancarroll Thank you, LGTM! please check the codecov report.

Signed-off-by: Finn Carroll <carrofin@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2025

✅ Gradle check result for 7641533: SUCCESS

@github-project-automation github-project-automation bot moved this from In Progress to Done in Performance Roadmap Jun 8, 2025
@github-project-automation github-project-automation bot moved this from Done to In Progress in Performance Roadmap Jun 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2025

✅ Gradle check result for 7641533: SUCCESS

@rishabhmaurya rishabhmaurya merged commit 3d58b8b into opensearch-project:main Jun 9, 2025
49 of 62 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Performance Roadmap Jun 9, 2025
neuenfeldttj pushed a commit to neuenfeldttj/OpenSearch that referenced this pull request Jun 26, 2025
* Refactor AuxTransport to org.opensearch.transport.AuxTransport.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* Add enableKey identifier to AuxTransports.

Plugins integrating with AuxTransports need a way to identify transport types
with a unique name. Specifically thinking in terms of security plugin
where security settings for a transport will be configured under a namespace
matching that transport. However since security plugin does have an
authoritative list of aux transports it needs a way to fetch this "namespace
key" from an AuxTransport isntance.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* enableKey -> settingKey. Rename for clarity.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* ARROW_FLIGHT_TRANSPORT_SETTING_KEY to private. Javadocs.Signed-off-by: TJ Neuenfeldt <tjneu@amazon.com>
neuenfeldttj pushed a commit to neuenfeldttj/OpenSearch that referenced this pull request Jun 26, 2025
* Refactor AuxTransport to org.opensearch.transport.AuxTransport.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* Add enableKey identifier to AuxTransports.

Plugins integrating with AuxTransports need a way to identify transport types
with a unique name. Specifically thinking in terms of security plugin
where security settings for a transport will be configured under a namespace
matching that transport. However since security plugin does have an
authoritative list of aux transports it needs a way to fetch this "namespace
key" from an AuxTransport isntance.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* enableKey -> settingKey. Rename for clarity.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* ARROW_FLIGHT_TRANSPORT_SETTING_KEY to private. Javadocs.
tandonks pushed a commit to tandonks/OpenSearch that referenced this pull request Aug 5, 2025
* Refactor AuxTransport to org.opensearch.transport.AuxTransport.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* Add enableKey identifier to AuxTransports.

Plugins integrating with AuxTransports need a way to identify transport types
with a unique name. Specifically thinking in terms of security plugin
where security settings for a transport will be configured under a namespace
matching that transport. However since security plugin does have an
authoritative list of aux transports it needs a way to fetch this "namespace
key" from an AuxTransport isntance.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* enableKey -> settingKey. Rename for clarity.

Signed-off-by: Finn Carroll <carrofin@amazon.com>

* ARROW_FLIGHT_TRANSPORT_SETTING_KEY to private. Javadocs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done
Status: Done/Won't Do

Development

Successfully merging this pull request may close these issues.

2 participants