search

fluent / fluent-plugin-prometheus

A fluent plugin that collects metrics and exposes for Prometheus.
Apache License 2.0
257 stars 80 forks source link

Add gzip support #223

Closed Athishpranav2003 closed 1 month ago

Athishpranav2003 commented 1 month ago

Added support for gzip compression while exposing metrics. Tested locally with prometheus server image

219

Lusitaniae commented 1 month ago

amazing turnaround, much appreciated for your efforts @Athishpranav2003!

Athishpranav2003 commented 1 month ago

Thanks @Lusitaniae One help The signoff check is not passing for me from first. Initially I had dummy author creds in local git so it messed it. I reverted to original stuff and still not passing the check. Can you help me with that alone. Also who has write access to merge this?

Lusitaniae commented 1 month ago

Looks like you just need to follow these steps:

In your local branch, run: git rebase HEAD~2 --signoff Force push your changes to overwrite the branch: git push --force-with-lease origin add-gzip-support

https://github.com/fluent/fluent-plugin-prometheus/pull/223/checks?check_run_id=28060535686

Athishpranav2003 commented 1 month ago

@ashie can you please check this

ashie commented 1 month ago

@Athishpranav2003 Thanks for your contribution! Could you follow DCO as @Lusitaniae mentioned? We require Signed-off-by: field in each commit message.

Looks like you just need to follow these steps:

In your local branch, run: git rebase HEAD~2 --signoff Force push your changes to overwrite the branch: git push --force-with-lease origin add-gzip-support

https://github.com/fluent/fluent-plugin-prometheus/pull/223/checks?check_run_id=28060535686

Athishpranav2003 commented 1 month ago

@ashie I have made the changes. In addition i have added zlib as dependency to avoid break incase some system misses it(highly unlikely).

ashie commented 1 month ago

I'll fix CI for Ruby 2.7 in another pull request, so please leave it.

Athishpranav2003 commented 1 month ago

@ashie do you need help in testing this locally before merging

ashie commented 1 month ago

It looks good, I'll merge this soon after checking if we missed anything in relation to the surrounding code.

Athishpranav2003 commented 1 month ago

Ok I will take a look at the comments.

Regarding UTs there is no UTs written for the plugin. So the UTs have go in a seperate PR if we want a Minitest kinda UTs. For now I was manually testing the changes.

Athishpranav2003 commented 1 month ago

@ashie i have addressed your comments and also refactored some small part of my code

Athishpranav2003 commented 1 month ago

@ashie i have added UTs now please take a look at it when u are free

Athishpranav2003 commented 1 month ago

@ashie addressed the comments

ashie commented 1 month ago

I've removed wrongly added fluent-plugin-prometheus-2.1.0.gem in your commit.

ashie commented 1 month ago

Thanks!