Skip to content

fix: Use dynamic port in Makefile start output#195

Closed
JohannesMahne wants to merge 1 commit intoelastic:mainfrom
JohannesMahne:fix/makefile-dynamic-port
Closed

fix: Use dynamic port in Makefile start output#195
JohannesMahne wants to merge 1 commit intoelastic:mainfrom
JohannesMahne:fix/makefile-dynamic-port

Conversation

@JohannesMahne
Copy link

Summary

  • Query the actual port from Docker instead of hardcoding 8080 in make start output
  • The displayed URLs now respect ENVOY_PORT overrides in .env.override

Test plan

  • Set ENVOY_PORT=8888 in .env.override
  • Run make start
  • Verify the output shows http://localhost:8888 instead of hardcoded 8080

@JohannesMahne JohannesMahne requested a review from a team as a code owner January 23, 2026 04:43
Query the actual port from Docker instead of hardcoding 8080,
so the displayed URLs respect ENVOY_PORT overrides in .env.override.
@JohannesMahne JohannesMahne force-pushed the fix/makefile-dynamic-port branch from 2fc291c to 58bc547 Compare January 23, 2026 04:46
@JohannesMahne
Copy link
Author

Test results
Screenshot 2026-01-23 at 06 49 44

Comment on lines +189 to +194
@PORT=$$(docker port frontend-proxy | head -1 | sed 's/.*://'); \
echo "Go to http://localhost:$$PORT for the demo UI."; \
echo "Go to http://localhost:$$PORT/jaeger/ui for the Jaeger UI."; \
echo "Go to http://localhost:$$PORT/grafana/ for the Grafana UI."; \
echo "Go to http://localhost:$$PORT/loadgen/ for the Load Generator UI."; \
echo "Go to http://localhost:$$PORT/feature/ to change feature flags."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this will hide any failures (e.g. docker port frontend-proxy) inside the pipeline and continue execution. If more robust error handling is needed, there are multiple options (e.g. use bash and pipefail) but the simplest is to lift this logic outside the Makefile into its own shell script, add error handling there, and call that from the Makefile.

@dmathieu
Copy link
Member

This is a change on something that is not elastic-specific. It should be upstreamed to avoid conflicts when merging.

@osullivandonal
Copy link

Thanks for the change @JohannesMahne , I agree with @dmathieu here, this repo is a fork of https://github.com/open-telemetry/opentelemetry-demo, if changes are needed to none elastic files its advised to make the change upstream in https://github.com/open-telemetry/opentelemetry-demo.

@JohannesMahne
Copy link
Author

100% - thank you for all the replies. I will look into the upstream repo and also #195 (comment).

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.

4 participants