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.

Sign up for GitHub

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

Jump to bottom

HDDS-9384. Update OpenTelemetry traces in the write path #5468

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

HDDS-9384. Update OpenTelemetry traces in the write path #5468

wants to merge 3 commits into from

Conversation

jojochuang
Copy link
Contributor

@jojochuang jojochuang commented Oct 19, 2023 •
edited
Loading

What changes were proposed in this pull request?

HDDS-9384. [hsync] Update OpenTelemetry traces in the write path

Please describe your PR in detail:

Traces for

  • Ratis callback function (ContainerStateMachine)
  • DataNode PutBlock/WriteChunk
  • Client BlockOutputStream WriteChunk

What is the link to the Apache JIRA

HDDS-9384

How was this patch tested?

Manual test. Screenshots

@jojochuang jojochuang changed the title Hdds 9384 HDDS-9384. [hsync] Update OpenTelemetry traces in the write path Oct 19, 2023
@adoroszlai
Copy link
Contributor

Maybe the updated PR template is a bit confusing, but "Title" is supposed to be entered only in the title field, not duplicated in the description field.

CC @hemantk-12

@hemantk-12
Copy link
Contributor

hemantk-12 commented Oct 19, 2023 •
edited
Loading

Maybe the updated PR template is a bit confusing, but "Title" is supposed to be entered only in the title field, not duplicated in the description field.

CC @hemantk-12

Yeah, I agree. I've also seen couple of PRs with similar duplicated title.

Is there a way to update PR template to fill "Title" field with this instead of commit message? Otherwise we can update the template to something:

## Provide a general summary of your changes in the Title above
Title should provide one sentence overview of the purpose of the PR.

## What changes were proposed in this pull request?

(Please fill in changes proposed in this fix
1. **Description:**
   * What changes are proposed in the PR? and Why? It would be better if it is written from third person's perspective not just for the reviewer. 
   * Provide as much context and rationale for the pull request as possible. It could be copy-paste from the Jira's description if the jira is well defined.
   * If it is complex code, describe the approach used to solve the issue. If possible attach design doc, issue investigation, github discussion, etc.
2. **PR examples:** [PR#3980](https://github.com/apache/ozone/pull/3980), [PR#5265](https://github.com/apache/ozone/pull/5265), [PR#4701](https://github.com/apache/ozone/pull/4701), [PR#5283](https://github.com/apache/ozone/pull/5283), [PR#5300](https://github.com/apache/ozone/pull/5300))

Example 3 in https://axolo.co/blog/p/part-3-github-pull-request-template

@jojochuang jojochuang added the hbase HBase on Ozone support label Jan 23, 2024
@jojochuang jojochuang marked this pull request as ready for review April 15, 2024 21:48
@adoroszlai adoroszlai changed the title HDDS-9384. [hsync] Update OpenTelemetry traces in the write path HDDS-9384. Update OpenTelemetry traces in the write path Apr 28, 2024
@jojochuang jojochuang requested a review from smengcl May 1, 2024 23:03
@jojochuang
Copy link
Contributor Author

Attach a few screenshots after the change. It's now able to connect thousands of trace spans.

Screenshot 2024-05-01 at 3 00 40 PM Screenshot 2024-05-01 at 3 08 19 PM Screenshot 2024-05-01 at 3 45 43 PM Screenshot 2024-05-01 at 4 00 31 PM

@jojochuang
Copy link
Contributor Author

@chungen0126

@jojochuang
HDDS-9384. [hsync] Update OpenTelemetry traces in the write path
ce76e24 
Tracing

Change-Id: I8afc1d03df1ace013fea3ba6b3016bb13f0743aa
(cherry picked from commit 7b36caf20060b9e41919b60bd7dac7d1b4d28037)

Add traces to BlockOutputStream

Change-Id: Ia726c4085be5e96174870b6d3c367dc7d44df24c
(cherry picked from commit 6456a356b7732862e6f0817bc3fb3b45a39eddd6)

Add more traces

Change-Id: I6efb95e7dc54a8988b7391a632cf788e79ae1e46
(cherry picked from commit f08125dc338d706adc9586bb0b31ec4f985b0e74)

Add starting point traces for StateMachine

Change-Id: I2cf55dcfa91fd2733880dc042a6dcec4c3dd6c81
(cherry picked from commit 4ca599671c843f0734974e03c0f972440e7df61c)

checkstyle

Change-Id: Ib09895542b0b161b80e67e0ed4e677619a620ee5

findbugs.

Change-Id: I62ba4c475bc53d378aefe7d97b215bc58469a9ca

Update

Change-Id: Id15908a3af2c89456e06285e7f101c2a39229c79

Remove docker config files

Change-Id: I6892097ac8d8af2b58dff5df464deeda3240afc9

Add more trace spans to dfsg.

Change-Id: Ia47ecd0b7da50f377ec2b80b403c811b835260ba

Add trace around initliazation and close of ofs.

Change-Id: I6265c755f90acb650b97cf4f5bcc99f11a847c06

Fix async span.

Change-Id: I31cfc4b4070df9f1fd1f514b47261cf864d72b32

checkstyle.

Change-Id: I1f97f4a99afbcc672d1dc099767d0495d421f4ae

Make trace code consistent.

Change-Id: I4bc277c2245102ffe2d38f0898fc465cafba713b
@jojochuang
Update XceiverClientRatis trace spans.
02d66c7 
Change-Id: I97ba366e4db53d2a2eb11dcdeb4130a070c4be6f
@jojochuang
Fix
4934c9b 
Change-Id: Iebb9337ebc1a40c314d69732a5249d9b6552563c
@jojochuang
Copy link
Contributor Author

Screenshot 2024-08-26 at 11 00 04 AM

Rewrote the PR because the block output stream is becoming more asynchronous. Here's the latest screenshot.

@jojochuang jojochuang marked this pull request as draft August 26, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chungen0126 chungen0126 chungen0126 left review comments

@smengcl smengcl Awaiting requested review from smengcl

Assignees
No one assigned
Labels
hbase HBase on Ozone support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jojochuang @adoroszlai @hemantk-12 @chungen0126