Merge pull request #316 from appsignal/unset-env-variables

🐞 Underscored environment variables are always overwritten
appsignal · Mar 19, 2018 · 6149b79 · 6149b79
2 parents 1f37ef3 + 6a85230
commit 6149b79
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 70 deletions.
diff --git a/lib/appsignal/config.ex b/lib/appsignal/config.ex
@@ -9,7 +9,7 @@ defmodule Appsignal.Config do
     endpoint: "https://push.appsignal.com",
     diagnose_endpoint: "https://appsignal.com/diag",
     env: :dev,
-    filter_parameters: nil,
+    filter_parameters: [],
     ignore_actions: [],
     ignore_errors: [],
     ignore_namespaces: [],
@@ -177,70 +177,28 @@ defmodule Appsignal.Config do
 
     System.put_env("_APPSIGNAL_ACTIVE", to_string(config[:active]))
     System.put_env("_APPSIGNAL_AGENT_PATH", List.to_string(:code.priv_dir(:appsignal)))
-    # FIXME - app_path should not be necessary
-    System.put_env("_APPSIGNAL_APP_PATH", List.to_string(:code.priv_dir(:appsignal)))
-
-    unless empty?(config[:name]) do
-      System.put_env("_APPSIGNAL_APP_NAME", app_name_to_string(config[:name]))
-    end
-
-    unless empty?(config[:ca_file_path]) do
-      System.put_env("_APPSIGNAL_CA_FILE_PATH", config[:ca_file_path])
-    end
-
+    System.put_env("_APPSIGNAL_APP_PATH", List.to_string(:code.priv_dir(:appsignal))) # FIXME - app_path should not be necessary
+    System.put_env("_APPSIGNAL_APP_NAME", to_string(config[:name]))
+    System.put_env("_APPSIGNAL_CA_FILE_PATH", to_string(config[:ca_file_path]))
     System.put_env("_APPSIGNAL_DEBUG_LOGGING", to_string(config[:debug]))
-
-    unless empty?(config[:dns_servers]) do
-      System.put_env("_APPSIGNAL_DNS_SERVERS", config[:dns_servers] |> Enum.join(","))
-    end
-
+    System.put_env("_APPSIGNAL_DNS_SERVERS", config[:dns_servers] |> Enum.join(","))
     System.put_env("_APPSIGNAL_ENABLE_HOST_METRICS", to_string(config[:enable_host_metrics]))
     System.put_env("_APPSIGNAL_ENVIRONMENT", to_string(config[:env]))
-
-    unless empty?(config[:filter_parameters]) do
-      System.put_env("_APPSIGNAL_FILTER_PARAMETERS", config[:filter_parameters] |> Enum.join(","))
-    end
-
+    System.put_env("_APPSIGNAL_FILTER_PARAMETERS", config[:filter_parameters] |> Enum.join(","))
     System.put_env("_APPSIGNAL_HOSTNAME", config[:hostname])
-
-    unless empty?(config[:http_proxy]) do
-      System.put_env("_APPSIGNAL_HTTP_PROXY", config[:http_proxy])
-    end
-
+    System.put_env("_APPSIGNAL_HTTP_PROXY", to_string(config[:http_proxy]))
     System.put_env("_APPSIGNAL_IGNORE_ACTIONS", config[:ignore_actions] |> Enum.join(","))
     System.put_env("_APPSIGNAL_IGNORE_ERRORS", config[:ignore_errors] |> Enum.join(","))
     System.put_env("_APPSIGNAL_IGNORE_NAMESPACES", config[:ignore_namespaces] |> Enum.join(","))
-
-    System.put_env(
-      "_APPSIGNAL_LANGUAGE_INTEGRATION_VERSION",
-      "elixir-" <> @language_integration_version
-    )
-
+    System.put_env("_APPSIGNAL_LANGUAGE_INTEGRATION_VERSION", "elixir-" <> @language_integration_version)
     System.put_env("_APPSIGNAL_LOG", config[:log])
-
-    unless empty?(config[:log_path]) do
-      System.put_env("_APPSIGNAL_LOG_FILE_PATH", config[:log_path])
-    end
-
+    System.put_env("_APPSIGNAL_LOG_FILE_PATH", to_string(config[:log_path]))
     System.put_env("_APPSIGNAL_PUSH_API_ENDPOINT", config[:endpoint] || "")
     System.put_env("_APPSIGNAL_PUSH_API_KEY", config[:push_api_key] || "")
-
-    unless empty?(config[:running_in_container]) do
-      System.put_env("_APPSIGNAL_RUNNING_IN_CONTAINER", to_string(config[:running_in_container]))
-    end
-
+    System.put_env("_APPSIGNAL_RUNNING_IN_CONTAINER", to_string(config[:running_in_container]))
     System.put_env("_APPSIGNAL_SEND_PARAMS", to_string(config[:send_params]))
-
-    unless empty?(config[:working_dir_path]) do
-      System.put_env("_APPSIGNAL_WORKING_DIR_PATH", config[:working_dir_path])
-    end
-
-    unless empty?(config[:files_world_accessible]) do
-      System.put_env(
-        "_APPSIGNAL_FILES_WORLD_ACCESSIBLE",
-        to_string(config[:files_world_accessible])
-      )
-    end
+    System.put_env("_APPSIGNAL_WORKING_DIR_PATH", to_string(config[:working_dir_path]))
+    System.put_env("_APPSIGNAL_FILES_WORLD_ACCESSIBLE", to_string(config[:files_world_accessible]))
   end
 
   @doc """
@@ -259,9 +217,6 @@ defmodule Appsignal.Config do
     end)
   end
 
-  defp app_name_to_string(name) when is_atom(name), do: Atom.to_string(name)
-  defp app_name_to_string(name) when is_binary(name), do: name
-
   def get_system_env do
     System.get_env()
     |> Enum.filter(fn

diff --git a/test/appsignal/config_test.exs b/test/appsignal/config_test.exs
@@ -412,18 +412,18 @@ defmodule Appsignal.ConfigTest do
       Appsignal.Config.write_to_environment
     end
 
-    test "empty config options don't get written to the env" do
+    test "empty config options get written to the env" do
       write_to_environment()
-      assert System.get_env("_APPSIGNAL_APP_NAME") == nil
-      assert System.get_env("_APPSIGNAL_CA_FILE_PATH") == nil
-      assert System.get_env("_APPSIGNAL_FILTER_PARAMETERS") == nil
-      assert System.get_env("_APPSIGNAL_HTTP_PROXY") == nil
+      assert System.get_env("_APPSIGNAL_APP_NAME") == ""
+      assert System.get_env("_APPSIGNAL_CA_FILE_PATH") == ""
+      assert System.get_env("_APPSIGNAL_FILTER_PARAMETERS") == ""
+      assert System.get_env("_APPSIGNAL_HTTP_PROXY") == ""
       assert System.get_env("_APPSIGNAL_IGNORE_ACTIONS") == ""
       assert System.get_env("_APPSIGNAL_IGNORE_ERRORS") == ""
       assert System.get_env("_APPSIGNAL_IGNORE_NAMESPACES") == ""
-      assert System.get_env("_APPSIGNAL_LOG_FILE_PATH") == nil
-      assert System.get_env("_APPSIGNAL_WORKING_DIR_PATH") == nil
-      assert System.get_env("_APPSIGNAL_RUNNING_IN_CONTAINER") == nil
+      assert System.get_env("_APPSIGNAL_LOG_FILE_PATH") == ""
+      assert System.get_env("_APPSIGNAL_WORKING_DIR_PATH") == ""
+      assert System.get_env("_APPSIGNAL_RUNNING_IN_CONTAINER") == ""
     end
 
     test "deletes existing configuration in environment" do
@@ -435,7 +435,7 @@ defmodule Appsignal.ConfigTest do
           with_config(%{name: ""}, fn() ->
             write_to_environment()
             # So it doesn't get written to the new agent environment configuration
-            assert System.get_env("_APPSIGNAL_APP_NAME")  == nil
+            assert System.get_env("_APPSIGNAL_APP_NAME") == ""
           end)
       end)
     end
@@ -508,10 +508,10 @@ defmodule Appsignal.ConfigTest do
       end)
     end
 
-    test "does not write dns_servers to env if empty" do
+    test "writes dns_servers to env if empty" do
       with_config(%{dns_servers: []}, fn() ->
         write_to_environment()
-        assert System.get_env("_APPSIGNAL_DNS_SERVERS") == nil
+        assert System.get_env("_APPSIGNAL_DNS_SERVERS") == ""
       end)
     end
 
@@ -547,7 +547,7 @@ defmodule Appsignal.ConfigTest do
       endpoint: "https://push.appsignal.com",
       diagnose_endpoint: "https://appsignal.com/diag",
       env: :dev,
-      filter_parameters: nil,
+      filter_parameters: [],
       ignore_actions: [],
       ignore_errors: [],
       ignore_namespaces: [],

diff --git a/test/appsignal/release_upgrade_test.exs b/test/appsignal/release_upgrade_test.exs
@@ -42,7 +42,7 @@ defmodule Appsignal.ReleaseUpgradeTest do
       enable_host_metrics: true,
       endpoint: "https://push.appsignal.com",
       env: :dev,
-      filter_parameters: nil,
+      filter_parameters: [],
       hostname: "Alices-MBP.example.com",
       ignore_actions: [],
       ignore_errors: [],