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

Extract and set host header, return X-Powered-By #163

Closed

Conversation

ortz
Copy link

@ortz ortz commented May 30, 2017

Summary

In order to use Veneur behind a webserver or a proxy, some changes need to be applied.
Veneur modifies the endpoint to avoid dns-caching and not preserving the original host header, this breaks Veneur if it sits behind a proxy that is listening to more than one site.

Motivation

Support usage of veneur behind another webserver / proxy (Nginx, Apache, etc...)
Adding visibility once it reached the veneur instance using X-Powered-By

Test plan

I wrote a test to verify the new extractHostPort works as expected and that I didn't break any other tests.

@ortz
Copy link
Author

ortz commented May 30, 2017

@ChimeraCoder fixed it to Veneur 1.2.0+ syntax

@cory-stripe
Copy link
Contributor

Hey @ortz! To resay what @aditya-stripe already did, thanks for this!

We'll chat and decide if we're ready to go full 1.8, but this looks good otherwise. Thanks for the clear explanation and accompanying test.

@cory-stripe
Copy link
Contributor

Update: We're working separately on moving fully to 1.8 which will pave the way for this.

@cory-stripe
Copy link
Contributor

Hey @ortz! Mind rebasing this!

@cory-stripe
Copy link
Contributor

Friendly ping! Mind rebasing @ortz?

@ortz ortz force-pushed the feature/set_host_header_and_return_x_powered_by branch from e04ea59 to b012a2e Compare July 9, 2017 06:54
@stripe-ci
Copy link

Gerald Rule: Copy Observability on Veneur and Unilog pull requests

cc @stripe/observability
cc @stripe/observability-stripe

@ortz
Copy link
Author

ortz commented Jul 9, 2017

@corey-stripe done

@corey-stripe
Copy link

Looks like this was meant for @cory-stripe :)

@ortz
Copy link
Author

ortz commented Jul 10, 2017

Hah :) yup.

@cory-stripe
Copy link
Contributor

Sorry @ortz, master moved again. Give me another rebase and I'll get this in? (Sorry!)

@cory-stripe
Copy link
Contributor

Hey @ortz can you rebase for me?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Or Tzabary seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ortz ortz closed this Mar 22, 2022
@ortz ortz deleted the feature/set_host_header_and_return_x_powered_by branch March 22, 2022 08:15
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.

5 participants