-
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
emailservice: remove explicit PORT
env variable
#254
Merged
cartersocha
merged 2 commits into
open-telemetry:main
from
ahayworth:ahayworth/emailservice-port-things
Aug 3, 2022
Merged
emailservice: remove explicit PORT
env variable
#254
cartersocha
merged 2 commits into
open-telemetry:main
from
ahayworth:ahayworth/emailservice-port-things
Aug 3, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a follow-up to #248, which in turn was resolving: #242 (comment) The ruby app is relying on convention, and by convention a `PORT` env varable will determine which port the `puma` webserver actually listens on. So - we don't need to set it explicitly in the compose file, but that means we need to set it elsewhere somehow. I tried a few hacks with the dockerfile: - Trying to pass `PORT=$EMAIL_SERVICE_PORT` - Trying to pass `-p $EMAIL_SERVICE_PORT` in the command But those didn't work and I gave up. So instead we just set it directly in the sinatra app ourselves. (Please don't ask me how that actually gets set down all the way through sinatra -> rack -> puma, because truthfully I do not know: rack-based servers are basically magic). We can see that it works: ``` @ahayworth ➜ /workspaces/opentelemetry-demo-webstore (ahayworth/emailservice-port-things ✗) $ git grep EMAIL_SERVICE_P ORT .env:EMAIL_SERVICE_PORT=6060 ``` ``` @ahayworth ➜ /workspaces/opentelemetry-demo-webstore (ahayworth/emailservice-port-things ✗) $ docker-compose up --build emailservice email-service | == Sinatra (v2.2.0) has taken the stage on 6060 for production with backup from Puma email-service | I, [2022-08-03T13:08:27.643291 #1] INFO -- : Instrumentation: OpenTelemetry::Instrumentation::Sinatra was successfully installed with the following options {} email-service | Puma starting in single mode... email-service | * Puma version: 5.6.4 (ruby 3.1.2-p20) ("Birdie's Version") email-service | * Min threads: 0 email-service | * Max threads: 5 email-service | * Environment: production email-service | * PID: 1 email-service | * Listening on http://0.0.0.0:6060 email-service | Use Ctrl-C to stop ```
mviitane
approved these changes
Aug 3, 2022
mic-max
approved these changes
Aug 3, 2022
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.
Thanks!
fatsheep9146
approved these changes
Aug 3, 2022
jmichalak9
pushed a commit
to jmichalak9/opentelemetry-demo
that referenced
this pull request
Mar 22, 2024
This is a follow-up to open-telemetry#248, which in turn was resolving: open-telemetry#242 (comment) The ruby app is relying on convention, and by convention a `PORT` env varable will determine which port the `puma` webserver actually listens on. So - we don't need to set it explicitly in the compose file, but that means we need to set it elsewhere somehow. I tried a few hacks with the dockerfile: - Trying to pass `PORT=$EMAIL_SERVICE_PORT` - Trying to pass `-p $EMAIL_SERVICE_PORT` in the command But those didn't work and I gave up. So instead we just set it directly in the sinatra app ourselves. (Please don't ask me how that actually gets set down all the way through sinatra -> rack -> puma, because truthfully I do not know: rack-based servers are basically magic). We can see that it works: ``` @ahayworth ➜ /workspaces/opentelemetry-demo-webstore (ahayworth/emailservice-port-things ✗) $ git grep EMAIL_SERVICE_P ORT .env:EMAIL_SERVICE_PORT=6060 ``` ``` @ahayworth ➜ /workspaces/opentelemetry-demo-webstore (ahayworth/emailservice-port-things ✗) $ docker-compose up --build emailservice email-service | == Sinatra (v2.2.0) has taken the stage on 6060 for production with backup from Puma email-service | I, [2022-08-03T13:08:27.643291 open-telemetry#1] INFO -- : Instrumentation: OpenTelemetry::Instrumentation::Sinatra was successfully installed with the following options {} email-service | Puma starting in single mode... email-service | * Puma version: 5.6.4 (ruby 3.1.2-p20) ("Birdie's Version") email-service | * Min threads: 0 email-service | * Max threads: 5 email-service | * Environment: production email-service | * PID: 1 email-service | * Listening on http://0.0.0.0:6060 email-service | Use Ctrl-C to stop ``` Co-authored-by: Carter Socha <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This is a follow-up to #248, which in turn was resolving: #242 (comment)
The ruby app is relying on convention, and by convention a
PORT
envvarable will determine which port the
puma
webserver actually listenson. So - we don't need to set it explicitly in the compose file, but
that means we need to set it elsewhere somehow.
I tried a few hacks with the dockerfile:
PORT=$EMAIL_SERVICE_PORT
-p $EMAIL_SERVICE_PORT
in the commandBut those didn't work and I gave up. So instead we just set it directly
in the sinatra app ourselves. (Please don't ask me how that actually
gets set down all the way through sinatra -> rack -> puma, because
truthfully I do not know: rack-based servers are basically magic).
We can see that it works:
cc @mic-max