Skip to content
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.

Sign up for GitHub

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

Remove dependency on infra in monitoring plugin #203551

Merged
merged 13 commits into from
Dec 11, 2024
Merged

Remove dependency on infra in monitoring plugin #203551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9667162
remove dependency on infra in monitoring plugin
flash1293 Dec 10, 2024
33b2454
fix some stugf
flash1293 Dec 10, 2024
b6de1ee
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Dec 10, 2024
2705a6f
Merge remote-tracking branch 'upstream/main' into flash1293/monitorin…
flash1293 Dec 10, 2024
d8292b7
fix
flash1293 Dec 10, 2024
6f4d06b
Merge branch 'flash1293/monitoring-infra' of github.com:flash1293/kib…
flash1293 Dec 10, 2024
fd270cb
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Dec 10, 2024
714bd91
everything goes to platform
flash1293 Dec 10, 2024
4f8b7db
Merge branch 'flash1293/monitoring-infra' of github.com:flash1293/kib…
flash1293 Dec 10, 2024
0d64665
Merge branch 'main' into flash1293/monitoring-infra
flash1293 Dec 10, 2024
eb5d104
fix test
flash1293 Dec 11, 2024
5a902a1
Merge remote-tracking branch 'upstream/main' into flash1293/monitorin…
flash1293 Dec 11, 2024
4cb26b2
Merge branch 'flash1293/monitoring-infra' of github.com:flash1293/kib…
flash1293 Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugins/logstash/kibana.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"owner": [
"@elastic/logstash"
],
"group": "observability",
"group": "platform",
"visibility": "private",
"plugin": {
"id": "logstash",
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/monitoring/kibana.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"owner": [
"@elastic/stack-monitoring"
],
"group": "observability",
"group": "platform",
"visibility": "private",
"plugin": {
"id": "monitoring",
Expand All @@ -23,7 +23,6 @@
"share"
],
"optionalPlugins": [
"infra",
"usageCollection",
"home",
"cloud",
Expand Down
16 changes: 12 additions & 4 deletions x-pack/plugins/monitoring/public/components/logs/__snapshots__/logs.test.js.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions x-pack/plugins/monitoring/public/components/logs/logs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const clusterColumns = [
},
];

function getLogsUiLink(clusterUuid, nodeId, indexUuid, sharePlugin, logsIndices) {
function getDiscoverLink(clusterUuid, nodeId, indexUuid, sharePlugin, logsIndices) {
const params = [];
if (clusterUuid) {
params.push(`elasticsearch.cluster.uuid:${clusterUuid}`);
Expand All @@ -126,6 +126,10 @@ function getLogsUiLink(clusterUuid, nodeId, indexUuid, sharePlugin, logsIndices)
const filter = params.join(' and ');
const discoverLocator = sharePlugin.url.locators.get('DISCOVER_APP_LOCATOR');

if (!discoverLocator) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ If for some reason the discover locator is not available, sharePlugin.url.locators.get will return undefined - this is catching this case.

return;
}

const base = discoverLocator.getRedirectUrl({
dataViewSpec: {
id: logsIndices,
Expand Down Expand Up @@ -175,7 +179,7 @@ export class LogsContent extends PureComponent {
}

renderCallout() {
const { capabilities: uiCapabilities, infra, kibanaServices } = Legacy.shims;
const { capabilities: uiCapabilities, kibanaServices } = Legacy.shims;
const show = uiCapabilities.discover && uiCapabilities.discover.show;

const {
Expand All @@ -190,8 +194,9 @@ export class LogsContent extends PureComponent {
if (!enabled || !show) {
return null;
}
const discoverLink = getDiscoverLink(clusterUuid, nodeId, indexUuid, sharePlugin, logsIndices);

return infra ? (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Rendering the callout was guarded by whether infra is available or not, which doesn't make a lot of sense anymore since we provide a link to discover, not infra here.

So it's now guarded by whether we can successfully generate a discover link or not.

return discoverLink ? (
<EuiCallOut
size="m"
title={i18n.translate('xpack.monitoring.logs.listing.calloutTitle', {
Expand All @@ -205,9 +210,7 @@ export class LogsContent extends PureComponent {
defaultMessage="Visit {link} to dive deeper."
values={{
link: (
<EuiLink
href={getLogsUiLink(clusterUuid, nodeId, indexUuid, sharePlugin, logsIndices)}
>
<EuiLink href={discoverLink}>
{i18n.translate('xpack.monitoring.logs.listing.calloutLinkText', {
defaultMessage: 'Discover',
})}
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/monitoring/public/components/logs/logs.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ jest.mock('../../legacy_shims', () => ({
shims: {
getBasePath: () => '',
capabilities: { discover: { show: true } },
infra: {},
},
},
}));
Expand All @@ -24,7 +23,9 @@ const sharePlugin = {
url: {
locators: {
get: () => {
return sharePluginMock.createLocator();
const locatorMock = sharePluginMock.createLocator();
locatorMock.getRedirectUrl.mockReturnValue('http://localhost:5601/app/discover');
return locatorMock;
},
},
},
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/monitoring/public/legacy_shims.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { TriggersAndActionsUIPublicPluginStart } from '@kbn/triggers-actions-ui-
import { TypeRegistry } from '@kbn/triggers-actions-ui-plugin/public/application/type_registry';
import { ActionTypeModel, RuleTypeModel } from '@kbn/triggers-actions-ui-plugin/public/types';
import { UsageCollectionSetup } from '@kbn/usage-collection-plugin/public';
import type { InfraClientStartExports } from '@kbn/infra-plugin/public';
import {
MonitoringStartPluginDependencies,
LegacyMonitoringStartPluginDependencies,
Expand Down Expand Up @@ -74,7 +73,6 @@ export interface IShims {
usageCollection: UsageCollectionSetup;
kibanaServices: CoreStart & { usageCollection: UsageCollectionSetup };
appMountParameters: AppMountParameters;
infra?: InfraClientStartExports;
}

export class Legacy {
Expand All @@ -87,7 +85,6 @@ export class Legacy {
triggersActionsUi,
usageCollection,
appMountParameters,
infra,
}: LegacyMonitoringStartPluginDependencies) {
this._shims = {
toastNotifications: core.notifications.toasts,
Expand Down Expand Up @@ -146,7 +143,6 @@ export class Legacy {
usageCollection,
},
appMountParameters,
infra,
};
}

Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/monitoring/public/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ export class MonitoringPlugin
usageCollection: plugins.usageCollection,
appMountParameters: params,
dataViews: pluginsStart.dataViews,
infra: pluginsStart.infra,
};

Legacy.init({
Expand All @@ -126,7 +125,6 @@ export class MonitoringPlugin
usageCollection: deps.usageCollection,
appMountParameters: deps.appMountParameters,
dataViews: deps.dataViews,
infra: deps.infra,
share: deps.share,
});

Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/monitoring/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export type { MLJobs } from '../server/lib/elasticsearch/get_ml_jobs';
import { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
import { DashboardStart } from '@kbn/dashboard-plugin/public';
import { FleetStart } from '@kbn/fleet-plugin/public';
import type { InfraClientStartExports } from '@kbn/infra-plugin/public';
import { SharePluginStart } from '@kbn/share-plugin/public';
import { ReactNode } from 'react';

Expand All @@ -27,7 +26,6 @@ export interface MonitoringStartPluginDependencies {
dataViews: DataViewsPublicPluginStart;
dashboard?: DashboardStart;
fleet?: FleetStart;
infra?: InfraClientStartExports;
share: SharePluginStart;
}

Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/monitoring/server/lib/setup/collection/get_collection_status.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { featuresPluginMock } from '@kbn/features-plugin/server/mocks';
import { infraPluginMock } from '@kbn/infra-plugin/server/mocks';
import { loggerMock } from '@kbn/logging-mocks';
import { usageCollectionPluginMock } from '@kbn/usage-collection-plugin/server/mocks';
import { configSchema, createConfig } from '../../../config';
Expand Down Expand Up @@ -37,7 +36,6 @@ const mockReq = (
plugins: {
usageCollection: usageCollectionSetup,
features: featuresPluginMock.createSetup(),
infra: infraPluginMock.createSetupContract(),
},
},
},
Expand Down
7 changes: 2 additions & 5 deletions x-pack/plugins/monitoring/server/routes/api/v1/alerts/enable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export function enableAlertsRoute(server: MonitoringCore, npRoute: RouteDependen
async (context, request, response) => {
try {
const alertingContext = await context.alerting;
const infraContext = await context.infra;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ This is the only place the infra plugin was used on the server side - it filled in the space it in some server info logs about rule creation.

Theoretically it would be possible to add the spaces plugin as dependency to get the same information, but it doesn't seem worth it doing so as this is strictly about info logging. So I just removed the space id from the log line

const actionContext = await context.actions;

const alerts = RulesFactory.getAll();
Expand All @@ -39,7 +38,7 @@ export function enableAlertsRoute(server: MonitoringCore, npRoute: RouteDependen

if (!isSufficientlySecure || !hasPermanentEncryptionKey) {
server.log.info(
`Skipping rule creation for "${infraContext.spaceId}" space; Stack Monitoring rules require API keys to be enabled and an encryption key to be configured.`
`Skipping rule creation; Stack Monitoring rules require API keys to be enabled and an encryption key to be configured.`
);
return response.ok({
body: {
Expand Down Expand Up @@ -90,9 +89,7 @@ export function enableAlertsRoute(server: MonitoringCore, npRoute: RouteDependen
alerts.map((alert) => alert.createIfDoesNotExist(rulesClient, actionsClient, actions))
);

server.log.info(
`Created ${createdAlerts.length} alerts for "${infraContext.spaceId}" space`
);
server.log.info(`Created ${createdAlerts.length} alerts`);

return response.ok({ body: { createdAlerts } });
} catch (err) {
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/monitoring/server/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import type {
import type { AlertingApiRequestHandlerContext } from '@kbn/alerting-plugin/server';
import type { RacApiRequestHandlerContext } from '@kbn/rule-registry-plugin/server';
import { AlertingServerSetup, AlertingServerStart } from '@kbn/alerting-plugin/server';
import { InfraPluginSetup, InfraRequestHandlerContext } from '@kbn/infra-plugin/server';
import { LicensingPluginStart } from '@kbn/licensing-plugin/server';
import { FeaturesPluginSetup } from '@kbn/features-plugin/server';
import { EncryptedSavedObjectsPluginSetup } from '@kbn/encrypted-saved-objects-plugin/server';
Expand All @@ -51,14 +50,12 @@ export interface PluginsSetup {
usageCollection?: UsageCollectionSetup;
features: FeaturesPluginSetup;
alerting?: AlertingServerSetup;
infra: InfraPluginSetup;
cloud?: CloudSetup;
}

export type RequestHandlerContextMonitoringPlugin = CustomRequestHandlerContext<{
actions?: ActionsApiRequestHandlerContext;
alerting?: AlertingApiRequestHandlerContext;
infra: InfraRequestHandlerContext;
ruleRegistry?: RacApiRequestHandlerContext;
}>;

Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/monitoring/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"@kbn/cloud-plugin",
"@kbn/encrypted-saved-objects-plugin",
"@kbn/features-plugin",
"@kbn/infra-plugin",
"@kbn/licensing-plugin",
"@kbn/triggers-actions-ui-plugin",
"@kbn/expect",
Expand Down
Loading