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

Migrate build to use docker compose #24

Closed
5 tasks
mic-max opened this issue May 19, 2022 · 3 comments · Fixed by #74
Closed
5 tasks

Migrate build to use docker compose #24

mic-max opened this issue May 19, 2022 · 3 comments · Fixed by #74
Assignees
Labels
enhancement New feature or request

Comments

@mic-max
Copy link
Contributor

mic-max commented May 19, 2022

The docker compose build will build all services on all platforms

  • Create a docker-compose.yml at the root of the repository
  • Adapt all services Dockerfile files
    • Remove duplication of demo.proto file
    • Compile protobuf source files during build
  • Testing

References

@mic-max mic-max added the enhancement New feature or request label May 19, 2022
@mic-max mic-max self-assigned this May 19, 2022
@mic-max mic-max mentioned this issue May 23, 2022
2 tasks
@cartersocha
Copy link
Contributor

Hey @mic-max, what's the status of this? Do you have any key blockers?

@julianocosta89
Copy link
Member

julianocosta89 commented May 29, 2022

Hello @mic-max, I've tried to reach out to you via Slack, but I guess here it is better, because we can have it documented.

I've taken a look at the PR #33 and I saw that there was a lot of changes regarding env vars, dockerfiles and proto files.

Don't you think it would be better to split them up into small PRs?

I've tried to set it up locally with docker compose and I couldn't make it work.
I have tried to make some changes, but still the services are not talking with each other and the traces are not arriving at Jaeger. It seems that the proto changes have affected the behavior of the services.

I've taken your docker compose as start and created a simpler one, just to make the app run.
I've pushed into a branch:
main...feature/docker-compose

If you agree I can open a PR, and then we can do the changes you have done in your PR gradually.

What do you think?

I don't want to step on your toes, please don't get me wrong.
If you don't agree I can remove the branch and we continue on yours.
You are the boss here 😅

@cartersocha
Copy link
Contributor

+1 to smaller, targeted PRs. @cijothomas 😉

This was referenced May 30, 2022
smith pushed a commit to smith/opentelemetry-demo that referenced this issue Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants