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.

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

[Synthetics] Added error track trace to status/tls rule context variable !! #198599

Merged
merged 5 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const ObservabilityUptimeAlertOptional = rt.partial({
'anomaly.start': schemaDate,
configId: schemaString,
'error.message': schemaString,
'error.stack_trace': schemaString,
'host.name': schemaString,
'kibana.alert.context': schemaUnknown,
'kibana.alert.evaluation.threshold': schemaStringOrNumber,
Expand Down

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

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const OBSERVER_NAME = 'observer.name';
export const SERVICE_NAME = 'service.name';
export const OBSERVER_GEO_NAME = 'observer.geo.name';
export const ERROR_MESSAGE = 'error.message';
export const ERROR_STACK_TRACE = 'error.stack_trace';
export const STATE_ID = 'monitor.state.id';

export const CERT_COMMON_NAME = 'tls.server.x509.subject.common_name';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export const getCertsRequestBody = ({
'service',
'labels',
'tags',
'error.message',
'error',
],
collapse: {
field: 'tls.server.hash.sha256',
Expand Down Expand Up @@ -222,6 +222,7 @@ export const processCertsResult = (result: CertificatesResults): CertResult => {
locationId: ping?.observer?.name,
locationName: ping?.observer?.geo?.name,
errorMessage: ping?.error?.message,
errorStackTrace: ping?.error?.stack_trace,
};
});
const total = result.aggregations?.total?.value ?? 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export const syntheticsRuleFieldMap: FieldMap = {
type: 'text',
required: false,
},
'error.stack_trace': {
type: 'wildcard',
maryam-saeidi marked this conversation as resolved.
Show resolved Hide resolved
required: false,
},
'agent.name': {
type: 'keyword',
required: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export const CertType = t.intersection([
'@timestamp': t.string,
serviceName: t.string,
errorMessage: t.string,
errorStackTrace: t.union([t.string, t.null]),
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to add t.null? I am trying to understand how it is different, for example, from errorMessage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because, it can also be null in heartbeat data

Copy link
Member

Choose a reason for hiding this comment

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

So, in heartbeat data, errorMessage is always present but not errorStackTrace? Where do you check the heartbeat data type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no both can be optional, but error_stack can also be null

labels: t.record(t.string, t.string),
tags: t.array(t.string),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
AGENT_NAME,
STATE_ID,
SERVICE_NAME,
ERROR_STACK_TRACE,
} from '../../../common/field_names';
import { OverviewPing } from '../../../common/runtime_types';
import { UNNAMED_LOCATION } from '../../../common/constants';
Expand All @@ -42,6 +43,7 @@ export const getMonitorAlertDocument = (
[OBSERVER_GEO_NAME]: locationNames,
[OBSERVER_NAME]: locationIds,
[ERROR_MESSAGE]: monitorSummary.lastErrorMessage,
[ERROR_STACK_TRACE]: monitorSummary.lastErrorStack,
[AGENT_NAME]: monitorSummary.hostName,
[ALERT_REASON]: monitorSummary.reason,
[STATE_ID]: monitorSummary.stateId,
Expand Down Expand Up @@ -115,6 +117,7 @@ export const getMonitorSummary = ({
monitorName,
monitorType: typeToLabelMap[monitorInfo.monitor?.type] || monitorInfo.monitor?.type,
lastErrorMessage: monitorInfo.error?.message!,
lastErrorStack: monitorInfo.error?.stack_trace!,
maryam-saeidi marked this conversation as resolved.
Show resolved Hide resolved
serviceName: monitorInfo.service?.name,
labels: monitorInfo.labels,
locationName: formattedLocationName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export interface MonitorSummaryStatusRule {
};
stateId?: string;
lastErrorMessage?: string;
lastErrorStack?: string;
timestamp: string;
labels?: Record<string, string>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
CERT_VALID_NOT_AFTER,
CERT_VALID_NOT_BEFORE,
ERROR_MESSAGE,
ERROR_STACK_TRACE,
MONITOR_ID,
MONITOR_NAME,
MONITOR_TYPE,
Expand Down Expand Up @@ -103,6 +104,7 @@ export const getCertSummary = (cert: Cert, expirationThreshold: number, ageThres
configId: cert.configId,
monitorTags: cert.tags,
errorMessage: cert.errorMessage,
errorStackTrace: cert.errorStackTrace,
labels: cert.labels,
};
};
Expand All @@ -123,6 +125,7 @@ export const getTLSAlertDocument = (cert: Cert, monitorSummary: CertSummary, uui
[OBSERVER_GEO_NAME]: monitorSummary.locationName ? [monitorSummary.locationName] : [],
[OBSERVER_NAME]: monitorSummary.locationId ? [monitorSummary.locationId] : [],
[ERROR_MESSAGE]: monitorSummary.errorMessage,
[ERROR_STACK_TRACE]: monitorSummary.errorStackTrace,
'location.id': monitorSummary.locationId ? [monitorSummary.locationId] : [],
'location.name': monitorSummary.locationName ? [monitorSummary.locationName] : [],
labels: cert.labels,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ export const commonMonitorStateI18: Array<{
}
),
},
{
name: 'lastErrorStack',
description: i18n.translate(
'xpack.synthetics.alertRules.monitorStatus.actionVariables.state.lastErrorStack',
{
defaultMessage: 'Monitor last error stack trace.',
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this way, it would be more readable?

Monitor's last error stack trace.

Copy link
Member

@maryam-saeidi maryam-saeidi Nov 1, 2024

Choose a reason for hiding this comment

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

@paulb-elastic @jasonrhodes, who would be the best person to give a quick review of default messages?

This comment is related to the below screenshot, maybe in this instance we just keep it consistent with the rest but in general I was curious about the process

Copy link
Member

Choose a reason for hiding this comment

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

For our team I would expect this would be DeDe, ie the docs rep assigned to the area.

}
),
},
{
name: 'locationName',
description: i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export const uptimeRuleFieldMap: FieldMap = {
type: 'text',
required: false,
},
'error.stack_trace': {
maryam-saeidi marked this conversation as resolved.
Show resolved Hide resolved
type: 'wildcard',
required: false,
},
'agent.name': {
type: 'keyword',
required: false,
Expand Down