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

Fix instructions to uninstall the collector on Windows #1076

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

pjanotti
Copy link
Contributor

Requirements

  • Content follows Splunk guidelines for style and formatting.
  • You are contributing original content.

Describe the change

The alternative instructions are incorrect. Tested the updated ones on my machine.

The alternative instructions are incorrect. Tested the updated ones on my machine.
@pjanotti
Copy link
Contributor Author

@theletterf @RassK 🤔 we should remove the "nice" WMI instructions so there is no need for alternative instructions.

@theletterf theletterf added the stop-alert Stops Slack notification in #o11y-docs-feedback so that PR can be worked wo unnecessary noise label Nov 29, 2023
@RassK
Copy link
Contributor

RassK commented Nov 30, 2023

@pjanotti I'm wondering what was the intention originally with .uninstall(). Can't see any return types that have this method 🤷
https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.management/get-itemproperty?view=powershell-7.4

Copy link
Contributor

@theletterf theletterf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Paulo!

@theletterf theletterf merged commit 0b07350 into splunk:main Nov 30, 2023
2 checks passed
@RassK
Copy link
Contributor

RassK commented Nov 30, 2023

@pjanotti another idea. If you can transfer the "complex" uninstall to install.ps1?
install.ps1 -uninstall if the flag is set, then instead of running the main path, you invoke internal uninstall function? - so the docs can be more simpler containing only install.ps1 -uninstall

@pjanotti pjanotti deleted the patch-1 branch November 30, 2023 14:31
@pjanotti
Copy link
Contributor Author

@RassK I like the idea of uninstalling from the script since (I assume) the bundle with the install script is typically kept on the machine. The only concern that I have is that the script can install instrumentation (right now just .NET but we are adding others) and them the question becomes if it should support specific uninstall for collect, instrumentations, etc or just a global uninstall everything from the bundle. At first I think uninstall everything from the bundle seems reasonable.

@jeffreyc-splunk what do you think?

@jeffreyc-splunk
Copy link
Contributor

@RassK I like the idea of uninstalling from the script since (I assume) the bundle with the install script is typically kept on the machine. The only concern that I have is that the script can install instrumentation (right now just .NET but we are adding others) and them the question becomes if it should support specific uninstall for collect, instrumentations, etc or just a global uninstall everything from the bundle. At first I think uninstall everything from the bundle seems reasonable.

@jeffreyc-splunk what do you think?

  • Adding an uninstall option to the script is probably a good idea, since we already have that for our linux script.
  • Our current installation instructions doesn't save the installer script locally on the host:
    & {Set-ExecutionPolicy Bypass -Scope Process -Force; $script = ((New-Object System.Net.WebClient).DownloadString('https://dl.signalfx.com/splunk-otel-collector.ps1')); $params = @{access_token = "SPLUNK_ACCESS_TOKEN"; realm = "SPLUNK_REALM"}; Invoke-Command -ScriptBlock ([scriptblock]::Create(". {$script} $(&{$args} @params)"))}
    
    Assuming the user hasn't manually saved the script on the host, they'd have to do something like this after we add the uninstall option:
    & {Set-ExecutionPolicy Bypass -Scope Process -Force; $script = ((New-Object System.Net.WebClient).DownloadString('https://dl.signalfx.com/splunk-otel-collector.ps1')); $params = @{uninstall = 1}; Invoke-Command -ScriptBlock ([scriptblock]::Create(". {$script} $(&{$args} @params)"))}
    

@theletterf
Copy link
Contributor

Would having an uninstall.ps1 script go against DRY principles or similar? :-)

@pjanotti
Copy link
Contributor Author

pjanotti commented Dec 4, 2023

Would having an uninstall.ps1 script go against DRY principles or similar? :-)

I think the idea was install to have a single script with another option to allow the uninstallation. Notice that we don't save the script so I think it is better actually to keep just the latest instructions and remove the WMI ones (since WMI registration of installed programs is known to get corrupted).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external stop-alert Stops Slack notification in #o11y-docs-feedback so that PR can be worked wo unnecessary noise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants