Rule: add ability to send POST requests

[GiedriusS]

Feature request description

Thanos Rule currently sends everything with GET requests. We should add an option to send the requests as POST. Accordingly, the logs should be improved to also log the POST (form) parameters.

Rationale

The alerting / recording rules' expressions might be very long and not fit into the URI.

[yashrsharma44]

If no one is taking this issue, I am having a try 😄

[yashrsharma44]

Hey @GiedriusS !
I was trying to wrap my head around the use case of this feature, so can you elaborate on what are the requirements and acceptance criteria for this issue?

Also can you elaborate more on this?

The alerting / recording rules' expressions might be very long and not fit into the URI.

[GiedriusS]

Hey @GiedriusS !
I was trying to wrap my head around the use case of this feature, so can you elaborate on what are the requirements and acceptance criteria for this issue?

Also can you elaborate more on this?

The alerting / recording rules' expressions might be very long and not fit into the URI.

There's no need for formal acceptance criteria - it's not work for us. At least for the majority of us.

Different implementations of HTTP browsers/servers have different limits on URL lengths. For example: https://technomanor.wordpress.com/2012/04/03/maximum-url-size/. How it stands right now, Thanos Rule encode everything into the URL when sending a request:

thanos/pkg/promclient/promclient.go

Line 339 in 2262f26

req, err := http.NewRequest("GET", u.String(), nil)

The recording or alerting rule might be too long and then it will get truncated when sending a request. That's why support for encoding request parameters via POST on /api/v1/query (and others) has been added. So:

• A new parameter needs to appear that controls whether GET or POST is used. We could theoretically convert everyone to use GET but very old versions do not support POST;
• The documentation needs to have verbiage about this;
• The tests need to be improved to test out this case.

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[GiedriusS]

Still very valid & help welcome!

[jlr52]

@GiedriusS

Hi I would love to contribute on this.

Does the ask only apply to promclient.go or does it apply to all queries in the repo that have post supported in https://github.com/thanos-io/thanos/blob/0aa297bd00816ed5efe5d6dea117cb7059a3d7b1/pkg/query/api/v1.go like the following

	r.Get("/query", instr("query", api.query))
	r.Post("/query", instr("query", api.query))
	r.Get("/query_range", instr("query_range", api.queryRange))
	r.Post("/query_range", instr("query_range", api.queryRange))
	r.Get("/series", instr("series", api.series))
	r.Post("/series", instr("series", api.series))
	r.Get("/labels", instr("label_names", api.labelNames))
	r.Post("/labels", instr("label_names", api.labelNames))

[bwplotka]

It's only for Thanos Rule client side for Query API. There is no need to change anything on server side (:

…

On Sat, 13 Jun 2020 at 11:08, jlr52 ***@***.***> wrote: @GiedriusS <https://github.com/GiedriusS> Hi I would love to contribute on this. Does the ask only apply to promclient.go or does it apply to all queries in the repo that have post supported in https://github.com/thanos-io/thanos/blob/0aa297bd00816ed5efe5d6dea117cb7059a3d7b1/pkg/query/api/v1.go like the following r.Get("/query", instr("query", api.query)) r.Post("/query", instr("query", api.query)) r.Get("/query_range", instr("query_range", api.queryRange)) r.Post("/query_range", instr("query_range", api.queryRange)) r.Get("/series", instr("series", api.series)) r.Post("/series", instr("series", api.series)) r.Get("/labels", instr("label_names", api.labelNames)) r.Post("/labels", instr("label_names", api.labelNames)) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2577 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3O6DXW3Y25NAQMU2C5DRWNF2JANCNFSM4M4DUYQQ> .

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[GiedriusS]

We plan on using Thanos Ruler more at $WORK so we might just implement this not too long from now if no one else wants to take a stab at this.

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[GiedriusS]

Still valid.

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[mritunjaysharma394]

Hello everyone! If this issue is free to take, I would love to learn and work on this issue 😄