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

[Stack Monitoring] Use server.publicBaseUrl in Alert links #131154

Merged
merged 3 commits into from
May 2, 2022
Merged

[Stack Monitoring] Use server.publicBaseUrl in Alert links #131154

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ac3e7a
[Stack Monitoring] Use server.publicBaseUrl in Alert links (#127126)
miltonhultgren Apr 28, 2022
ee80d4f
Use empty string as fallback value
miltonhultgren Apr 29, 2022
264d31f
Merge branch 'main' into 127126-sm-alert-kibana-url
kibanamachine May 2, 2022
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
5 changes: 4 additions & 1 deletion x-pack/plugins/monitoring/server/alerts/base_rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ export class BaseRule {
if (ccs) {
globalState.push(`ccs:${ccs}`);
}
return `${Globals.app.url}/app/monitoring#/${link}?_g=(${globalState.toString()})`;

const baseUrl = Globals.app.url || '(server.publicBaseUrl)';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how we should react if the setting isn't configured, the default is simply 'undefined'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could use the serverInfo as a fallback, or make it a relative path URL.

Copy link
Member

Choose a reason for hiding this comment

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

We expose the kibana base path itself as an action context variable within alerting, and leave it "set" to undefined if the config is not set. It renders as the empty string. Customers can "optionally" add it via a section block like, so it won't render anything if it's undefined:

Note you can one-line this multi-line template.

{{#kibanaBaseUrl}}
  [head to Kibana]({{kibanaBaseUrl}})
{{/kibanaBaseUrl}}


return `${baseUrl}/app/monitoring#/${link}?_g=(${globalState.toString()})`;
Copy link
Member

Choose a reason for hiding this comment

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

Since the input may be coming from the user (cloud sets the kibanaBaseUrl and it's not overridable by customers), would be best to use something like a relative URL "joiner" (not sure what the current flavor would be). Reason is, the customer may or may not be adding an trailing / to the URL, and you don't want to generate ...//app/... (though that might well work in some envs)

I believe I've cheated and used path.join() for this as well, but ... using a url or URL or whatever is correct these days would be better.

Copy link
Contributor Author

@miltonhultgren miltonhultgren Apr 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Screenshot 2022-04-29 at 14 33 41

The Kibana docs state this value cannot end in /, so this should be safe?
I tested a few different configuration and kept getting errors about this so Kibana wouldn't start.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, if that's the case, shouldn't need a special join!

}
}
8 changes: 2 additions & 6 deletions x-pack/plugins/monitoring/server/static_globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { CoreSetup, ElasticsearchClient, Logger, PluginInitializerContext } from '@kbn/core/server';
import url from 'url';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { MonitoringConfig } from './config';
import { PluginsSetup } from './types';
Expand All @@ -32,7 +31,7 @@ export type EndpointTypes =
export type ClientParams = estypes.SearchRequest | undefined;

interface IAppGlobals {
url: string;
url?: string;
isCloud: boolean;
config: MonitoringConfig;
getLogger: GetLogger;
Expand Down Expand Up @@ -79,11 +78,8 @@ export class Globals {
return body;
});

const { protocol, hostname, port } = coreSetup.http.getServerInfo();
const pathname = coreSetup.http.basePath.serverBasePath;

Globals._app = {
url: url.format({ protocol, hostname, port, pathname }),
url: coreSetup.http.basePath.publicBaseUrl,
isCloud: setupPlugins.cloud?.isCloudEnabled || false,
config,
getLogger,
Expand Down