Skip to content

Commit 547e518

Browse files
feat: Allow register plugins dynamically (#266)
**Requirements** Add possibility to Register plugins dynamically for separation concerns from LD flagging and Observability. It is not customer facing API and doesn't require documentation change. ``` var client = await LdClient.InitAsync(ldConfig, context, TimeSpan.FromSeconds(5)); client.RegisterPlugin(observabilityPlugin); ``` <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes hook execution and evaluation/identify wrapping for all clients (unified Executor path) and introduces concurrent hook list updates; failures are logged but mis-registered plugins could affect observability behavior. > > **Overview** > Adds **`LdClient.RegisterPlugin`** so plugins (e.g. observability) can be attached after `Init`/`InitAsync`, without listing them in `ConfigurationBuilder.Plugins`. > > The hook pipeline is refactored for **runtime hook registration**: `IHookExecutor` gains **`AddHooks`**, and **`Executor`** keeps an immutable **`Stages`** snapshot (hooks + before/after executors) published under a lock; each evaluation/identify call snapshots `_stages` so before/after stages stay consistent if hooks are added mid-flight. With zero hooks, evaluation/identify skip hook work. > > **`NoopExecutor`** is removed—the client always uses **`Executor`** (including when no hooks are configured at startup). **`EnvironmentMetadata`** is always created on the client (not only when config plugins exist), and is reused for dynamic registration. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6e1d766. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2133517 commit 547e518

4 files changed

Lines changed: 142 additions & 50 deletions

File tree

pkgs/sdk/client/src/Internal/Hooks/Executor/Executor.cs

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,65 @@ namespace LaunchDarkly.Sdk.Client.Internal.Hooks.Executor
1111
{
1212
internal sealed class Executor : IHookExecutor
1313
{
14-
private readonly List<Hook> _hooks;
15-
private readonly Logger _logger;
14+
// Immutable bundle of per-generation state. Once a Stages instance is published via the
15+
// volatile _stages field, none of its fields (including the contents of Hooks) are
16+
// mutated. AddHooks always builds a fresh Stages over a fresh List<Hook>.
17+
private sealed class Stages
18+
{
19+
public readonly List<Hook> Hooks;
20+
public readonly IStageExecutor<EvaluationSeriesContext> BeforeEvaluation;
21+
public readonly IStageExecutor<EvaluationSeriesContext, EvaluationDetail<LdValue>> AfterEvaluation;
22+
public readonly IStageExecutor<IdentifySeriesContext> BeforeIdentify;
23+
public readonly IStageExecutor<IdentifySeriesContext, IdentifySeriesResult> AfterIdentify;
1624

17-
private readonly IStageExecutor<EvaluationSeriesContext> _beforeEvaluation;
18-
private readonly IStageExecutor<EvaluationSeriesContext, EvaluationDetail<LdValue>> _afterEvaluation;
25+
public Stages(Logger logger, List<Hook> hooks)
26+
{
27+
Hooks = hooks;
28+
BeforeEvaluation = new BeforeEvaluation(logger, hooks, EvaluationStage.Order.Forward);
29+
AfterEvaluation = new AfterEvaluation(logger, hooks, EvaluationStage.Order.Reverse);
30+
BeforeIdentify = new BeforeIdentify(logger, hooks, EvaluationStage.Order.Forward);
31+
AfterIdentify = new AfterIdentify(logger, hooks, EvaluationStage.Order.Reverse);
32+
}
33+
}
1934

20-
private readonly IStageExecutor<IdentifySeriesContext> _beforeIdentify;
21-
private readonly IStageExecutor<IdentifySeriesContext, IdentifySeriesResult> _afterIdentify;
35+
private readonly Logger _logger;
36+
private readonly object _lock = new object();
37+
private volatile Stages _stages;
2238

2339
public Executor(Logger logger, IEnumerable<Hook> hooks)
2440
{
2541
_logger = logger;
26-
_hooks = hooks.ToList();
27-
_beforeEvaluation = new BeforeEvaluation(logger, _hooks, EvaluationStage.Order.Forward);
28-
_afterEvaluation = new AfterEvaluation(logger, _hooks, EvaluationStage.Order.Reverse);
29-
_beforeIdentify = new BeforeIdentify(logger, _hooks, EvaluationStage.Order.Forward);
30-
_afterIdentify = new AfterIdentify(logger, _hooks, EvaluationStage.Order.Reverse);
42+
_stages = new Stages(logger, hooks.ToList());
43+
}
44+
45+
public void AddHooks(IEnumerable<Hook> hooks)
46+
{
47+
if (hooks == null) return;
48+
lock (_lock)
49+
{
50+
var added = hooks.ToList();
51+
if (added.Count == 0) return;
52+
var current = _stages.Hooks;
53+
var newHooks = new List<Hook>(current.Count + added.Count);
54+
newHooks.AddRange(current);
55+
newHooks.AddRange(added);
56+
_stages = new Stages(_logger, newHooks);
57+
}
3158
}
3259

3360
public EvaluationDetail<T> EvaluationSeries<T>(EvaluationSeriesContext context,
3461
LdValue.Converter<T> converter, Func<EvaluationDetail<T>> evaluate)
3562
{
36-
var seriesData = _beforeEvaluation.Execute(context, default);
63+
// Snapshot the stages once so Before/After see a single, consistent generation
64+
// even if AddHooks publishes a new Stages mid-call.
65+
var stages = _stages;
66+
if (stages.Hooks.Count == 0) return evaluate();
67+
68+
var seriesData = stages.BeforeEvaluation.Execute(context, default);
3769

3870
var detail = evaluate();
3971

40-
_afterEvaluation.Execute(context,
72+
stages.AfterEvaluation.Execute(context,
4173
new EvaluationDetail<LdValue>(converter.FromType(detail.Value), detail.VariationIndex, detail.Reason),
4274
seriesData);
4375

@@ -46,22 +78,25 @@ public EvaluationDetail<T> EvaluationSeries<T>(EvaluationSeriesContext context,
4678

4779
public async Task<bool> IdentifySeries(Context context, TimeSpan maxWaitTime, Func<Task<bool>> identify)
4880
{
81+
var stages = _stages;
82+
if (stages.Hooks.Count == 0) return await identify();
83+
4984
var identifyContext = new IdentifySeriesContext(context, maxWaitTime);
50-
var seriesData = _beforeIdentify.Execute(identifyContext, default);
85+
var seriesData = stages.BeforeIdentify.Execute(identifyContext, default);
5186

5287
try
5388
{
5489
var result = await identify();
5590

56-
_afterIdentify.Execute(identifyContext,
91+
stages.AfterIdentify.Execute(identifyContext,
5792
new IdentifySeriesResult(IdentifySeriesResult.IdentifySeriesStatus.Completed),
5893
seriesData);
5994

6095
return result;
6196
}
6297
catch (Exception)
6398
{
64-
_afterIdentify.Execute(identifyContext,
99+
stages.AfterIdentify.Execute(identifyContext,
65100
new IdentifySeriesResult(IdentifySeriesResult.IdentifySeriesStatus.Error),
66101
seriesData);
67102

@@ -71,7 +106,8 @@ public async Task<bool> IdentifySeries(Context context, TimeSpan maxWaitTime, Fu
71106

72107
public void Dispose()
73108
{
74-
foreach (var hook in _hooks)
109+
var stages = _stages;
110+
foreach (var hook in stages.Hooks)
75111
{
76112
try
77113
{

pkgs/sdk/client/src/Internal/Hooks/Executor/NoopExecutor.cs

Lines changed: 0 additions & 24 deletions
This file was deleted.

pkgs/sdk/client/src/Internal/Hooks/Interfaces/IHookExecutor.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Threading.Tasks;
34
using LaunchDarkly.Sdk.Client.Hooks;
45

56
namespace LaunchDarkly.Sdk.Client.Internal.Hooks.Interfaces
67
{
78
/// <summary>
89
/// An IHookExecutor is responsible for executing the logic contained in a series of hook stages.
9-
///
10-
/// The purpose of this interface is to allow the SDK to swap out the executor based on having any hooks
11-
/// configured or not. If there are no hooks, the interface methods can be no-ops.
1210
/// </summary>
1311
internal interface IHookExecutor : IDisposable
1412
{
@@ -34,5 +32,13 @@ EvaluationDetail<T> EvaluationSeries<T>(EvaluationSeriesContext context,
3432
/// <param name="identify">async function that performs the identify operation</param>
3533
/// <returns>the result of the identify operation</returns>
3634
Task<bool> IdentifySeries(Context context, TimeSpan maxWaitTime, Func<Task<bool>> identify);
35+
36+
/// <summary>
37+
/// Adds additional hooks to the executor so that subsequent <see cref="EvaluationSeries{T}"/> and
38+
/// <see cref="IdentifySeries"/> calls invoke them. The implementation is responsible for
39+
/// synchronizing concurrent calls.
40+
/// </summary>
41+
/// <param name="hooks">the hooks to add; may be empty or null (no-op in either case)</param>
42+
void AddHooks(IEnumerable<Hook> hooks);
3743
}
3844
}

pkgs/sdk/client/src/LdClient.cs

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using LaunchDarkly.Sdk.Client.Internal.Events;
1616
using LaunchDarkly.Sdk.Client.Internal.Interfaces;
1717
using LaunchDarkly.Sdk.Client.PlatformSpecific;
18+
using LaunchDarkly.Sdk.Client.Plugins;
1819
using LaunchDarkly.Sdk.Client.Subsystems;
1920
using LaunchDarkly.Sdk.Integrations.Plugins;
2021
using LaunchDarkly.Sdk.Internal;
@@ -72,6 +73,7 @@ public sealed class LdClient : ILdClient
7273
readonly TaskExecutor _taskExecutor;
7374
readonly AnonymousKeyContextDecorator _anonymousKeyContextDecorator;
7475
private readonly AutoEnvContextDecorator _autoEnvContextDecorator;
76+
private readonly EnvironmentMetadata _environmentMetadata;
7577
private readonly IHookExecutor _hookExecutor;
7678

7779
private readonly Logger _log;
@@ -224,16 +226,14 @@ public sealed class LdClient : ILdClient
224226

225227
// Build the plugin config and environment metadata
226228
var pluginConfig = (_config.Plugins ?? Components.Plugins()).Build();
227-
var environmentMetadata = pluginConfig.Plugins.Any() ? CreateEnvironmentMetadata() : null;
229+
_environmentMetadata = CreateEnvironmentMetadata();
228230
var hooks = pluginConfig.Plugins.Any()
229-
? this.GetPluginHooks(pluginConfig.Plugins, environmentMetadata, _log)
231+
? this.GetPluginHooks(pluginConfig.Plugins, _environmentMetadata, _log)
230232
: new List<Hook>();
231233

232-
_hookExecutor = hooks.Any()
233-
? (IHookExecutor)new Executor(_log.SubLogger(LogNames.HooksSubLog), hooks)
234-
: new NoopExecutor();
234+
_hookExecutor = new Executor(_log.SubLogger(LogNames.HooksSubLog), hooks);
235235

236-
this.RegisterPlugins(pluginConfig.Plugins, environmentMetadata, _log);
236+
this.RegisterPlugins(pluginConfig.Plugins, _environmentMetadata, _log);
237237

238238
// Start the background mode manager
239239
_backgroundModeManager = _config.BackgroundModeManager ?? new DefaultBackgroundModeManager();
@@ -902,6 +902,80 @@ public bool FlushAndWait(TimeSpan timeout) =>
902902
public Task<bool> FlushAndWaitAsync(TimeSpan timeout) =>
903903
_eventProcessor.FlushAndWaitAsync(timeout);
904904

905+
/// <summary>
906+
/// Registers a single <see cref="Plugin"/> with this client after construction.
907+
/// </summary>
908+
/// <remarks>
909+
/// <para>
910+
/// Retrieves hooks via <c>GetHooks</c>, calls <c>Register</c>, then merges the hooks into
911+
/// the live pipeline. This ordering differs from construction-time registration for plugins
912+
/// configured via <see cref="ConfigurationBuilder.Plugins"/>, where hooks are added to the
913+
/// executor before <c>Register</c> is called: here, hooks are not active during
914+
/// <c>Register</c>, so flag evaluations or identify calls made inside <c>Register</c> will
915+
/// not invoke this plugin's hooks. After this method returns successfully, subsequent
916+
/// evaluations and identify calls will invoke them.
917+
/// </para>
918+
/// <para>
919+
/// Exceptions thrown by the plugin's <c>Register</c> or <c>GetHooks</c> are caught and
920+
/// logged; they do not propagate to the caller. If either throws, the plugin is not
921+
/// registered and its hooks are not added to the live pipeline. Hooks returned by
922+
/// <c>GetHooks</c> are disposed if <c>Register</c> fails.
923+
/// </para>
924+
/// </remarks>
925+
/// <param name="plugin">the plugin to register; must not be null</param>
926+
/// <exception cref="ArgumentNullException">if <paramref name="plugin"/> is null</exception>
927+
public void RegisterPlugin(Plugin plugin)
928+
{
929+
if (plugin == null) throw new ArgumentNullException(nameof(plugin));
930+
931+
IList<Hook> pluginHooks = null;
932+
try
933+
{
934+
pluginHooks = plugin.GetHooks(_environmentMetadata);
935+
}
936+
catch (Exception ex)
937+
{
938+
_log.Error("Error getting hooks from plugin {0}: {1}",
939+
plugin.Metadata.Name ?? "unknown", ex);
940+
return;
941+
}
942+
943+
try
944+
{
945+
plugin.Register(this, _environmentMetadata);
946+
}
947+
catch (Exception ex)
948+
{
949+
_log.Error("Error registering plugin {0}: {1}",
950+
plugin.Metadata.Name ?? "unknown", ex);
951+
DisposePluginHooks(pluginHooks);
952+
return;
953+
}
954+
955+
if (pluginHooks != null && pluginHooks.Count > 0)
956+
{
957+
_hookExecutor.AddHooks(pluginHooks);
958+
}
959+
}
960+
961+
private void DisposePluginHooks(IList<Hook> pluginHooks)
962+
{
963+
if (pluginHooks == null) return;
964+
965+
foreach (var hook in pluginHooks)
966+
{
967+
try
968+
{
969+
hook?.Dispose();
970+
}
971+
catch (Exception e)
972+
{
973+
_log.Error("During disposal of hook \"{0}\" reported error: {1}",
974+
hook?.Metadata.Name, e.Message);
975+
}
976+
}
977+
}
978+
905979
/// <inheritdoc/>
906980
public bool Identify(Context context, TimeSpan maxWaitTime)
907981
{

0 commit comments

Comments
 (0)