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.

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

[#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics. #1160

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

zhuyaogai
Copy link
Contributor

What changes were proposed in this pull request?

Add labels with disk path for local storage total_localfile_write_data metrics.

Why are the changes needed?

Fix: #1108

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add more unit test case.

@jerqi jerqi requested a review from zuston August 19, 2023 08:27
@jerqi jerqi changed the title [#1108] [FEATURE] Add labels with disk path for local storage total_localfile_write_data metrics. [#1108] feat(server): Add labels with disk path for local storage total_localfile_write_data metrics. Aug 19, 2023
@zhuyaogai
Copy link
Contributor Author

@zuston BTW, do we need to add an aggregate label like LOCAL_DISK_PATH_LABEL_ALL?

Copy link
Member

@zuston zuston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall lgtm.

@zuston BTW, do we need to add an aggregate label like LOCAL_DISK_PATH_LABEL_ALL?

I think the name of all is enough, like #1107 did

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2023

Codecov Report

Merging #1160 (455455d) into master (680b4ea) will increase coverage by 1.11%.
Report is 1 commits behind head on master.
The diff coverage is 87.50%.

@@             Coverage Diff              @@
##             master    #1160      +/-   ##
============================================
+ Coverage     53.63%   54.75%   +1.11%     
- Complexity     2576     2577       +1     
============================================
  Files           391      371      -20     
  Lines         22359    20005    -2354     
  Branches       1875     1876       +1     
============================================
- Hits          11992    10953    -1039     
+ Misses         9660     8415    -1245     
+ Partials        707      637      -70     
Files Changed Coverage Δ
...he/uniffle/server/storage/LocalStorageManager.java 87.62% <85.71%> (-0.14%) ⬇️
...rg/apache/uniffle/server/ShuffleServerMetrics.java 94.91% <100.00%> (ø)

... and 20 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zuston zuston merged commit 4869630 into apache:master Aug 21, 2023
32 checks passed
@zuston
Copy link
Member

zuston commented Aug 21, 2023

Thanks for your contribution! @zhuyaogai Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add labels with disk path for local storage total_localfile_write_data metrics
3 participants