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

add push-labels option for java sdk and add --custom-lables option for mount #4312

Merged
merged 5 commits into from
Jan 9, 2024

Conversation

reAsOn2010
Copy link
Contributor

@reAsOn2010 reAsOn2010 commented Jan 5, 2024

close #4245

Does some defensive programming (check the format of parameter) needed ?

@CLAassistant
Copy link

CLAassistant commented Jan 5, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (d1dcd95) 55.81% compared to head (2d2ec59) 55.78%.
Report is 6 commits behind head on main.

Files Patch % Lines
cmd/mount.go 55.00% 6 Missing and 3 partials ⚠️
cmd/mdtest.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4312      +/-   ##
==========================================
- Coverage   55.81%   55.78%   -0.04%     
==========================================
  Files         153      153              
  Lines       39631    39654      +23     
==========================================
- Hits        22122    22120       -2     
- Misses      15030    15053      +23     
- Partials     2479     2481       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhijian-pro
Copy link
Contributor

Does some defensive programming (check the format of parameter) needed ?

Yes

@zhijian-pro
Copy link
Contributor

The above code can only solve the problem of java sdk custom lable, go process also need to be added.
If you are interested, you can improve the section.

func wrapRegister(mp, name string) (prometheus.Registerer, *prometheus.Registry) {

@reAsOn2010
Copy link
Contributor Author

OK, I searched the code base with pattern "push-gateway" then I only find the code in java sdk. I'll dive into the golang process code.

@zhijian-pro
Copy link
Contributor

zhijian-pro commented Jan 5, 2024

The previous issue was a special case, and we should support --custom-lables, whether push (push-gateway) or pull (prometheus).

@zhijian-pro zhijian-pro changed the title add push-labels option for java sdk add push-labels option for java sdk and add --custom-lables for mount Jan 8, 2024
@zhijian-pro zhijian-pro changed the title add push-labels option for java sdk and add --custom-lables for mount add push-labels option for java sdk and add --custom-lables option for mount Jan 8, 2024
@reAsOn2010
Copy link
Contributor Author

Is this PR causing test failure?

@zhijian-pro
Copy link
Contributor

It doesn't matter if mutate-test doesn't pass.

@zhijian-pro
Copy link
Contributor

LGTM!

@zhijian-pro zhijian-pro requested a review from SandyXSD January 9, 2024 04:31
@SandyXSD SandyXSD merged commit 94bdcc6 into juicedata:main Jan 9, 2024
29 of 34 checks passed
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.

Support --push-gateway-labels option
4 participants