-
Notifications
You must be signed in to change notification settings - Fork 19
Add monitoring to devcontainer startup #268
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
base: master
Are you sure you want to change the base?
Conversation
ea38b47 to
758981e
Compare
|
|
||
|
|
||
| case "${SERVER}" in | ||
| "dev-stable") echo "https://workbench-dev.verily.com/api/${SERVER}" ;; |
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.
Should these be
| "dev-stable") echo "https://workbench-dev.verily.com/api/${SERVER}" ;; | |
| "dev-stable") echo "https://workbench-dev.verily.com/api/${1}" ;; |
|
|
||
| MONITORING_UTILS_FILE="/home/core/monitoring-utils.sh" | ||
| FIRST_BOOT_START_FILE="/home/core/first-boot-start" | ||
| if [[ -f "${MONITORING_UTILS_FILE}" && ! -f "${FIRST_BOOT_START_FILE}" ]]; then |
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.
Just curious, do we expect cases where the monitoring-utils file doesn't exist?
| function get_service_url() { | ||
| SERVER="$(get_metadata_value "terra-cli-server" "")" | ||
| if [[ -z "${SERVER}" ]]; then | ||
| SERVER="dev-stable" |
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.
Should this be prod?
|
|
||
| source /home/core/service-utils.sh | ||
|
|
||
| WORKSPACE_ID="$(get_metadata_value "terra-workspace-id" "")" |
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.
source metadata-util
| && docker ps -q --filter "name=application-server" | grep -q .; then | ||
| echo "Proxy is ready." | ||
| status="$(get_guest_attribute "startup_script/status" "")" | ||
| success=0 |
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.
nit: status_code since it's not just success
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.
so 0 is success and 1 is failure?
i'm little confused because devcontainer start is passing in success:1
| payload=$(cat <<EOF | ||
| { | ||
| "state": "DEVCONTAINER_END", | ||
| "success": ${SUCCESS} |
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.
is it too late to change this to status too?
| echo "VM state recorded successfully: ${response_body}" | ||
| } | ||
|
|
||
| function record_devcontainer_start() { |
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.
can we refactor the two method to calls function log_event() and pass in payload?
| payload=$(cat <<EOF | ||
| { | ||
| "state": "DEVCONTAINER_START", | ||
| "success": 1 |
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.
In the openapi in your add endpoint PR this is a boolean, will it map correctly?
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.
Haven't been able to test this locally yet, will be checking that this works properly!
| # First boot file does not exist | ||
| # Record startup begin for monitoring | ||
| source "${MONITORING_UTILS_FILE}" | ||
| record_devcontainer_start "${CLOUD_PLATFORM}" |
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.
does it take in CLOUD_PLATFORM?
No description provided.