OpenSearch
OpenSearch copied to clipboard
[Tracing Instrumentation] Add instrumentation in transport action
Description
Add instrumentation in TransportAction class so that it's generically available to all the actions.
Related Issues
Resolves #10095
Check List
- [x] New functionality includes testing.
- [x] All tests pass
- [x] New functionality has been documented.
- [x] New functionality has javadoc added
- [x] Commits are signed per the DCO using --signoff
- [x] Commit changes are listed out in CHANGELOG.md file (See: Changelog)
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, Please take a look. Thanks!
Compatibility status:
Checks if related components are compatible with change db2e32e
Incompatible components
Incompatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/reporting.git]
Skipped components
Compatible components
Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/performance-analyzer-rca.git]
Gradle Check (Jenkins) Run Completed with:
- RESULT: FAILURE :x:
- URL: https://build.ci.opensearch.org/job/gradle-check/25771/
- CommitID: 3df1593d24d16d4c6baa2cc2bde9d0f463e59a7d 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?
Gradle Check (Jenkins) Run Completed with:
- RESULT: FAILURE :x:
- URL: https://build.ci.opensearch.org/job/gradle-check/25772/
- CommitID: 325a8f2b9709c63489689efc48387604b9e2079b 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?
Gradle Check (Jenkins) Run Completed with:
- RESULT: FAILURE :x:
- URL: https://build.ci.opensearch.org/job/gradle-check/25774/
- CommitID: 9f3d58c46dfde5e690c986d814b51a5c4f033f5e 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?
Gradle Check (Jenkins) Run Completed with:
- RESULT: FAILURE :x:
- URL: https://build.ci.opensearch.org/job/gradle-check/25782/
- CommitID: db2e32ec212483614b649ac9a80c242fc17c019a 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, Please take a look. Thanks!
@Gaganjuneja I honestly doubt that this is what we should be doing:
- this change will break each and every plugin (since it changes the
TransportAction
base class) - more importantly, transport actions are always executed in context of remote (or local) request, there is no need to instrument each but only the one place when they are materialized and executed
As with HTTP actions, we have to take a step back and start from the beginning, the network layer, which is denoted by InboundHandler
@reta, Please take a look. Thanks!
@Gaganjuneja I honestly doubt that this is what we should be doing:
- this change will break each and every plugin (since it changes the
TransportAction
base class)- more importantly, transport actions are always executed in context of remote (or local) request, there is no need to instrument each but only the one place when they are materialized and executed
As with HTTP actions, we have to take a step back and start from the beginning, the network layer, which is denoted by
InboundHandler
@reta, I agree, We should be instrumenting the specific implementation of a TransportAction based on the requirement and that would add more value, This I realized when I was looking at the TransportBulkAction. I have raised another PR (#10143) for InboundHandler. One catch here is that it might break again the security plugin.
This PR is stalled because it has been open for 30 days with no activity.
Hi @Gaganjuneja, the PR is stalled. Is this being worked upon? Feel free to reach out to maintainers for further reviews.
This PR is stalled because it has been open for 30 days with no activity.
@Gaganjuneja @reta Do we still plan to iterate on this PR (considering this is introducing a breaking change ?) If not, then can we consider closing this PR out ?
@Gaganjuneja @reta Do we still plan to iterate on this PR (considering this is introducing a breaking change ?) If not, then can we consider closing this PR out ?
@Gaganjuneja certainly your call, I think the breaking part should not be an issue (in this particular case)
This PR is stalled because it has been open for 30 days with no activity.