mirror of
https://github.com/elastic/kibana.git
synced 2025-06-27 10:40:07 -04:00
[Core] Improve log message for topological order (circular dependency) errors (#222039)
## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. Make it easier to debug topological plugin order errors by including the exact cycle/s causing the issue in the error message:  Previous error message: ``` Error: Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ["discover","esql","canvas","crossClusterReplication","discoverEnhanced","indexLifecycleManagement","logstash","monitoring","observabilityAiAssistantManagement","remoteClusters","reporting","rollup","contentConnectors","dataQuality","datasetQuality","fleet","indexManagement","ml","osquery","streamsApp","apm","exploratoryView","infra","inventory","observability","observabilityAIAssistantApp","observabilityLogsExplorer","observabilityOnboarding","streamsAppWrapper","slo","synthetics","uptime","ux","enterpriseSearch","searchAssistant","searchIndices","searchInferenceEndpoints","searchPlayground","cloudSecurityPosture","elasticAssistant","securitySolution","securitySolutionEss"] ``` New error message tells you exactly where the cycle is: ``` Error: Topological ordering of plugins did not complete due to circular dependencies: Detected circular dependencies: discover -> elasticAssistant -> ml -> discover Plugins with cyclic or missing dependencies: ["discover","esql","canvas","crossClusterReplication","discoverEnhanced","indexLifecycleManagement","logstash","monitoring","observabilityAiAssistantManagement","remoteClusters","reporting","rollup","contentConnectors","dataQuality","datasetQuality","fleet","indexManagement","ml","osquery","streamsApp","apm","exploratoryView","infra","inventory","observability","observabilityAIAssistantApp","observabilityLogsExplorer","observabilityOnboarding","streamsAppWrapper","slo","synthetics","uptime","ux","enterpriseSearch","searchAssistant","searchIndices","searchInferenceEndpoints","searchPlayground","cloudSecurityPosture","elasticAssistant","securitySolution","securitySolutionEss"] ``` ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [X] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [X] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [X] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [X] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [X] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jean-Louis Leysens <jeanlouis.leysens@elastic.co>
This commit is contained in:
Kenneth Kreindler
GitHub
parent
fbddd79f24
commit
b2d91b43f3
2 changed files with 244 additions and 11 deletions
|
|
@ -25,7 +25,7 @@ import { configServiceMock, getEnvOptions } from '@kbn/config-mocks';
|
|||
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
|
||||
|
||||
import { PluginWrapper } from './plugin';
|
||||
import { PluginsSystem } from './plugins_system';
|
||||
import { findCircularDependencies, normalizeCycle, PluginsSystem } from './plugins_system';
|
||||
import { coreInternalLifecycleMock } from '@kbn/core-lifecycle-server-mocks';
|
||||
|
||||
function createPlugin(
|
||||
|
|
@ -165,9 +165,11 @@ test('getPluginDependencies returns dependency tree with keys topologically sort
|
|||
test('`setupPlugins` throws plugin has missing required dependency', async () => {
|
||||
pluginsSystem.addPlugin(createPlugin('some-id', { required: ['missing-dep'] }));
|
||||
|
||||
await expect(pluginsSystem.setupPlugins(setupDeps)).rejects.toMatchInlineSnapshot(
|
||||
`[Error: Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ["some-id"]]`
|
||||
);
|
||||
await expect(pluginsSystem.setupPlugins(setupDeps)).rejects.toMatchInlineSnapshot(`
|
||||
[Error: Topological ordering of plugins did not complete due to circular dependencies:
|
||||
|
||||
Plugins with cyclic or missing dependencies: ["some-id"]]
|
||||
`);
|
||||
});
|
||||
|
||||
test('`setupPlugins` throws if plugins have circular required dependency', async () => {
|
||||
|
|
@ -175,9 +177,14 @@ test('`setupPlugins` throws if plugins have circular required dependency', async
|
|||
pluginsSystem.addPlugin(createPlugin('depends-on-1', { required: ['depends-on-2'] }));
|
||||
pluginsSystem.addPlugin(createPlugin('depends-on-2', { required: ['depends-on-1'] }));
|
||||
|
||||
await expect(pluginsSystem.setupPlugins(setupDeps)).rejects.toMatchInlineSnapshot(
|
||||
`[Error: Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ["depends-on-1","depends-on-2"]]`
|
||||
);
|
||||
await expect(pluginsSystem.setupPlugins(setupDeps)).rejects.toMatchInlineSnapshot(`
|
||||
[Error: Topological ordering of plugins did not complete due to circular dependencies:
|
||||
|
||||
Detected circular dependencies:
|
||||
depends-on-1 -> depends-on-2 -> depends-on-1
|
||||
|
||||
Plugins with cyclic or missing dependencies: ["depends-on-1","depends-on-2"]]
|
||||
`);
|
||||
});
|
||||
|
||||
test('`setupPlugins` throws if plugins have circular optional dependency', async () => {
|
||||
|
|
@ -185,9 +192,14 @@ test('`setupPlugins` throws if plugins have circular optional dependency', async
|
|||
pluginsSystem.addPlugin(createPlugin('depends-on-1', { optional: ['depends-on-2'] }));
|
||||
pluginsSystem.addPlugin(createPlugin('depends-on-2', { optional: ['depends-on-1'] }));
|
||||
|
||||
await expect(pluginsSystem.setupPlugins(setupDeps)).rejects.toMatchInlineSnapshot(
|
||||
`[Error: Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ["depends-on-1","depends-on-2"]]`
|
||||
);
|
||||
await expect(pluginsSystem.setupPlugins(setupDeps)).rejects.toMatchInlineSnapshot(`
|
||||
[Error: Topological ordering of plugins did not complete due to circular dependencies:
|
||||
|
||||
Detected circular dependencies:
|
||||
depends-on-1 -> depends-on-2 -> depends-on-1
|
||||
|
||||
Plugins with cyclic or missing dependencies: ["depends-on-1","depends-on-2"]]
|
||||
`);
|
||||
});
|
||||
|
||||
test('`setupPlugins` ignores missing optional dependency', async () => {
|
||||
|
|
@ -816,6 +828,128 @@ describe('asynchronous plugins', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('normalizeCycle', () => {
|
||||
it.each([
|
||||
[[], []],
|
||||
[['a'], ['a']],
|
||||
[
|
||||
['a', 'b'],
|
||||
['a', 'b'],
|
||||
],
|
||||
[
|
||||
['b', 'a'],
|
||||
['a', 'b'],
|
||||
],
|
||||
[
|
||||
['a', 'b', 'c'],
|
||||
['a', 'b', 'c'],
|
||||
],
|
||||
[
|
||||
['c', 'a', 'b'],
|
||||
['a', 'b', 'c'],
|
||||
],
|
||||
[
|
||||
['a', 'c', 'b'],
|
||||
['a', 'c', 'b'],
|
||||
],
|
||||
])("normalizes cycle '%s'", (plugins: PluginName[], expected: PluginName[]) => {
|
||||
expect(normalizeCycle(plugins)).toEqual(expected);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[['a', 'b', 'a']],
|
||||
[['b', 'a', 'b']],
|
||||
[['a', 'b', 'c', 'a']],
|
||||
[['c', 'a', 'b', 'c']],
|
||||
[['a', 'c', 'b', 'a']],
|
||||
])("throws error for invalid cycle format '%s'", (plugins: PluginName[]) => {
|
||||
expect(() => normalizeCycle(plugins)).toThrowError();
|
||||
});
|
||||
});
|
||||
|
||||
describe('findCircularDependencies', () => {
|
||||
it.each([
|
||||
[new Map([]) as Map<PluginName, Set<PluginName>>, []],
|
||||
[new Map([['a', new Set(['b'])]]), []],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['c'])],
|
||||
['c', new Set(['d'])],
|
||||
['d', new Set(['e'])],
|
||||
]),
|
||||
[],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['a'])],
|
||||
]),
|
||||
[['a', 'b']],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['c'])],
|
||||
['c', new Set(['a'])],
|
||||
]),
|
||||
[['a', 'b', 'c']],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['a'])],
|
||||
['c', new Set(['d'])],
|
||||
['d', new Set(['c'])],
|
||||
]),
|
||||
[
|
||||
['a', 'b'],
|
||||
['c', 'd'],
|
||||
],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['c'])],
|
||||
['c', new Set(['d'])],
|
||||
['d', new Set(['c'])],
|
||||
]),
|
||||
[['c', 'd']],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['c'])],
|
||||
['c', new Set(['d'])],
|
||||
['d', new Set(['a'])],
|
||||
]),
|
||||
[['a', 'b', 'c', 'd']],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['a'])],
|
||||
['b', new Set(['a'])],
|
||||
]),
|
||||
[['a', 'b']],
|
||||
],
|
||||
[
|
||||
new Map([
|
||||
['a', new Set(['b'])],
|
||||
['b', new Set(['c'])],
|
||||
['c', new Set(['a'])],
|
||||
['d', new Set(['e'])],
|
||||
]),
|
||||
[['a', 'b', 'c']],
|
||||
],
|
||||
])(
|
||||
"returns correct circular dependencies for '%s'",
|
||||
(dependencyGraph: Map<PluginName, Set<PluginName>>, expected: PluginName[][]) => {
|
||||
expect(findCircularDependencies(dependencyGraph)).toEqual(expected);
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
describe('stop', () => {
|
||||
beforeAll(() => {
|
||||
jest.useFakeTimers({ legacyFakeTimers: true });
|
||||
|
|
|
|||
|
|
@ -362,9 +362,25 @@ const getTopologicallySortedPluginNames = (plugins: Map<PluginName, PluginWrappe
|
|||
}
|
||||
|
||||
if (pluginsDependenciesGraph.size > 0) {
|
||||
// Identify circular dependencies
|
||||
let cyclePaths: string[] = [];
|
||||
|
||||
try {
|
||||
const circularDependencies = findCircularDependencies(pluginsDependenciesGraph);
|
||||
|
||||
cyclePaths = circularDependencies.map((cycle) => `\n ${cycle.join(' -> ')} -> ${cycle[0]}`);
|
||||
} catch (e) {
|
||||
cyclePaths = [];
|
||||
}
|
||||
|
||||
const edgesLeft = JSON.stringify([...pluginsDependenciesGraph.keys()]);
|
||||
|
||||
throw new Error(
|
||||
`Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ${edgesLeft}`
|
||||
`Topological ordering of plugins did not complete due to circular dependencies:` +
|
||||
`${
|
||||
cyclePaths.length > 0 ? `\n\nDetected circular dependencies:${cyclePaths.join('')}` : ''
|
||||
}` +
|
||||
`\n\nPlugins with cyclic or missing dependencies: ${edgesLeft}`
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -405,3 +421,86 @@ const buildPluginRuntimeDependencyMap = (
|
|||
}
|
||||
return runtimeDependencies;
|
||||
};
|
||||
|
||||
/**
|
||||
* Finds all circular dependencies in the plugin graph
|
||||
* @param dependencyGraph Map of plugin names to their unresolved dependencies
|
||||
* @returns Array of circular dependency paths
|
||||
*/
|
||||
export const findCircularDependencies = (
|
||||
dependencyGraph: Map<PluginName, Set<PluginName>>
|
||||
): PluginName[][] => {
|
||||
// Store found cycles as a set of stringified paths to avoid duplicates
|
||||
const cycleSet = new Set<string>();
|
||||
const cycles: PluginName[][] = [];
|
||||
|
||||
// Find all cycles for each node in the graph
|
||||
for (const startNode of dependencyGraph.keys()) {
|
||||
// Track visited and recursion stack for this specific search
|
||||
const visited = new Set<PluginName>();
|
||||
const recursionStack = new Set<PluginName>();
|
||||
const path: PluginName[] = [];
|
||||
|
||||
const dfs = (node: PluginName) => {
|
||||
visited.add(node);
|
||||
recursionStack.add(node);
|
||||
path.push(node);
|
||||
|
||||
const dependencies = dependencyGraph.get(node) || new Set<PluginName>();
|
||||
|
||||
for (const dependency of dependencies) {
|
||||
// If we haven't visited this dependency yet, explore it
|
||||
if (!visited.has(dependency)) {
|
||||
dfs(dependency);
|
||||
}
|
||||
// If the dependency is in our current recursion path, we found a cycle
|
||||
else if (recursionStack.has(dependency)) {
|
||||
// Extract the cycle
|
||||
const cycleStartIndex = path.indexOf(dependency);
|
||||
if (cycleStartIndex !== -1) {
|
||||
const cycle = path.slice(cycleStartIndex);
|
||||
// Create a canonical representation by starting from alphabetically first node
|
||||
const normalizedCycle = normalizeCycle(cycle);
|
||||
|
||||
// Add to cycles if not already seen
|
||||
const cycleKey = JSON.stringify(normalizedCycle);
|
||||
if (!cycleSet.has(cycleKey)) {
|
||||
cycleSet.add(cycleKey);
|
||||
cycles.push(cycle);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Backtrack
|
||||
path.pop();
|
||||
recursionStack.delete(node);
|
||||
};
|
||||
|
||||
dfs(startNode);
|
||||
}
|
||||
|
||||
return cycles;
|
||||
};
|
||||
|
||||
/**
|
||||
* Normalizes a cycle by rotating it to start with the alphabetically first node
|
||||
* This helps identify duplicate cycles regardless of where we start traversing
|
||||
*/
|
||||
export const normalizeCycle = (cycle: PluginName[]): PluginName[] => {
|
||||
if (cycle.length <= 1) return cycle;
|
||||
if (new Set(cycle).size !== cycle.length) {
|
||||
throw new Error(`Cycle contains duplicate plugins: ${cycle}`);
|
||||
}
|
||||
|
||||
// Find the index of the alphabetically first node
|
||||
let minIndex = 0;
|
||||
for (let i = 1; i < cycle.length; i++) {
|
||||
if (cycle[i].localeCompare(cycle[minIndex]) < 0) {
|
||||
minIndex = i;
|
||||
}
|
||||
}
|
||||
|
||||
// Rotate the array to start with that node
|
||||
return [...cycle.slice(minIndex), ...cycle.slice(0, minIndex)];
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue