-
Notifications
You must be signed in to change notification settings - Fork 206
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
Add telemetry support to docker-compose deployments #217
base: master
Are you sure you want to change the base?
Conversation
docker-compose-with-temporal.yml
Outdated
- | | ||
if [[ "$${RTEL_ENABLED:-false}" == "false" ]]; then | ||
echo 1>&2 "RTEL_ENABLED is not true, going to sleep." | ||
sleep inf |
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.
why not gracefully shut down at this point?
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.
because I need restart: on-failure
for the enabled case, but if the container exits when disabled (default) it will restart loop :/
docker-compose-with-temporal.yml
Outdated
exec retool-telemetry | ||
fi | ||
environment: | ||
RTEL_DEPLOYMENT_MODE: 'docker' |
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.
maybe: align this to https://github.com/tryretool/retool_development/blob/dev/backend/src/server/modules/jobsRunner/startupTelemetry.ts#L49-L62
DEPLOYMENT_TEMPLATE_TYPE
DEPLOYMENT_TEMPLATE_VERSION
in this case docker-compose
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.
ah yes, I need to address in the helm side too. will do.
networks: | ||
- backend-network |
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.
networks: | |
- backend-network | |
networks: | |
- backend-network | |
- code-executor-network |
@@ -4,5 +4,5 @@ if command -v docker-compose &> /dev/null ; then | |||
exit 0 | |||
fi | |||
|
|||
sudo -E curl -L https://github.com/docker/compose/releases/download/1.29.0/docker-compose-`uname -s`-`uname -m` -o /usr/local/bin/docker-compose | |||
sudo -E curl -L https://github.com/docker/compose/releases/download/v2.24.7/docker-compose-`uname -s`-`uname -m` -o /usr/local/bin/docker-compose |
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.
🙏
@@ -120,6 +124,45 @@ services: | |||
- ./retool:/usr/local/retool-git-repo | |||
- ${BOOTSTRAP_SOURCE:-./retool}:/usr/local/retool-repo | |||
|
|||
telemetry: |
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.
(same comments apply)
29a35dc
to
df7da2b
Compare
docker-compose-with-temporal.yml
Outdated
@@ -130,6 +135,45 @@ services: | |||
privileged: true | |||
restart: on-failure | |||
|
|||
telemetry: | |||
image: "telemetry:ra-dev-0" # TODO: change to public image |
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.
will hold off on deploying until these new images are published.
actually this makes me think... the telemetry image will be something like tryretool/telemetry:3.39.0-edge
, but the version will never get updated in here.
should we redo how versions and those local dockerfiles work here? now that code-executor images use the same version as main backend (and telemetry does too), and we have stable vs. edge releases, surely we can simplify. could we just pin these to the latest
tag (which now maps to latest stable)? and drop the local dockerfiles?
docker-compose-with-temporal.yml
Outdated
- | | ||
if [[ "$${RTEL_ENABLED:-false}" == "false" ]]; then | ||
echo 1>&2 "RTEL_ENABLED is not true, going to sleep." | ||
sleep inf |
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.
because I need restart: on-failure
for the enabled case, but if the container exits when disabled (default) it will restart loop :/
docker-compose-with-temporal.yml
Outdated
exec retool-telemetry | ||
fi | ||
environment: | ||
RTEL_DEPLOYMENT_MODE: 'docker' |
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.
ah yes, I need to address in the helm side too. will do.
df7da2b
to
74eb640
Compare
74eb640
to
d51b8c2
Compare
telemetry
container which just sleeps by default, but can be enabled by uncommenting a few lines in yourdocker.env
file../docker_setup
, mainly so the early exit happens before prompting for the domain, and to fix env var string quoting to make docker-compose 2.24 happy. The string quoting fix will only apply to fresh installations—my assumption is existing docker-compose deployments will likely not do the docker-compose upgrade, so their existing docker.env file will continue working unchanged.