[ps-installation-script] better error message in case of failed instrumentation installer download #6165
Conversation
| @@ -456,7 +456,17 @@ if ($with_dotnet_instrumentation) { | |||
| $download = "https://github.com/signalfx/splunk-otel-dotnet/releases/latest/download/$module_name" | |||
| $dotnet_autoinstr_path = Join-Path $tempdir $module_name | |||
| echo "Downloading .NET Instrumentation installer ..." | |||
| Invoke-WebRequest -Uri $download -OutFile $dotnet_autoinstr_path -UseBasicParsing | |||
| try { | |||
| [Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 | |||
There was a problem hiding this comment.
This should do the trick, may need to add comment that 3072 is TLS 1.2 and 12288 is TLS 1.3
[Net.ServicePointManager]::SecurityProtocol = 3072 -bor 12288
There was a problem hiding this comment.
I considered that but decided it'd be preferable to:
- set it consistently in the script (as similar is configured in multiplace places already)
- use enum values instead of magic numbers e.g.
[Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12 -bor [Net.SecurityProtocolType]::Tls13
There was a problem hiding this comment.
Makes sense to separate it to a new issue then, if there are multiple places. The idea is not to require older security protocols but instead use newest as possible and fallback if not possible.
[Net.SecurityProtocolType]::Tls13 is available in .NET FX 4.8+. I think PowerShell 5.1 what we require (min) is still at 4.5. So the enum value is not available there.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6165 +/- ##
=======================================
Coverage 44.49% 44.49%
=======================================
Files 391 391
Lines 27034 27034
=======================================
Hits 12029 12029
Misses 14165 14165
Partials 840 840 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lachmatt
deleted the
better-error-message-for-failed-net-instr-downloads
branch
Description:
Produce a better message in case of failed attempt to download .NET instrumentation installer.
Follows the same pattern as other download attempts in the script (e.g. 1).
Additionally, sets the security protocol (which might be needed on a legacy systems) similarly to other downloads in the script (1,2).