Add the means to extract the contextual properties from HttpChannel, TcpChannel and TrasportChannel without excessive typecasting

[reta]

Description

While instrumenting the transports (both TCP and HTTP), we found out that security plugin heavily depends on netty transport (in fact, it does not support anything else) but it is excessive typecasting the different transport object to their Netty4Xxx equivalents for accessing handlers and attributes. It significantly complicates any instrumentation efforts and needs general mechanism to access contextual information without typecasting.

Related Issues

Related to opensearch-project/security#3515

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)
• Public documentation issue/PR created

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.

[reta]

@peternied @Gaganjuneja fyi

[reta]

(I will add changelog entry right when Gradle check fails, since it should)

[cwperks]

Thank you @reta! There are a couple of areas that the security plugin is coupled to netty:

1. The security plugin's implementation of SSL relies on adding a step to the netty pipeline. The SSL Handler is the first step of the pipeline: https://github.com/opensearch-project/security/blob/c5f8c83cba3b4d6dfb7f9ec268a23874f7909003/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java#L147-L152
2. As part of Add early rejection from RestHandler for unauthorized requests security#3418, a step in the netty pipeline was added before decompression to verify the request based on headers and uses netty channel attributes

As far as I can see, there are no automated tests to verify the behavior amongst other http engines

[reta]

I think for the instrumentation part with this change we should be set, what do you think?

The security plugin would need to set attributes as well. Can that be added?

Which ones?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/27631/
• CommitID: 0eb752b
  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?

[github-actions]

Compatibility status:

Checks if related components are compatible with change 1c184e2

Incompatible components

Incompatible components: [https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      2 org.opensearch.repositories.azure.AzureBlobContainerRetriesTests.testReadRangeBlobWithRetries

• URL: https://build.ci.opensearch.org/job/gradle-check/27636/
• CommitID: c84c736
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[codecov]

Codecov Report

Merging #10562 (1c184e2) into main (9bcd7ea) will increase coverage by 0.00%.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               main   #10562   +/-   ##
=========================================
  Coverage     71.16%   71.16%           
- Complexity    58399    58403    +4     
=========================================
  Files          4844     4844           
  Lines        275289   275306   +17     
  Branches      40083    40086    +3     
=========================================
+ Hits         195898   195928   +30     
+ Misses        62997    62956   -41     
- Partials      16394    16422   +28     

Files	Coverage Δ
...src/main/java/org/opensearch/http/HttpChannel.java	50.00% <0.00%> (-50.00%)	⬇️
...lemetry/tracing/channels/TraceableHttpChannel.java	54.54% <0.00%> (-2.60%)	⬇️
...tracing/channels/TraceableTcpTransportChannel.java	83.87% <0.00%> (-2.80%)	⬇️
...org/opensearch/transport/TaskTransportChannel.java	80.00% <0.00%> (-5.72%)	⬇️
...main/java/org/opensearch/transport/TcpChannel.java	85.71% <0.00%> (-14.29%)	⬇️
.../org/opensearch/transport/TcpTransportChannel.java	76.66% <0.00%> (-2.65%)	⬇️
...ava/org/opensearch/transport/TransportChannel.java	40.00% <0.00%> (-4.45%)	⬇️
.../opensearch/transport/netty4/Netty4TcpChannel.java	81.13% <0.00%> (-6.63%)	⬇️
.../org/opensearch/http/netty4/Netty4HttpChannel.java	65.38% <0.00%> (-19.62%)	⬇️

... and 469 files with indirect coverage changes

[reta]

The security plugin would need to set attributes as well. Can that be added?

Discussed with @cwperks offline, we'll looking into that separately

[peternied]

Thanks for making access to this more generic - I much prefer this approach rather than downstream 'peaking' of the types. Couple of minor comments around the new interface.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/27648/
• CommitID: ab4140c
  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?

[reta]

@cwperks @peternied @Gaganjuneja thanks for the review folks, the attributes issue is more complicated, looking if we even need that, but in any case the solution would be different and it would make sense to have a dedicated pull request for it.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/27659/
• CommitID: 1c184e2
  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?

[Gaganjuneja]

LGTM.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: SUCCESS ✅
• URL: https://build.ci.opensearch.org/job/gradle-check/27665/
• CommitID: 1c184e2