Add mirrors to download the agent #671
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ruby integration had mirrors defined to download the agent from different places. Now the Elixir integration has an extensible mirror list to download the agent as well.
3 tasks
thijsc
approved these changes
Sep 1, 2021
tombruijn
reviewed
Sep 6, 2021
| "https://appsignal-agent-releases.global.ssl.fastly.net", | ||
| "https://d135dj0rjqvssy.cloudfront.net", | ||
| ] | ||
| end |
There was a problem hiding this comment.
@luismiramirez have we updated the appsignal-agent's ship task to add these mirrors to the agent.exs template?
There was a problem hiding this comment.
Discussed with team in call. I'll pick up this change.
tombruijn
added a commit
that referenced
this pull request
Sep 13, 2021
In PR #671 we've added support for agent & extension downloads from mirrors. An edge case popped up that failed the installation. This scenario happens when the installer fetches things from the tmp directory. When the installer finds a matching agent & extension download in the host's `/tmp` directory it will use that archive rather than download it again. When it uses this installation method it does not store a `download_url` on the `download.report` since PR #671, because it doesn't know which mirror was originally used to download the archive. When writing the download report it would error on the missing `download_url` key. ``` could not compile dependency :appsignal, "mix compile" failed. You can recompile this dependency with "mix deps.compile appsignal", update it with "mix deps.update appsignal" or clean it with "mix deps.clean appsignal" ** (KeyError) key :download_url not found in: %{checksum: "verified"} mix_helpers.exs:512: Mix.Appsignal.Helper.write_download_report/1 mix_helpers.exs:495: Mix.Appsignal.Helper.write_report/1 mix_helpers.exs:248: Mix.Appsignal.Helper.compile/1 mix_helpers.exs:27: Mix.Appsignal.Helper.install/0 /integration/appsignal-elixir/mix.exs:6: Mix.Tasks.Compile.Appsignal.run/1 (mix 1.12.2) lib/mix/task.ex:394: anonymous fn/3 in Mix.Task.run_task/3 (mix 1.12.2) lib/mix/tasks/compile.all.ex:92: Mix.Tasks.Compile.All.run_compiler/2 (mix 1.12.2) lib/mix/tasks/compile.all.ex:72: Mix.Tasks.Compile.All.compile/4 ``` This change fixes that by making the `download_url` key optional, it will default to `nil` when the key is not found. This way it won't error in this edge case.
tombruijn
added a commit
that referenced
this pull request
Sep 13, 2021
In PR #671 we've added support for agent & extension downloads from mirrors. An edge case popped up that failed the installation. This scenario happens when the installer fetches things from the tmp directory. When the installer finds a matching agent & extension download in the host's `/tmp` directory it will use that archive rather than download it again. When it uses this installation method it does not store a `download_url` on the `download.report` since PR #671, because it doesn't know which mirror was originally used to download the archive. When writing the download report it would error on the missing `download_url` key. ``` could not compile dependency :appsignal, "mix compile" failed. You can recompile this dependency with "mix deps.compile appsignal", update it with "mix deps.update appsignal" or clean it with "mix deps.clean appsignal" ** (KeyError) key :download_url not found in: %{checksum: "verified"} mix_helpers.exs:512: Mix.Appsignal.Helper.write_download_report/1 mix_helpers.exs:495: Mix.Appsignal.Helper.write_report/1 mix_helpers.exs:248: Mix.Appsignal.Helper.compile/1 mix_helpers.exs:27: Mix.Appsignal.Helper.install/0 /integration/appsignal-elixir/mix.exs:6: Mix.Tasks.Compile.Appsignal.run/1 (mix 1.12.2) lib/mix/task.ex:394: anonymous fn/3 in Mix.Task.run_task/3 (mix 1.12.2) lib/mix/tasks/compile.all.ex:92: Mix.Tasks.Compile.All.run_compiler/2 (mix 1.12.2) lib/mix/tasks/compile.all.ex:72: Mix.Tasks.Compile.All.compile/4 ``` This change fixes that by making the `download_url` key optional, it will default to `nil` when the key is not found. This way it won't error in this edge case.
2 tasks
tombruijn
added a commit
that referenced
this pull request
Oct 12, 2021
When installer can't download the install package from the fastly mirror it falls back on the cloudfront mirror since PR #671. On newer OTP versions the cloudfront mirror fails to download the error message `{:error, :closed}` from hackney, which isn't very descriptive. After some searching around and trying out different things, I found that removing the `ciphers` option made the cloudfront mirror download work. After some more research with the help of @luismiramirez we found this blog post: https://nts.strzibny.name/hackney-ssl-configuration/ that lists the options I decided to implement in this commit. Added to the list of versions is also `tlsv1.3` as was previously configured in the ciphers options as well. This `versions` option seems like a simpler way to configure these ciphers on newer OTP versions, than calling `:ssl.cipher_suites` with different arguments for different OTP versions. This method doesn't work on older OTP versions, which is why the original implementation was kept in tact for OTP 22 and lower. By passing in the `ssl_options` to hackney's `request` function we override the OTP defaults for SSL, which we then need to set again for the specific OTP version. I've updated the mix helper and transmitter to both use the same config, and use the same config for all OTP versions. I've testing this on the CI with both the fastly and cloudfront mirror as the download and they both pass. This commit itself doesn't test the cloudfront mirror, because it's not the first mirror used and the first mirror doesn't fail to download. In the PR for this commit I'll link a CI build that does only use the cloudfront mirror for testing.
tombruijn
added a commit
that referenced
this pull request
Oct 12, 2021
When installer can't download the install package from the fastly mirror it falls back on the cloudfront mirror since PR #671. On newer OTP versions the cloudfront mirror fails to download the error message `{:error, :closed}` from hackney, which isn't very descriptive. After some searching around and trying out different things, I found that removing the `ciphers` option made the cloudfront mirror download work. After some more research with the help of @luismiramirez we found this blog post: https://nts.strzibny.name/hackney-ssl-configuration/ that lists the options I decided to implement in this commit. Added to the list of versions is also `tlsv1.3` as was previously configured in the ciphers options as well. This `versions` option seems like a simpler way to configure these ciphers on newer OTP versions, than calling `:ssl.cipher_suites` with different arguments for different OTP versions. This method doesn't work on older OTP versions, which is why the original implementation was kept in tact for OTP 22 and lower. By passing in the `ssl_options` to hackney's `request` function we override the OTP defaults for SSL, which we then need to set again for the specific OTP version. I've updated the mix helper and transmitter to both use the same config, and use the same config for all OTP versions. I've testing this on the CI with both the fastly and cloudfront mirror as the download and they both pass. This commit itself doesn't test the cloudfront mirror, because it's not the first mirror used and the first mirror doesn't fail to download. In the PR for this commit I'll link a CI build that does only use the cloudfront mirror for testing. Part of #683
tombruijn
added a commit
that referenced
this pull request
Oct 12, 2021
When installer can't download the install package from the fastly mirror it falls back on the cloudfront mirror since PR #671. On newer OTP versions the cloudfront mirror fails to download the error message `{:error, :closed}` from hackney, which isn't very descriptive. After some searching around and trying out different things, I found that removing the `ciphers` option made the cloudfront mirror download work. After some more research with the help of @luismiramirez we found this blog post: https://nts.strzibny.name/hackney-ssl-configuration/ that lists the options I decided to implement in this commit. Added to the list of versions is also `tlsv1.3` as was previously configured in the ciphers options as well. This `versions` option seems like a simpler way to configure these ciphers on newer OTP versions, than calling `:ssl.cipher_suites` with different arguments for different OTP versions. This method doesn't work on older OTP versions, which is why the original implementation was kept in tact for OTP 22 and lower. By passing in the `ssl_options` to hackney's `request` function we override the OTP defaults for SSL, which we then need to set again for the specific OTP version. I've updated the mix helper and transmitter to both use the same config, and use the same config for all OTP versions. I've testing this on the CI with both the fastly and cloudfront mirror as the download and they both pass. This commit itself doesn't test the cloudfront mirror, because it's not the first mirror used and the first mirror doesn't fail to download. In the PR for this commit I'll link a CI build that does only use the cloudfront mirror for testing. Part of #683
tombruijn
added a commit
that referenced
this pull request
Oct 13, 2021
When installer can't download the install package from the fastly mirror it falls back on the cloudfront mirror since PR #671. On newer OTP versions the cloudfront mirror fails to download the error message `{:error, :closed}` from hackney, which isn't very descriptive. After some searching around and trying out different things, I found that removing the `ciphers` option made the cloudfront mirror download work. After some more research with the help of @luismiramirez we found this blog post: https://nts.strzibny.name/hackney-ssl-configuration/ that lists the options I decided to implement in this commit. Added to the list of versions is also `tlsv1.3` as was previously configured in the ciphers options as well. This `versions` option seems like a simpler way to configure these ciphers on newer OTP versions, than calling `:ssl.cipher_suites` with different arguments for different OTP versions. This method doesn't work on older OTP versions, which is why the original implementation was kept in tact for OTP 22 and lower. By passing in the `ssl_options` to hackney's `request` function we override the OTP defaults for SSL, which we then need to set again for the specific OTP version. I've updated the mix helper and transmitter to both use the same config, and use the same config for all OTP versions. I've testing this on the CI with both the fastly and cloudfront mirror as the download and they both pass. This commit itself doesn't test the cloudfront mirror, because it's not the first mirror used and the first mirror doesn't fail to download. In the PR for this commit I'll link a CI build that does only use the cloudfront mirror for testing. Part of #683
jeffkreeftmeijer
pushed a commit
that referenced
this pull request
Oct 18, 2021
In PR #671 we've added support for agent & extension downloads from mirrors. An edge case popped up that failed the installation. This scenario happens when the installer fetches things from the tmp directory. When the installer finds a matching agent & extension download in the host's `/tmp` directory it will use that archive rather than download it again. When it uses this installation method it does not store a `download_url` on the `download.report` since PR #671, because it doesn't know which mirror was originally used to download the archive. When writing the download report it would error on the missing `download_url` key. ``` could not compile dependency :appsignal, "mix compile" failed. You can recompile this dependency with "mix deps.compile appsignal", update it with "mix deps.update appsignal" or clean it with "mix deps.clean appsignal" ** (KeyError) key :download_url not found in: %{checksum: "verified"} mix_helpers.exs:512: Mix.Appsignal.Helper.write_download_report/1 mix_helpers.exs:495: Mix.Appsignal.Helper.write_report/1 mix_helpers.exs:248: Mix.Appsignal.Helper.compile/1 mix_helpers.exs:27: Mix.Appsignal.Helper.install/0 /integration/appsignal-elixir/mix.exs:6: Mix.Tasks.Compile.Appsignal.run/1 (mix 1.12.2) lib/mix/task.ex:394: anonymous fn/3 in Mix.Task.run_task/3 (mix 1.12.2) lib/mix/tasks/compile.all.ex:92: Mix.Tasks.Compile.All.run_compiler/2 (mix 1.12.2) lib/mix/tasks/compile.all.ex:72: Mix.Tasks.Compile.All.compile/4 ``` This change fixes that by making the `download_url` key optional, it will default to `nil` when the key is not found. This way it won't error in this edge case.
jeffkreeftmeijer
pushed a commit
that referenced
this pull request
Oct 18, 2021
When installer can't download the install package from the fastly mirror it falls back on the cloudfront mirror since PR #671. On newer OTP versions the cloudfront mirror fails to download the error message `{:error, :closed}` from hackney, which isn't very descriptive. After some searching around and trying out different things, I found that removing the `ciphers` option made the cloudfront mirror download work. After some more research with the help of @luismiramirez we found this blog post: https://nts.strzibny.name/hackney-ssl-configuration/ that lists the options I decided to implement in this commit. Added to the list of versions is also `tlsv1.3` as was previously configured in the ciphers options as well. This `versions` option seems like a simpler way to configure these ciphers on newer OTP versions, than calling `:ssl.cipher_suites` with different arguments for different OTP versions. This method doesn't work on older OTP versions, which is why the original implementation was kept in tact for OTP 22 and lower. By passing in the `ssl_options` to hackney's `request` function we override the OTP defaults for SSL, which we then need to set again for the specific OTP version. I've updated the mix helper and transmitter to both use the same config, and use the same config for all OTP versions. I've testing this on the CI with both the fastly and cloudfront mirror as the download and they both pass. This commit itself doesn't test the cloudfront mirror, because it's not the first mirror used and the first mirror doesn't fail to download. In the PR for this commit I'll link a CI build that does only use the cloudfront mirror for testing. Part of #683
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ruby integration had mirrors defined to download the agent from
different places. Now the Elixir integration has an extensible mirror
list to download the agent as well.