Hotrod Containerized #694

gbaufake · 2018-02-10T00:43:42Z

Hello Jaeger,
I took the Hotrod example and tried to containerize it.

The odd behavior that is hard to predict is the random UDP port each time the container spins up

hotrod_1 | 2018-02-10T00:39:34.927Z ERROR log/logger.go:42 write udp 127.0.0.1:49372->127.0.0.1:6831: write: connection refused {"service": "frontend"}

hotrod_1 | 2018-02-10T00:41:49.254Z ERROR log/logger.go:42 write udp 127.0.0.1:46555->127.0.0.1:6831: write: connection refused {"service": "frontend"}

Is there some way to use one UDP port?

Best Regards,
Guilherme Baufaker Rêgo

coveralls · 2018-02-10T00:55:20Z

Coverage remained the same at 100.0% when pulling d5250a6 on gbaufake:hotrod-docker into 9fe8cc8 on jaegertracing:master.

pavolloffay

@gbaufake thanks! Just some minor things.

pavolloffay · 2018-02-12T13:19:33Z

examples/Dockerfile

@@ -0,0 +1,4 @@
+FROM alpine


For the consistency could you please:

use from scratch

COPY collector-linux /go/bin/

Dockerfile should be in examples/hotrod directory

@pavolloffay is this collector-linux folder in jaeger project? I didn't find it

pavolloffay · 2018-02-12T13:23:39Z

examples/Dockerfile

@@ -0,0 +1,4 @@
+FROM alpine
+EXPOSE 8080


I think it should expose more ports

What ports else do I need to export?

When you run all it prints all used ports

pavolloffay · 2018-02-12T13:25:53Z

examples/hotrod-containerized/README.md

@@ -0,0 +1,41 @@
+# Hot R.O.D. - Rides on Demand - Containerized Version


Why new directory hotrod-containerized?

it can live in examples/hotrod and just add instructions to the original readme

+1, I'd prefer all in one directory

OK. Changing to one hotrod directory

pavolloffay · 2018-02-12T13:33:01Z

examples/Dockerfile

+FROM alpine
+EXPOSE 8080
+COPY hotrod /
+ENTRYPOINT ./main all


This allows using only all command. Maybe it's better to use just CMD with all

Fwiw I never tested HotRod as individual processes

pavolloffay · 2018-02-12T13:34:06Z

examples/hotrod-containerized/docker-compose.yml

+    - "8081:8081"
+    - "8082:8082"
+    - "8083:8083"
+  links:


Do we want compose file for this at all?

If so could you please remove links?

From the author comments it does look like compose has some issues with ports. I would split it in a separate PR, assuming we even need it.

@yurishkuro @pavolloffay I used docker-compose as "quick and easy way" to create booth jaegertracing/all-in-one container and jaegertracing/hotrod.

+1 let's go with dockerfile and then add compose if necessary

yurishkuro · 2018-02-12T16:46:19Z

How will we test that the generated image is operational?

pavolloffay · 2018-02-13T11:00:30Z

How will we test that the generated image is operational?

We could have similar tests to build-all-in-one-image.sh. Build the image, start it and curl an endpoint.

gbaufake · 2018-02-14T16:30:53Z

@pavolloffay and @yurishkuro I moved everything to Hotrod folder and adjusted minor things on Dockerfile.
Can you guys check why it uses a different UDP port to connect to Jaeger on 6831?

Examples:
write udp 127.0.0.1:46555->127.0.0.1:6831: write: connection refused {"service": "frontend"}
write udp 127.0.0.1:49372->127.0.0.1:6831: write: connection refused {"service": "frontend"}
write udp 127.0.0.1:35316->127.0.0.1:6831: write: connection refused {"service": "frontend"}
write udp 127.0.0.1:43547->127.0.0.1:6831: write: connection refused {"service": "frontend"}

yurishkuro · 2018-02-14T16:26:53Z

examples/hotrod/README.md

+
+```
+chmod +x ./create-image.sh
+./create-image.sh


x flag can be set and recorded in git. If not, the instruction can be sh ./create-image.sh

yurishkuro · 2018-02-14T16:28:00Z

examples/hotrod/README.md

+
+
+# HotROD on Containerized Version
+## Requirements


this is a bit confusing - the point is to let people run from published containers, is it not? In which case go is not a requirement. I think the instruction on how to run should be around L34. Basically think about it holistically - if someone who doesn't even know Go but wants to play with hotrod/jaeger, we should have instructions right at the top, and then below "if you want to build from source then ... (L34-39)"

I would remove everything and just keep two docker run commands :)

yurishkuro · 2018-02-14T16:32:19Z

examples/hotrod/README.md

+
+
+[hotrod-tutorial]: https://medium.com/@YuriShkuro/take-opentracing-for-a-hotrod-ride-f6e3141f7941
+[hotrod-openshift]: https://blog.openshift.com/openshift-commons-briefing-82-distributed-tracing-with-jaeger-prometheus-on-kubernetes


remove dups in L45-46

yurishkuro · 2018-02-14T16:35:31Z

examples/hotrod/create-image.sh

+make install
+cd $GOPATH/src/github.com/jaegertracing/jaeger/examples/hotrod
+CGO_ENABLED=0 GOOS=linux installsuffix=cgo go build -o collector-linux main.go
+docker build -t jaegertracing/hotrod -f Dockerfile .


I think this won't be enough, we need to add this to Travis build and publish to docker hub along with other images.

@pavolloffay how would you like to proceed here? This might be getting a bit more involved for @gbaufake who is new to this repo. We could merge this (without README changes) and then make the build changes.

agree, We can merge this with dockerfile only. Then I can add publish and test for it.

Rather then this script we could add make rules, otherwise there will be some repetition here and in the main makefile.

pavolloffay · 2018-02-14T16:54:52Z

examples/hotrod/Dockerfile

@@ -0,0 +1,4 @@
+FROM scratch
+EXPOSE 8080


I think there are still some ports missing, look at logs when you start all

pavolloffay · 2018-02-14T16:58:06Z

examples/hotrod/Dockerfile

@@ -0,0 +1,4 @@
+FROM scratch
+EXPOSE 8080
+COPY . /


Could you please add only specific files and keed the name hotrod-demo?

pavolloffay · 2018-02-14T17:08:32Z

examples/hotrod/README.md

+
+
+# HotROD on Containerized Version
+## Requirements


I would remove everything and just keep two docker run commands :)

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

and some minor adjustments on Dockerfile Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

gbaufake · 2018-02-15T19:35:43Z

@pavolloffay @yurishkuro I have made the modifications that you asked.
Any other changes required?

yurishkuro · 2018-02-16T00:13:16Z

examples/hotrod/create-image.sh

+cd $GOPATH/src/github.com/jaegertracing/jaeger
+make install
+cd $GOPATH/src/github.com/jaegertracing/jaeger/examples/hotrod
+CGO_ENABLED=0 GOOS=linux installsuffix=cgo go build -o collector-linux main.go


nit: collector-linux => hotrod-linux

changed in d5250a6

yurishkuro · 2018-02-16T00:13:24Z

examples/hotrod/Dockerfile

+FROM scratch
+EXPOSE 8080 8081 8082 8083
+COPY collector-linux /
+CMD ["./collector-linux", "all"]


nit: collector-linux => hotrod-linux

changed in d5250a6

yurishkuro · 2018-02-16T00:14:42Z

examples/hotrod/README.md

+
+
+
+# HotROD on Containerized Version


I would prefer if we didn't change the readme at this time. The first docker command won't run since the hotrod image is not actually published.

Removed the changes README in d5250a6

- exposing more ports on Dockerfile and copy only binary to Dockerfile Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

pavolloffay · 2018-02-19T14:25:47Z

@gbaufake thanks

pavolloffay · 2018-02-19T17:18:24Z

Static files are not present in the image. I am creating a fix for it.

gbaufake · 2018-02-19T21:02:11Z

@pavolloffay
Thanks for merging.
Maybe it was caused when I switched from the whole folder just to binary one;
Can you check the UDP ports used to connect to Jaeger?

gbaufake requested review from black-adder, pavolloffay, vprithvi and yurishkuro as code owners February 10, 2018 00:43

pavolloffay reviewed Feb 12, 2018

View reviewed changes

gbaufake force-pushed the hotrod-docker branch 2 times, most recently from 0398563 to 4e0bf0a Compare February 14, 2018 16:06

yurishkuro reviewed Feb 14, 2018

View reviewed changes

pavolloffay reviewed Feb 14, 2018

View reviewed changes

Guilherme Baufaker Rêgo added 3 commits February 14, 2018 18:43

Binding all interfaces on hotrod

a6813b6

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

Hotrod Containerized on Docker

30a6771

Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

Moving HotRod Containerized to Hotrod folder

13599f1

and some minor adjustments on Dockerfile Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

gbaufake force-pushed the hotrod-docker branch from c40cb5f to de53814 Compare February 14, 2018 20:44

yurishkuro approved these changes Feb 16, 2018

View reviewed changes

yurishkuro reviewed Feb 16, 2018

View reviewed changes

gbaufake force-pushed the hotrod-docker branch from de53814 to ab25c02 Compare February 16, 2018 11:06

Copy only Hotrod binary to Dockerfile

d5250a6

- exposing more ports on Dockerfile and copy only binary to Dockerfile Signed-off-by: Guilherme Baufaker Rêgo <gbaufake@redhat.com>

gbaufake force-pushed the hotrod-docker branch from ab25c02 to d5250a6 Compare February 16, 2018 11:10

pavolloffay merged commit a631623 into jaegertracing:master Feb 19, 2018

This was referenced Feb 20, 2018

Package static assets inside the query-service binary #609

Closed

hotrod docker image and configurable jaeger agent host and port #204

Closed

yurishkuro mentioned this pull request Jul 24, 2018

Add flags to allow changing ports for HotROD services #951

Merged

		@@ -0,0 +1,41 @@
		# Hot R.O.D. - Rides on Demand - Containerized Version



		[hotrod-tutorial]: https://medium.com/@YuriShkuro/take-opentracing-for-a-hotrod-ride-f6e3141f7941
		[hotrod-openshift]: https://blog.openshift.com/openshift-commons-briefing-82-distributed-tracing-with-jaeger-prometheus-on-kubernetes

Hotrod Containerized #694

Hotrod Containerized #694

Conversation

gbaufake commented Feb 10, 2018 • edited Loading

coveralls commented Feb 10, 2018 • edited Loading

pavolloffay left a comment

Choose a reason for hiding this comment

pavolloffay Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro commented Feb 12, 2018

pavolloffay commented Feb 13, 2018

gbaufake commented Feb 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbaufake commented Feb 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavolloffay commented Feb 19, 2018

pavolloffay commented Feb 19, 2018

gbaufake commented Feb 19, 2018 • edited Loading

gbaufake commented Feb 10, 2018 •

edited

Loading

coveralls commented Feb 10, 2018 •

edited

Loading

pavolloffay Feb 12, 2018 •

edited

Loading

gbaufake commented Feb 14, 2018 •

edited

Loading

gbaufake commented Feb 19, 2018 •

edited

Loading