-
Notifications
You must be signed in to change notification settings - Fork 321
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 an environment variable to enable HTTP2 #286
Conversation
I don't think an environment variable is the best way to do this. If HTTP/2 has these nasty issues, we should avoid using it whenever possible. For example, in prometheus/prometheus#9068 we think that HTTP/2 is required for scraping, but it isn't needed anywhere else so we should only have it enabled in the scraping engine. An environment variable would turn it on or off everywhere, not giving us the control we need. |
We do not want to give control to the user. This is to be able to contact the people who have met issues with HTTP2 at scale and ask them to test with the newer go versions to see if we can re-enable it prometheus-wide. |
I agree that an environment variable could be effective for testing the re-adoption of HTTP/2 as a whole, but as you pointed out there are still seemingly unresolved bugs and I'm not sure how soon the project will be switching it back on. Ideally, it works great everywhere, but until we know that's the case there will still be users who need HTTP/2 in a certain component and will take assurance that although it could have bugs, it won't affect the rest of their instance. |
Should we label this experimental? To me it seems that it is primarily a tool to ease HTTP/2 back into Prometheus, and therefore may not stay permanently. |
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
The plan is to have the env variable, let the people who discovered the issues test this, and re-enable this project-wide. |
I have rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works great!
We should put this in the changelog of the new Prometheus release so people now to test |
I prefer to contact the people who had the issue initially , otherwise it becomes a config option. the goal here is really canary testing. |
HTTP2 had many issues in Prometheus. Before enabling it again, because
some of the underlying issues have been fixed, I would like to introduce
an environment variable to enable it.
That would enable us to contact some "power users" to test this in the
conditions where they had issues to see if this can be re-enabled in
Prometheus generally.
refs #249 #261
Signed-off-by: Julien Pivotto roidelapluie@inuits.eu