Proposal: Plugin configuration options

[mayurkale22]

This is my initial proposal to kick of the discussion on plugin configurations.

Currently, PluginConfig accepts value with only bolean flag. If true, plugin is loaded otherwise skip it. PluginOptions designed to pass configuration options to each plugins. All fields are optional. We can have some defaultConfig with default configuration values.

TODO Pointer:

opentelemetry-js/packages/opentelemetry-node-tracer/src/instrumentation/PluginLoader.ts

Line 33 in b208b2b

// TODO: Consider to add configuration options

interface PluginOptions {
  /**
   * Whether to enable the plugin.
   */
  enabled?: boolean;

  /**
   * Request methods that match any string in ignoreMethods will not be traced.
   */
  ignoreMethods?: string[];


  /**
   * URLs that partially match any regex in ignoreUrls will not be traced.
   * In addition, URLs that are _exact matches_ of strings in ignoreUrls will
   * also not be traced.
   */
  ignoreUrls?: Array<string | RegExp>;

  /**
   * If true, additional information about query parameters and
   * results will be attached (as labels) to spans representing
   * database operations.
   */
   enhancedDatabaseReporting?: boolean;
}

Usage Example:

OPTION 1

pluginLoader.load({
  http:  { enabled: true, ignoreMethods: ['POST'], ignoreUrls: ['/ignored/string'] },
  mongodb:  { enabled: true, enhancedDatabaseReporting: true },
  grpc:  { enabled: true },
})

Related Issues: #210

These are just initial options, feel free to add/suggest others. I am also open to discuss other perspectives.

[OlivierAlbertini]

I would add the option to specify the package for each plugin in order to customize if needed. It will reduce possible frustrations.

[mayurkale22]

@OlivierAlbertini Hmm, I agree. What do you think about below configs? Instead of having the same options on each plugin I tried to take it out from the plugins which means (for example) ignoreMethods config would be applicable for all configured/enabled plugins. Users won't have options to customize the options based on the plugin.

OPTION 2

PluginConfig {
  enabled?: boolean,
  plugins: {
    http: 'package name/path',
    mongodb: '@opentelemetry/plugin-mongodb',
    grpc: '@opentelemetry/plugin-grpc',
  },
  ignoreMethods?: string[],
  ignoreUrls?: Array<string | RegExp>,
  enhancedDatabaseReporting?: boolean,
}

[OlivierAlbertini]

@OlivierAlbertini Hmm, I agree. What do you think about below configs? Instead of having the same options on each plugin I tried to take it out from the plugins which means (for example) ignoreMethods config would be applicable for all configured/enabled plugins. Users won't have options to customize the options based on the plugin.

PluginConfig {
  enabled?: boolean,
  plugins: {
    http: 'package name/path',
    mongodb: '@opentelemetry/plugin-mongodb',
    grpc: '@opentelemetry/plugin-grpc',
  },
  ignoreMethods?: string[],
  ignoreUrls?: Array<string | RegExp>,
  enhancedDatabaseReporting?: boolean,
}

I prefer your previous version:

pluginLoader.load({
  http:  { enabled: true, ignoreMethods: ['POST'], ignoreUrls: ['/ignored/string'] },
  mongodb:  { enabled: true, enhancedDatabaseReporting: true },
  grpc:  { enabled: true },
});

But I would add:

pluginLoader.load({
  http:  { enabled: true, package: "@opentelemetry/plugin-http", ignoreMethods: ['POST'], ignoreUrls: ['/ignored/string'] },
  mongodb:  { enabled: true,  package: "@macompany/plugin-mongodb", enhancedDatabaseReporting: true },
  grpc:  { enabled: false },
});

and we could provide an interface that specify all accepted plugins:
https://www.typescriptlang.org/play/index.html#code/KYDwDg9gTgLgBASwHY2FAZgQwMbDgBQBsBXAc2QGEIl0FSBnOAbwCg44ALGGMALgMywEmQgB4AEtzBEylarVIA+ANxtOU+v3yCYwsZJ4zySKjToqWLAL4tQkWIhRosuAiWOmFzNcCSYARoTA-P4QEEGYSKrsYDgA1pikwXD0MFDIpKo2tuDQ8MioGDh4BtLucmakcKCoSAAmjEYVXt7sAPQAVB1qcB1wAErAAI7EwKlwALbAMBwQDXAzmPATS9gccJEAnilpGY6IpEjQwACy07PzAO4IhIRwR-D+eGnFdQB0PR1tanRHUKfnOaaHbpJCkADaAF1VJZ2l1PnAAKr9AAyjEW8FiQhEhG2Kxgaw2SG2-ySIH2v2OiKghEY11u9wgj2eUFeH3YvTgAEkkBs6nUELpqAAaJGo9EcJYbf5wAD6oBwy1WHDGsrgEHQIIyjGQBz+wGptLg9MICJE9AgjOZC1ZuHen2+7Ep-0NwIAglBWZtRKlQVUAD4DYCkACi4As2Ww1HGUcqwO02LETRM8jo9EUcAAvK0OVwePwmDYrEA

[mayurkale22]

Second option looks more elegant to me, but it does not offer plugin level config/option. Do we expect users to use config based on plugin like enhancedDatabaseReporting true for mongodb and false for redis?

[OlivierAlbertini]

Second option looks more elegant to me, but it does not offer plugin level config/option.

From a user perspective, I don't want to read all PluginConfig fields to see if it's useful or not for grpc plugin (particularly if I use only grpc plugin).

What do you pass to the plugin itself.. the hole PluginConfig ?
Also we can't disable a specific plugin.

[mayurkale22]

What do you pass to the plugin itself.. the hole PluginConfig ?

In case of OPTION 1: specific plugin config.
In case of OPTION 2: the whole PluginConfig.

Also we can't disable a specific plugin.

In case of OPTION 1: grpc: { enabled: false } would work.
In case of OPTION 2: We have to exclude the plugin from plugins Object. i.e. If I need only http and mongodb, I would do something like this:

PluginConfig {
  plugins: {
    http: 'package name/path',
    mongodb: '@opentelemetry/plugin-mongodb',
  },
 ....
}