Add uploaders of installation script(s) logs #213
Conversation
samiura
force-pushed
the
installation-logs-uploaders
branch
2 times, most recently
from
28a76e7 to
f83a9b2
Compare
samiura
force-pushed
the
installation-logs-uploaders
branch
from
f83a9b2 to
0c294be
Compare
install-script/install.ps1
Outdated
| # The API URL used to communicate with the SumoLogic backend | ||
| [string] $Api, | ||
|
|
||
| # DidsableIntallationTelemetry is used to disable reporting the installation |
There was a problem hiding this comment.
Typo: Didsable -> Disable
install-script/install.ps1
Outdated
| $InstallationLogFile = New-TemporaryFile | ||
|
|
||
| if ($InstallationLogFileEndpoint -eq "") { | ||
| ## https://sumologic.atlassian.net/wiki/spaces/MAIN/pages/1601341782/Production+Deployments+and+More#ProductionDeploymentsandMore-StagingDeployments |
There was a problem hiding this comment.
As much as this is useful for us, you should probably drop that comment since this is a public repo.
install-script/install.ps1
Outdated
| $errMsg += ": ${content}" | ||
| } | ||
|
|
||
| Write-Error -Message $errMsg |
There was a problem hiding this comment.
Is this not equivalent to Write-Error $errMsg?
Also, what happens when there is an error; e.g. have you tried running the script against a non-existent endpoint?
Since we're OK if we don't get those installation logs and we don't want to "scare" the user with an error here, maybe the best course of action is to drop Write-Error altogether?
There was a problem hiding this comment.
Oh I see the point. I was thinking halting was the cause of customer panic. Okay, we just have to use something else all together/or as you suggested remove. Thinking about using Write-Verbose but I guess removing is the best option as you suggested.
install-script/install.ps1
Outdated
| [string] $Api, | ||
|
|
||
| # DidsableIntallationTelemetry is used to disable reporting the installation | ||
| # DisableIntallationTelemetry is used to disable reporting the installation |
There was a problem hiding this comment.
typo: DisableInstallationTelemetry
There was a problem hiding this comment.
Already addressed in the latest commit. #213 (comment)
There was a problem hiding this comment.
in the PR: DisableIntallationTelemetry (installation is missing an S)
samiura
force-pushed
the
installation-logs-uploaders
branch
from
ab7a2dd to
ea5eb46
Compare
chan-tim-sumo
left a comment
chan-tim-sumo
left a comment
There was a problem hiding this comment.
looks good overall :D few comments
| Write-Error $_.Exception.InnerException.Message -ErrorAction Stop | ||
| } finally { | ||
| Stop-Transcript | Out-Null | ||
| if ($InstallationToken -eq $true) { |
There was a problem hiding this comment.
not too familiar with powershell, i'm assuming this line means if installation token equals to true?
is installation token a boolean?
| if (!($response.IsSuccessStatusCode)) { | ||
| $statusCode = [int]$response.StatusCode | ||
| $reasonPhrase = $response.StatusCode.ToString() | ||
| $errMsg = "${statusCode} ${reasonPhrase}" |
There was a problem hiding this comment.
will this continue to run even after the error message pops up? should the error message get thrown? i saw write-error on line 408:
Write-Error "Installation token has not been provided." -ErrorAction Stop
| # shellcheck disable=SC2317,SC2119,SC2120,SC2317,SC2329,SC1072,SC1073,SC1125 | ||
| function reporter { | ||
| if ! $DISABLE_INSTALLATION_TELEMETRY; then | ||
| echo "SUMOLOGIC_INSTALLATION_TOKEN=${SUMOLOGIC_INSTALLATION_TOKEN}" >> "$INSTALLATION_LOGFILE" |
There was a problem hiding this comment.
is writing the installation token to the log file to /tmp safe?
i saw in the repo
function set_tmpdir() {
# generate a new tmpdir using mktemp
# need to specify the template for some MacOS versions
TMPDIR=$(mktemp -d -t 'sumologic-otel-collector-XXXX')
}
i was looking at what mktemp is, and it creates a secure temporary file. would using something like that be better?
Added logs uploader for installation scripts and made sure they do not interrupt the installation process even in case of endpoint failure(s). This PR is based on
https://github.com/SumoLogic/sumologic-otel-collector-packaging/pull/64/files#diff-fa1bbdf24117b058c9042b791f3279c4cd60ecfac8af20e23f702b98c5ad6376