Integration configuration settings

[delner]

In order to encourage more flexible configuration design in our integrations, this pull request introduces a common core for integration configuration settings. This allows us to improve on several things:

Allow the use of block syntax:

Datadog.configure do |c|
  c.use :active_record do |active_record|
    active_record.service_name = 'default-db'
  end
end

Introduce configuration multiplexing

Some integrations must trace multiple endpoints through the same code path, and require additional configuration for each endpoint to do so. Multiplexing allows you to define an arbitrary key that can be associated to specific trace settings, then recalled when needed.

Datadog.configure do |c|
  # Describes tracer settings for a specific context.
  # In this example, these are settings for the 'backup' database.
  c.use :active_record, describes: :backup do |active_record|
    active_record.service_name = 'backup-db'
  end
end

Support for composite configurations

Some integrations activate additional integrations in order to perform tracing. However, those additional integrations are not configurable via the parent integration, and require users to configure them separately.

By implementing object-based configuration with custom methods, it's possible to passthrough configuration settings to underlying integrations without coupling the parent integration to the configuration details of the additional integration.

Rails is a good example of this, which activates rack and active_record by default.

Datadog.configure do |c|
  c.use :rails do |rails|
    rails.service_name = 'my-app'
    
    # Settings for Rack tracing
    rails.rack do |rack|
      rack.headers request: ['Accept-Encoding'], response: ['Content-Encoding']
    end

    # Settings for ActiveRecord tracing
    rails.active_record do |active_record|
      active_record.service_name = 'my-database'
    end
  end
end

Additional remarks

By implementing this configuration core, we also gain additional benefits of DRYing up configuration code, and reducing coupling. These changes are backwards compatible, and can be implemented on an integration-to-integration basis, which would grant us flexibility in their implementation.

See examples of integrations that implement this configuration core here:

• active_record: Implement ActiveRecord integration configuration #451
• sequel: Implement Sequel integration configuration #452

[palazzem]

@delner quick question before proceeding with the review. The previous Configuration API works exactly as before so no changes are required in users code, right?

[delner]

@palazzem Yup, its completely backwards compatible with users applications. They should not require any updates to their configurations in order to be compatible.

[pawelchcki]

Two small comments.
Other than that it looks really good!

[pawelchcki]

caching of the proxy object would need to be ported back.

But it looks to me like integration.configuration(cfg_name) already avoids temporary object creation 👍

[delner]

Ahhh, yes, good point. Would save some memory.

[pawelchcki]

Autopatch seems to not be used anymore. Is this only for backwards compatibility ? If so I think we can just not support it in the new configuration approach anymore.

[delner]

Backwards compatibility, yeah. I'm not sure whether we can remove this, since there are a lot of users who upgrade from really old versions (< 0.10.0) and leave auto_patch in their configuration. Let me look deeper into this, but this is a great point for consideration.

[palazzem]

@pawelchcki @delner no breaking changes must be introduced that way. If there is a risk to break something at bootstrap time, we need to add a deprecation warning and decide when we're going to remove that field. Probably we can remove most of the things once we're heading to 1.0 release.

[pawelchcki]

@palazzem agreed.

However this shouldn't affect integrations that do not explicitly include new Registerable module. Also the user still would be able to call the same method signature register_as :name, auto_patch: true. Now auto_patch will be ignored a bit earlier.

It doesn't look like breaking change to me.

[delner]

@palazzem @palazzem Well, it seems like since we have to deprecate and remove auto_patch from the Registry anyways, why not leave this in for now, and deprecate/remove at the same time when we do it for the registry? Would be just as easy, and consistent.

[delner]

Just looking over this, I realized this needs more test coverage for the new components. I will implement some basic specs, then resubmit this for review.

[delner]

@pawelchcki @palazzem Added more test coverage; this should be good for another review.

[pawelchcki]

Thanks! @delner looks good!