Repository navigation
feat: Add the file-based override source #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
kinyoklion
wants to merge
5
commits into
rlamb/overrides-java-override-events
Choose a base branch
from
rlamb/overrides-java-file-override-source
base: rlamb/overrides-java-override-events
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
cb99b6c
feat: Add the file-based override source
kinyoklion 87d5dd3
fix: Describe and expect the fallthrough expansion of value-only over…
kinyoklion bcee6f4
fix: Pass the logger to the file data poller
kinyoklion dfabc7f
fix: Log a summary of the exception when override files cannot be wat…
kinyoklion 21f7364
fix: Drop a file path listed more than once instead of treating its e…
kinyoklion File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
154 changes: 154 additions & 0 deletions
154
...ver/src/main/java/com/launchdarkly/sdk/server/integrations/FileOverrideSourceBuilder.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| package com.launchdarkly.sdk.server.integrations; | ||
|
|
||
| import com.launchdarkly.logging.LDLogger; | ||
| import com.launchdarkly.sdk.server.integrations.FileOverrides.ChangeDetection; | ||
| import com.launchdarkly.sdk.server.subsystems.ClientContext; | ||
| import com.launchdarkly.sdk.server.subsystems.ComponentConfigurer; | ||
| import com.launchdarkly.sdk.server.subsystems.OverrideSource; | ||
|
|
||
| import java.nio.file.InvalidPathException; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.time.Duration; | ||
| import java.util.ArrayList; | ||
| import java.util.LinkedHashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| /** | ||
| * Configures the file-based override source. Obtain an instance with {@link FileOverrides#source()}, | ||
| * call {@link #filePaths(String...)} or {@link #filePaths(Path...)} to specify the files, adjust | ||
| * any other options, and pass the builder to | ||
| * {@link DataSystemBuilder#overrides(ComponentConfigurer)}. | ||
| * <p> | ||
| * For more details, see {@link FileOverrides}. | ||
| * <p> | ||
| * Flag overrides are currently experimental and subject to change. | ||
| * | ||
| * @since 7.18.0 | ||
| */ | ||
| public final class FileOverrideSourceBuilder implements ComponentConfigurer<OverrideSource> { | ||
| private final List<Path> filePaths = new ArrayList<>(); | ||
| private FileData.DuplicateKeysHandling duplicateKeysHandling = FileData.DuplicateKeysHandling.FAIL; | ||
| private ChangeDetection changeDetection = ChangeDetection.POLLING; | ||
| private Duration pollInterval = FileOverrides.DEFAULT_POLL_INTERVAL; | ||
|
|
||
| FileOverrideSourceBuilder() {} | ||
|
|
||
| /** | ||
| * Adds any number of files to load overrides from, specifying each file path as a string. The | ||
| * order is significant: it determines which file wins under the duplicate keys handling when the | ||
| * same key appears in more than one file. | ||
| * <p> | ||
| * Files are parsed as JSON if their first non-whitespace character is '{'. Otherwise, they are | ||
| * parsed as YAML. | ||
| * | ||
| * @param filePaths path(s) to the file(s); may be absolute or relative to the current working directory | ||
| * @return the same builder | ||
| * @throws InvalidPathException if one of the parameters is not a valid file path | ||
| */ | ||
| public FileOverrideSourceBuilder filePaths(String... filePaths) throws InvalidPathException { | ||
| for (String p : filePaths) { | ||
| this.filePaths.add(Paths.get(p)); | ||
| } | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Adds any number of files to load overrides from, specifying each file path as a Path. The | ||
| * order is significant: it determines which file wins under the duplicate keys handling when the | ||
| * same key appears in more than one file. | ||
| * | ||
| * @param filePaths path(s) to the file(s); may be absolute or relative to the current working directory | ||
| * @return the same builder | ||
| */ | ||
| public FileOverrideSourceBuilder filePaths(Path... filePaths) { | ||
| for (Path p : filePaths) { | ||
| this.filePaths.add(p); | ||
| } | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Specifies how to handle the same key appearing in more than one file. | ||
| * <p> | ||
| * With {@link FileData.DuplicateKeysHandling#FAIL}, the default, a reload fails if the same flag or | ||
| * segment key appears in more than one file, and the previously loaded overrides stay in effect. | ||
| * With {@link FileData.DuplicateKeysHandling#IGNORE}, the entry from the first configured file that | ||
| * defines the key is kept and the others are discarded. A null value selects the default. | ||
| * | ||
| * @param duplicateKeysHandling specifies how to handle duplicate keys | ||
| * @return the same builder | ||
| */ | ||
| public FileOverrideSourceBuilder duplicateKeysHandling(FileData.DuplicateKeysHandling duplicateKeysHandling) { | ||
| this.duplicateKeysHandling = duplicateKeysHandling == null | ||
| ? FileData.DuplicateKeysHandling.FAIL : duplicateKeysHandling; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Selects how the source detects file changes. The default is {@link ChangeDetection#POLLING}. | ||
| * The two modes are alternatives, so setting one replaces the other. | ||
| * | ||
| * @param changeDetection the change detection mode | ||
| * @return the same builder | ||
| */ | ||
| public FileOverrideSourceBuilder changeDetection(ChangeDetection changeDetection) { | ||
| this.changeDetection = changeDetection; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the interval between examinations of the files in {@link ChangeDetection#POLLING} mode. | ||
| * {@link ChangeDetection#WATCHING} mode ignores it. The default is | ||
| * {@link FileOverrides#DEFAULT_POLL_INTERVAL}. An interval below | ||
| * {@link FileOverrides#MINIMUM_POLL_INTERVAL} is raised to the minimum. | ||
| * | ||
| * @param pollInterval the polling interval | ||
| * @return the same builder | ||
| */ | ||
| public FileOverrideSourceBuilder pollInterval(Duration pollInterval) { | ||
| this.pollInterval = pollInterval; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Called internally by the SDK to create the override source. | ||
| * | ||
| * @param context the client context | ||
| * @return the override source | ||
| * @throws IllegalArgumentException if no file paths were specified, or the change detection mode | ||
| * or the poll interval is null | ||
| */ | ||
| @Override | ||
| public OverrideSource build(ClientContext context) { | ||
| if (filePaths.isEmpty()) { | ||
| throw new IllegalArgumentException("no file paths were specified for the file-based override source"); | ||
| } | ||
| if (changeDetection == null) { | ||
| throw new IllegalArgumentException("a change detection mode is required for the file-based override source"); | ||
| } | ||
| if (pollInterval == null) { | ||
| throw new IllegalArgumentException("a poll interval is required for the file-based override source"); | ||
| } | ||
| LDLogger logger = context.getBaseLogger().subLogger("FileOverrideSource"); | ||
|
|
||
| // A path listed more than once would make every entry of that file a duplicate of itself, so | ||
| // the repeats are dropped and the first occurrence keeps its place in the order. | ||
| Set<Path> absolutePaths = new LinkedHashSet<>(); | ||
| for (Path p : filePaths) { | ||
| absolutePaths.add(p.toAbsolutePath().normalize()); | ||
| } | ||
| if (absolutePaths.size() != filePaths.size()) { | ||
| logger.warn("File paths list the same file more than once; the repeats are ignored"); | ||
| } | ||
|
|
||
| Duration effectivePollInterval = pollInterval; | ||
| if (changeDetection == ChangeDetection.POLLING && pollInterval.compareTo(FileOverrides.MINIMUM_POLL_INTERVAL) < 0) { | ||
| logger.warn("Poll interval {} is below the minimum; using {}", pollInterval, FileOverrides.MINIMUM_POLL_INTERVAL); | ||
| effectivePollInterval = FileOverrides.MINIMUM_POLL_INTERVAL; | ||
| } | ||
|
|
||
| return new FileOverrideSourceImpl(new ArrayList<>(absolutePaths), duplicateKeysHandling, changeDetection, effectivePollInterval, logger); | ||
| } | ||
| } |
171 changes: 171 additions & 0 deletions
171
...server/src/main/java/com/launchdarkly/sdk/server/integrations/FileOverrideSourceImpl.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| package com.launchdarkly.sdk.server.integrations; | ||
|
|
||
| import com.launchdarkly.logging.LDLogger; | ||
| import com.launchdarkly.logging.LogValues; | ||
| import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; | ||
| import com.launchdarkly.sdk.server.integrations.FileOverrides.ChangeDetection; | ||
| import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.FileSummary; | ||
| import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; | ||
| import com.launchdarkly.sdk.server.subsystems.OverrideSink; | ||
| import com.launchdarkly.sdk.server.subsystems.OverrideSource; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Path; | ||
| import java.time.Duration; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| /** | ||
| * The file-based override source. It reads one or more files in the file data source document | ||
| * format, merges them in the configured order, and supplies the result to the override sink as | ||
| * one snapshot. It reloads when the files change, by polling or by watching. | ||
| * <p> | ||
| * A configured file that does not exist contributes no overrides. A file that exists but cannot | ||
| * be read or parsed fails that reload: the sink is not called, so the last good overrides stay in | ||
| * effect, and the reload is retried. | ||
| */ | ||
| final class FileOverrideSourceImpl implements OverrideSource { | ||
| private final List<Path> paths; | ||
| private final ChangeDetection changeDetection; | ||
| private final Duration pollInterval; | ||
| private final LDLogger logger; | ||
| private final OverrideFileLoader loader; | ||
|
|
||
| private volatile FileDataReloader reloader; | ||
| private volatile FileDataPoller poller; | ||
| private volatile FileDataWatcher watcher; | ||
| private final AtomicBoolean closed = new AtomicBoolean(false); | ||
|
|
||
| FileOverrideSourceImpl( | ||
| List<Path> paths, | ||
| FileData.DuplicateKeysHandling duplicateKeysHandling, | ||
| ChangeDetection changeDetection, | ||
| Duration pollInterval, | ||
| LDLogger logger | ||
| ) { | ||
| this.paths = new ArrayList<>(paths); | ||
| this.changeDetection = changeDetection; | ||
| this.pollInterval = pollInterval; | ||
| this.logger = logger; | ||
| this.loader = new OverrideFileLoader(paths, duplicateKeysHandling); | ||
| } | ||
|
|
||
| /** | ||
| * Performs the initial load synchronously, so overrides present in the files are in effect when | ||
| * the client constructor returns, then starts change detection. A file that does not exist yet | ||
| * contributes no overrides. A file that cannot be read or parsed is not fatal: the client runs | ||
| * with the last good overrides, the failure is logged, and the retry, plus the change signal, | ||
| * recovers once the file is readable. | ||
| */ | ||
| @Override | ||
| public void start(OverrideSink sink) { | ||
| reloader = new FileDataReloader( | ||
| loader::load, | ||
| new FileDataReloader.Handler() { | ||
| @Override | ||
| public void apply(LoadResult result) { | ||
| sink.setOverrides(result.getData()); | ||
| logOverridesInEffect(result); | ||
| } | ||
|
|
||
| @Override | ||
| public void onError(FileDataException e) { | ||
| // The reloader has logged the failure. The last good overrides stay in effect. | ||
| } | ||
| }, | ||
| logger, | ||
| FileDataReloader.DEFAULT_DEBOUNCE_DELAY, | ||
| FileDataReloader.DEFAULT_RETRY_DELAY, | ||
| true | ||
| ); | ||
| reloader.reloadNow(); | ||
|
|
||
| if (closed.get()) { | ||
| return; | ||
| } | ||
| switch (changeDetection) { | ||
| case WATCHING: | ||
| try { | ||
| FileDataWatcher w = FileDataWatcher.create(paths, logger); | ||
| watcher = w; | ||
| w.start(reloader::trigger); | ||
| } catch (IOException e) { | ||
| // COVERAGE: constructing a watcher only fails under unusual OS conditions | ||
| logger.error("Unable to watch override files: {}", LogValues.exceptionSummary(e)); | ||
| logger.debug(LogValues.exceptionTrace(e)); | ||
| } | ||
| break; | ||
| case POLLING: | ||
| default: | ||
| poller = new FileDataPoller(paths, pollInterval, reloader::trigger, logger); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns the interval between file examinations in polling mode. Visible for tests. | ||
| * | ||
| * @return the effective poll interval | ||
| */ | ||
| Duration getPollInterval() { | ||
| return pollInterval; | ||
| } | ||
|
|
||
| // Reports the overrides now in effect and the file each came from. The reloader applies a | ||
| // snapshot only when the content changed, so this logs each change once. | ||
| private void logOverridesInEffect(LoadResult result) { | ||
| List<String> details = new ArrayList<>(result.getFiles().size()); | ||
| for (FileSummary file : result.getFiles()) { | ||
| if (!file.isPresent()) { | ||
| details.add(file.getPath() + ": absent"); | ||
| } else if (file.getFlags() == 0 && file.getSegments() == 0) { | ||
| details.add(file.getPath() + ": no entries"); | ||
| } else { | ||
| details.add(file.getPath() + ": " + countsText(file.getFlags(), file.getSegments())); | ||
| } | ||
| } | ||
| String fileDetails = String.join("; ", details); | ||
| if (result.getFlagCount() == 0 && result.getSegmentCount() == 0) { | ||
| logger.info("Flag overrides: none in effect ({})", fileDetails); | ||
| return; | ||
| } | ||
| logger.info("Flag overrides in effect: {} ({})", countsText(result.getFlagCount(), result.getSegmentCount()), | ||
| fileDetails); | ||
| } | ||
|
|
||
| // Formats flag and segment counts, for example "2 flags, 1 segment". | ||
| static String countsText(int flags, int segments) { | ||
| List<String> parts = new ArrayList<>(2); | ||
| if (flags > 0) { | ||
| parts.add(pluralize(flags, "flag")); | ||
| } | ||
| if (segments > 0) { | ||
| parts.add(pluralize(segments, "segment")); | ||
| } | ||
| return String.join(", ", parts); | ||
| } | ||
|
|
||
| private static String pluralize(int count, String noun) { | ||
| return count == 1 ? "1 " + noun : count + " " + noun + "s"; | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| if (closed.getAndSet(true)) { | ||
| return; | ||
| } | ||
| FileDataWatcher w = watcher; | ||
| if (w != null) { | ||
| w.close(); | ||
| } | ||
| FileDataPoller p = poller; | ||
| if (p != null) { | ||
| p.close(); | ||
| } | ||
| FileDataReloader r = reloader; | ||
| if (r != null) { | ||
| r.close(); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Watching setup failure stops all reloads
Medium Severity
When
WATCHINGcannot start, the error is logged andstartcontinues, but neither a watcher nor a poller is left running. After a successful initial load the reloader also has no retry armed, so later file creates, edits, and deletes never apply. That breaks the documented contract that a missing file taking effect later, or an operator updating overrides during an incident, will be picked up.Reviewed by Cursor Bugbot for commit a0f4c55. Configure here.