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

Elixir Chat Service #1621

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from

Conversation

joshleecreates
Copy link
Contributor

@joshleecreates joshleecreates commented Jun 22, 2024

Changes

Fix #1511.
Creates an elixir-based chat service (Draft)

TODO

  • Update frontend to join channels based on the username
  • Create an "Admin" frontend page to join user chats
  • Add a proper UI for chat messages
  • Add an ecto/postgres integration for persisting chats from the genserver
  • Add a custom macro to simplify telemetry generation in channels
  • Move to main docker-compose
  • Fix span representation for message producers/consumers
  • Ensure ecto spans are generated
  • Move UI out of phoenix and remove phoenix page controller

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
  • Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

@joshleecreates
Copy link
Contributor Author

The backend for this is more or less ready, I just need to add Postgres. If anybody is interested in helping with the front-end or doing something with React or more fancy than what I have here, the help would be welcome

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 29, 2024
@github-actions github-actions bot added the helm-update-required Requires an update to the Helm chart when released label Jul 7, 2024
@julianocosta89
Copy link
Member

Hey @joshleecreates 👋🏽 , I've tried starting this up to see what I could collaborate, but got some bumps on the road.

1st the tailwind.config.js wasn't committed, I've copied the one from here https://github.com/dwyl/phoenix-chat-example/blob/8e09af3c5d85fcdc4b31cfd0b5fd230350ce38e1/assets/tailwind.config.js

Then, the Dockerfile wasn't building.
I've started troubleshooting a bit and saw that the rel/ folder is not committed as well.
Again I've copied it from https://github.com/dwyl/phoenix-chat-example/tree/8e09af3c5d85fcdc4b31cfd0b5fd230350ce38e1/rel

Then the app wasn't starting, so I've tried commenting out the runner container in the Dockerfile to check how the app was being build, and for some reason when I run the server script it simply kills the app, no logs, only Killed.

I may have copied wrong stuff over, would you be able to push the missing files?

Also, I've done some work to have the /chat mapped to envoy, plus added postgres and a couple of env vars to be able to run the chat with docker compose, should I just push to your PR?

@joshleecreates
Copy link
Contributor Author

Hey @joshleecreates 👋🏽 , I've tried starting this up to see what I could collaborate, but got some bumps on the road.

1st the tailwind.config.js wasn't committed, I've copied the one from here https://github.com/dwyl/phoenix-chat-example/blob/8e09af3c5d85fcdc4b31cfd0b5fd230350ce38e1/assets/tailwind.config.js

Then, the Dockerfile wasn't building. I've started troubleshooting a bit and saw that the rel/ folder is not committed as well. Again I've copied it from https://github.com/dwyl/phoenix-chat-example/tree/8e09af3c5d85fcdc4b31cfd0b5fd230350ce38e1/rel

Then the app wasn't starting, so I've tried commenting out the runner container in the Dockerfile to check how the app was being build, and for some reason when I run the server script it simply kills the app, no logs, only Killed.

I may have copied wrong stuff over, would you be able to push the missing files?

Also, I've done some work to have the /chat mapped to envoy, plus added postgres and a couple of env vars to be able to run the chat with docker compose, should I just push to your PR?

Awesome, thank you for trying it out!

I'll upload the missing files. The tailwind stuff shouldn't be needed so I'll just remove those references. I'm not sure how I left out rel/ 🤪

My plan was to squash some commits / rebase before opening this, let me do that today before you add your commit if thats easier?

@julianocosta89
Copy link
Member

sounds good!
regarding tailwind, I've just added it because the app asked for it.
I initially commented out the COPY /rel step in the Dockerfile, but the app asked to add it, so I uncommented and followed the suggestions on the logs.

It is tricky to me because I'm not an Elixir/Erlang dev, so it is just a trial and error debug all the way 😅

This commit adds OpenTelemetry instrumentation to Phoenix:

1. Dependencies are added in mix.exs
2. Configuration options are added in config/config.exs
3. The OTel instrumentation libraries for Phoenix and ecto are started
with the application
genserver for persisting chats

simplify skeleton

macos nix

allow for listing of topics, more idiomatic

working POC of frontend with joining different channels, channel list

dockerfile for chatservice

remove HTML helper cruft
@joshleecreates
Copy link
Contributor Author

@julianocosta89 this currently has no way to create the schema in postgres via Docker. You need to somehow run mix ecto.create and mix ecto.migrate against the dev database in the src/chatservice/docker-compose.yml. Later today I can add the Postgres docker service with migrations.

@joshleecreates
Copy link
Contributor Author

(but feel free to add your commit in the meantime)

@julianocosta89
Copy link
Member

@joshleecreates I've added /chat to Envoy.
Now we are able to access the chat via localhost:8080/chat.

I didn't manage to get any further than the username page, but I could already see a trace:
Screenshot 2024-07-09 at 14 09 14

Copy link

github-actions bot commented Sep 4, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 4, 2024
@julianocosta89
Copy link
Member

Hey @joshleecreates sorry for taking that long to come back to you.
I've tried using it and I actually couldn't.

The chat page loaded fine, but when trying to connect to http://localhost:8080/chat?channel=juliano nothing happened.
I just got the login input again.

@joshleecreates
Copy link
Contributor Author

@julianocosta89 the way the front end is set up right now you still need to "login" even when connecting to a specific channel. The login doesn't really do anything other than set the username and if the channel is not set using the url it creates a channel based on the username.

@julianocosta89
Copy link
Member

That's what I'm not getting 😕

Here I have the url set to user1:
Screenshot 2024-09-05 at 08 45 02

But whenever I click to login, the url is overwritten with ? only:
Screenshot 2024-09-05 at 08 45 13

And if I try to open the user1 url again, it goes back to the login.

I'm never getting out of this page 😅

@joshleecreates
Copy link
Contributor Author

Bizarre. Are you free to pair on this at all? It's behaving as if it can't connect to the backend. Probably a better front end would have some error messages 😅

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Copy link

github-actions bot commented Oct 8, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helm-update-required Requires an update to the Helm chart when released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a new Elixir example service
2 participants