-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add tracing instrumentation for indexing paths #10273
Add tracing instrumentation for indexing paths #10273
Conversation
Gradle Check (Jenkins) Run Completed with:
|
Compatibility status:Checks if related components are compatible with change 533c716 Incompatible componentsIncompatible components: [https://github.com/opensearch-project/cross-cluster-replication.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git] |
@rayshrey I think the PR needs refactoring, can you please do a |
server/src/main/java/org/opensearch/action/bulk/TransportBulkAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/action/support/replication/TransportReplicationAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/telemetry/tracing/AttributeNames.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/telemetry/tracing/SpanBuilder.java
Outdated
Show resolved
Hide resolved
@rayshrey can you also please attach the outputs of the traces? |
Please make SpanName also meaningful so that if somebody just looking at the span should be able to recognise that what's happening in that Span. |
@Gaganjuneja We are using shardPrimaryWrite, shardReplicaWrite and bulkShardAction as the span names currently. Do you have any suggestions/references for the span names ? |
Gradle Check (Jenkins) Run Completed with:
|
@reta lgtm! |
Gradle Check (Jenkins) Run Completed with:
|
server/src/main/java/org/opensearch/action/resync/TransportResyncReplicationAction.java
Show resolved
Hide resolved
Signed-off-by: Shreyansh Ray <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
server/src/main/java/org/opensearch/action/resync/TransportResyncReplicationAction.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Shreyansh Ray <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
@reta Thanks for the approval. Can we resolve the open conversations now and merge this PR (since we have got approvals from all the concerned folks) ? |
server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java
Outdated
Show resolved
Hide resolved
One minor thing, sorry about that, we missed it |
Signed-off-by: Shreyansh Ray <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-10273-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 e691df09c66dcc1693897543fd7633c4b208ce48
# Push it to GitHub
git push --set-upstream origin backport/backport-10273-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x Then, create a pull request where the |
@rayshrey please send manual backport to |
) * Add tracing instrumentation for indexing paths Signed-off-by: Shreyansh Ray <[email protected]> * Fix failing tests and review changes Signed-off-by: Shreyansh Ray <[email protected]> * Fix test failures due to Span not being properly closed Signed-off-by: Shreyansh Ray <[email protected]> * Changes to spans in primary and replica actions Signed-off-by: Shreyansh Ray <[email protected]> * Review comments fixes and refactoring Signed-off-by: Shreyansh Ray <[email protected]> * Precommit auto-changes Signed-off-by: Shreyansh Ray <[email protected]> * Add refresh policy as attribute Signed-off-by: Shreyansh Ray <[email protected]> * Fix changelog entry Signed-off-by: Shreyansh Ray <[email protected]> * Instrument primary/replica write in TransportWriteAction instead of TransportShardBulkAction Signed-off-by: Shreyansh Ray <[email protected]> * Modify SpanBuilder Signed-off-by: Shreyansh Ray <[email protected]> * spotlessApply and precommit Signed-off-by: Shreyansh Ray <[email protected]> * Change span names Signed-off-by: Shreyansh Ray <[email protected]> * Pass Noop Tracer instead of injected tracer Signed-off-by: Shreyansh Ray <[email protected]> * Reverting previous changes Signed-off-by: Shreyansh Ray <[email protected]> * Remove tracer variable from TransportShardBulkAction Signed-off-by: Shreyansh Ray <[email protected]> --------- Signed-off-by: Shreyansh Ray <[email protected]>
) * Add tracing instrumentation for indexing paths Signed-off-by: Shreyansh Ray <[email protected]> * Fix failing tests and review changes Signed-off-by: Shreyansh Ray <[email protected]> * Fix test failures due to Span not being properly closed Signed-off-by: Shreyansh Ray <[email protected]> * Changes to spans in primary and replica actions Signed-off-by: Shreyansh Ray <[email protected]> * Review comments fixes and refactoring Signed-off-by: Shreyansh Ray <[email protected]> * Precommit auto-changes Signed-off-by: Shreyansh Ray <[email protected]> * Add refresh policy as attribute Signed-off-by: Shreyansh Ray <[email protected]> * Fix changelog entry Signed-off-by: Shreyansh Ray <[email protected]> * Instrument primary/replica write in TransportWriteAction instead of TransportShardBulkAction Signed-off-by: Shreyansh Ray <[email protected]> * Modify SpanBuilder Signed-off-by: Shreyansh Ray <[email protected]> * spotlessApply and precommit Signed-off-by: Shreyansh Ray <[email protected]> * Change span names Signed-off-by: Shreyansh Ray <[email protected]> * Pass Noop Tracer instead of injected tracer Signed-off-by: Shreyansh Ray <[email protected]> * Reverting previous changes Signed-off-by: Shreyansh Ray <[email protected]> * Remove tracer variable from TransportShardBulkAction Signed-off-by: Shreyansh Ray <[email protected]> --------- Signed-off-by: Shreyansh Ray <[email protected]>
* Add tracing instrumentation for indexing paths * Fix failing tests and review changes * Fix test failures due to Span not being properly closed * Changes to spans in primary and replica actions * Review comments fixes and refactoring * Precommit auto-changes * Add refresh policy as attribute * Fix changelog entry * Instrument primary/replica write in TransportWriteAction instead of TransportShardBulkAction * Modify SpanBuilder * spotlessApply and precommit * Change span names * Pass Noop Tracer instead of injected tracer * Reverting previous changes * Remove tracer variable from TransportShardBulkAction --------- Signed-off-by: Shreyansh Ray <[email protected]>
) * Add tracing instrumentation for indexing paths Signed-off-by: Shreyansh Ray <[email protected]> * Fix failing tests and review changes Signed-off-by: Shreyansh Ray <[email protected]> * Fix test failures due to Span not being properly closed Signed-off-by: Shreyansh Ray <[email protected]> * Changes to spans in primary and replica actions Signed-off-by: Shreyansh Ray <[email protected]> * Review comments fixes and refactoring Signed-off-by: Shreyansh Ray <[email protected]> * Precommit auto-changes Signed-off-by: Shreyansh Ray <[email protected]> * Add refresh policy as attribute Signed-off-by: Shreyansh Ray <[email protected]> * Fix changelog entry Signed-off-by: Shreyansh Ray <[email protected]> * Instrument primary/replica write in TransportWriteAction instead of TransportShardBulkAction Signed-off-by: Shreyansh Ray <[email protected]> * Modify SpanBuilder Signed-off-by: Shreyansh Ray <[email protected]> * spotlessApply and precommit Signed-off-by: Shreyansh Ray <[email protected]> * Change span names Signed-off-by: Shreyansh Ray <[email protected]> * Pass Noop Tracer instead of injected tracer Signed-off-by: Shreyansh Ray <[email protected]> * Reverting previous changes Signed-off-by: Shreyansh Ray <[email protected]> * Remove tracer variable from TransportShardBulkAction Signed-off-by: Shreyansh Ray <[email protected]> --------- Signed-off-by: Shreyansh Ray <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Description
Added tracing instrumentation for indexing paths.
Sample spans:
Related Issues
Resolves #8555 partially
Check List
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.