-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[frontend] [ops golf?] Pass collector URL to frontend via reverse proxy. #508
[frontend] [ops golf?] Pass collector URL to frontend via reverse proxy. #508
Conversation
@puckpuck had an excellent idea to use envoy rather than nginx. there is one small catch here, that we have to build an envoy container in order to get Someone more enterprising than I can enable tracing/metrics off this post v1. |
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.
Just a few small naming things.
src/frontendProxy
-> src/frontend proxy
all lowercase. This is to stay consistent with the other services
The frontend is broken on my machine? Is port 8080 still the right connection or do I need to navigate to the reverse proxy? |
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.
Small pause to prevent merging before localhost question is answered
If we update the docker file to port 80 we should be good |
and changelog + architecture |
what part of the dockerfile? the compose file or the frontend? |
We need to update the readmes to indicate we run on port 80 now |
Updated the docker readme with the current state |
This reverts commit 63c7bcf.
…ectorToFE # Conflicts: # docs/kubernetes_deployment.md
…xy. (open-telemetry#508) * support setting collector URL for browser * rename to frontend-proxy * refactor to use envoy rather than nginx * cleanup * fix composefile * delete unneeded nginx conf template * cleanup naming * set cors to * * also front ffs with envoy * update docs, changelog, etc * update one more port mapping in docs * update for envoy url * update for envoy url * Revert "update for envoy url" This reverts commit 63c7bcf. * fixup Co-authored-by: Pierre Tessier <pierre@pierretessier.com>
This fixes the current problem of the collector URL not being passed into the frontend, as Next.JS will not pass env vars into the client except ones known at build time.
It accomplishes this by way of establishing a reverse proxy that serves the Next.JS app (best practice!) and uses it to set a cookie that the client code reads on load.