Skip to content

Commit

Permalink
Add config/appsignal.rb config file
Browse files Browse the repository at this point in the history
Add `config/appsignal.rb` config file support. This is a replacement for
the `config/appsignal.yml` config file.

Load the `config/appsignal.rb` file automatically on `Appsignal.start`.
For most apps, like Rails apps, this will work the same way as
`config/appsignal.yml` does now.

If `Appsignal.configure` is called manually in the app, like a Rails
initializer, the `config/appsignal.rb` file is not loaded. Instead, we
recommend moving the `Appsignal.configure` call to the
`config/appsignal.rb` file.

## New behavior

With this change the config is loaded as described in these scenarios:

- Scenario 1:
  - `Appsignal.start` is called.
  - No config is loaded yet.
  - The `config/appsignal.rb` file is present.
  - The `config/appsignal.rb` is required and configures the gem.
- Scenario 2:
  - `Appsignal.start` is called.
  - No config is loaded yet.
  - The `config/appsignal.rb` file is present.
  - The `config/appsignal.yml` file is present.
  - The `config/appsignal.rb` is required and configures the gem using
    the `Appsignal.configure` helper.
  - The `config/appsignal.yml` is not loaded.
- Scenario 3:
  - `Appsignal.configure` is called.
  - `Appsignal.start` is called.
  - The `config/appsignal.rb` file is not required.
- Scenario 4:
  - `Appsignal.configure` is called.
  - The `config/appsignal.yml` file is present.
  - The `config/appsignal.yml` file is loaded.
  - `Appsignal.start` is called.
- Scenario 5:
  - `Appsignal.configure` is called.
  - The `config/appsignal.rb` file is present.
  - The `config/appsignal.rb` file is not loaded.
  - `Appsignal.start` is called.

This is more complex that I would have liked. In hindsight I shouldn't
have made `Appsignal.configure` load the `config/appsignal.yml` file.
That would have made it a bit more predictable. But I don't want
`Appsignal.configure` in the `config/appsignal.rb` file to also load the
`config/appsignal.yml` file, because then it just gets more confusing.

In the next major version I want to remove `config/appsignal.yml`
support, so this is the first step.

## App/root path

I remember some of our users configured the Config class's `root_path`
to specify where to load the config file from.
If an app configures itself via a `config/appsignal.rb`, this makes it
impossible to customize the root path. For that purpose I've added the
`APPSIGNAL_APP_PATH` environment variable to customize the config path.
Which I've also used to make it easier to test the different scenarios.

---

Part of #1324
  • Loading branch information
tombruijn committed Nov 4, 2024
1 parent ff31be8 commit 41f9e9d
Show file tree
Hide file tree
Showing 6 changed files with 371 additions and 8 deletions.
19 changes: 19 additions & 0 deletions .changesets/add-config-appsignal-rb-file-support.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
bump: minor
type: add
---

Add `config/appsignal.rb` config file support. When a `config/appsignal.rb` file is present in the app, the Ruby gem will automatically load it when `Appsignal.start` is called.

The `config/appsignal.rb` config file is a replacement for the `config/appsignal.yml` config file. When both files are present, only the `config/appsignal.rb` config file is loaded.

Example `config/appsignal.rb` config file:

```ruby
# config/appsignal.rb
Appsignal.configure do |config|
config.name = "My app name"
end
```

It is not possible to configure multiple environments in the `config/appsignal.rb` config file as it was possible in the YAML version.
85 changes: 80 additions & 5 deletions lib/appsignal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def testing?
#
# @return [void]
# @since 0.7.0
def start # rubocop:disable Metrics/AbcSize
def start # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/CyclomaticComplexity
if ENV.fetch("_APPSIGNAL_DIAGNOSE", false)
internal_logger.warn("Skipping start in diagnose context")
return
Expand All @@ -103,19 +103,49 @@ def start # rubocop:disable Metrics/AbcSize
return
end

if config_file_context?
internal_logger.warn(
"Ignoring call to Appsignal.start in config file context."
)
return
end

unless extension_loaded?
internal_logger.info("Not starting AppSignal, extension is not loaded")
return
end

internal_logger.debug("Loading AppSignal gem")

@config ||= Config.new(Config.determine_root_path, Config.determine_env)
@config.validate
context = Appsignal::Config::Context.new(
:env => Config.determine_env,
:root_path => Config.determine_root_path
)
# If there's a config/appsignal.rb file
if context.dsl_config_file?
if config
# Do not load it if a config is already present
internal_logger.warn(
"Ignoring `#{context.dsl_config_file}` file because " \
"`Appsignal.configure` has already been called."
)
else
# Load it when no config is present
load_dsl_config_file(context.dsl_config_file)
# Disable AppSignal on config file error
config[:active] = false if config && config_file_error?
end
else
# Load config if no config file was found and no config is present yet
# This will load the config/appsignal.yml file automatically
@config ||= Config.new(context.root_path, context.env)
end
# Validate the config, if present
config&.validate

_start_logger

if config.valid?
if config&.valid?
if config.active?
@started = true
internal_logger.info "Starting AppSignal #{Appsignal::VERSION} " \
Expand Down Expand Up @@ -244,7 +274,12 @@ def configure(env_param = nil, root_path: nil)
else
@config = Config.new(
root_path_param || Config.determine_root_path,
Config.determine_env(env_param)
Config.determine_env(env_param),
# If in the context of an `config/appsignal.rb` config file, do not
# load the `config/appsignal.yml` file.
# The `.rb` file is a replacement for the `.yml` file so it shouldn't
# load both.
:load_yaml_file => !config_file_context?
)
end

Expand Down Expand Up @@ -397,6 +432,11 @@ def active?
config&.active? && extension_loaded?
end

# @api private
def dsl_config_file_loaded?
defined?(@dsl_config_file_loaded) ? true : false
end

private

def params_match_loaded_config?(env_param, root_path_param)
Expand All @@ -408,6 +448,41 @@ def params_match_loaded_config?(env_param, root_path_param)
(root_path_param.nil? || config.root_path == root_path_param)
end

# Load the `config/appsignal.rb` config file, if present.
#
# If the config file has already been loaded once and it's trying to be
# loaded more than once, which should never happen, it will not do
# anything.
def load_dsl_config_file(path)
return if defined?(@dsl_config_file_loaded)

ENV["_APPSIGNAL_CONFIG_FILE_CONTEXT"] = "true"
@dsl_config_file_loaded = true
require path
rescue => error
@config_file_error = error
message = "Not starting AppSignal because an error occurred while " \
"loading the AppSignal config file.\n" \
"File: #{path.inspect}\n" \
"#{error.class.name}: #{error}"
Kernel.warn "appsignal ERROR: #{message}"
internal_logger.error "#{message}\n#{error.backtrace.join("\n")}"
ensure
ENV.delete("_APPSIGNAL_CONFIG_FILE_CONTEXT")
end

# Returns true if we're currently in the `config/appsignal.rb` file
# context.
def config_file_context?
ENV.fetch("_APPSIGNAL_CONFIG_FILE_CONTEXT", nil) == "true"
end

# Returns true if loading the `config/appsignal.rb` file resulted in an
# error.
def config_file_error?
defined?(@config_file_error) ? true : false
end

def start_internal_stdout_logger
@internal_logger = Appsignal::Utils::IntegrationLogger.new($stdout)
internal_logger.formatter = log_formatter("appsignal")
Expand Down
33 changes: 30 additions & 3 deletions lib/appsignal/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ def self.determine_env(initial_env = nil)
# Determine which root path AppSignal should initialize with.
# @api private
def self.determine_root_path
app_path_env_var = ENV.fetch("APPSIGNAL_APP_PATH", nil)
return app_path_env_var if app_path_env_var

loader_defaults.reverse.each do |loader_defaults|
root_path = loader_defaults[:root_path]
return root_path if root_path
Expand All @@ -61,6 +64,26 @@ def self.determine_root_path
Dir.pwd
end

# @api private
class Context
DSL_FILENAME = "config/appsignal.rb"

attr_reader :env, :root_path

def initialize(env: nil, root_path: nil)
@env = env
@root_path = root_path
end

def dsl_config_file
File.join(root_path, DSL_FILENAME)
end

def dsl_config_file?
File.exist?(dsl_config_file)
end
end

# @api private
DEFAULT_CONFIG = {
:activejob_report_errors => "all",
Expand Down Expand Up @@ -213,8 +236,10 @@ def self.determine_root_path
# How to integrate AppSignal manually
def initialize(
root_path,
env
env,
load_yaml_file: true
)
@load_yaml_file = load_yaml_file
@root_path = root_path.to_s
@config_file_error = false
@config_file = config_file
Expand Down Expand Up @@ -269,8 +294,10 @@ def load_config
@initial_config[:env] = @env

# Load the config file if it exists
@file_config = load_from_disk || {}
merge(file_config)
if @load_yaml_file
@file_config = load_from_disk || {}
merge(file_config)
end

# Load config from environment variables
@env_config = load_from_environment
Expand Down
Loading

0 comments on commit 41f9e9d

Please sign in to comment.