Skip to content

Commit

Permalink
[Response Ops][Alerting] Revert ignore_malformed changes (elastic#1…
Browse files Browse the repository at this point in the history
…63610)

Reverting elastic#163414 and
elastic#163487

## Summary

@pmuellr uncovered a bug in ES with `ignore_malformed` and datastreams
while working on elastic#154266

> Found what I hope is an ES bug yesterday w/data streams (DS). It
doesn’t like ignore_malformed on the @timestamp field
:slightly_smiling_face:. I think this is a bug since [the doc says
(https://www.elastic.co/guide/en/elasticsearch/reference/current/ignore-malformed.html#ignore-malformed-setting)
Mapping types that don’t support the setting will ignore it if set on
the index level.
I think it’s understandable - the @timestamp field is a key field for DS
(can be overridden) - so you’d not be surprised it’s treated specially.
But … why not just ignore it in that case, like the other mapping types
that are ignored.
I tried overriding ignore_malformed for just that field, and it
complained that I couldn’t use that option on that field! hahahahah
So, we’d be left having to add ignore_malformed to every mapped field in
our mappings, except for @timestamp.
For the time being, I’ve removed all the ignore_malformed stuff in my
AaD DS PR, when using DS, but left it when using alias/index.
Unless someone knows more about this special ignored_malformed /
@timestamp field / data-stream relationship, I’ll boil down a simple
test case and open an issue for ES.

In order to avoid having even more divergent code between serverless &
serverful, we will revert this change until we can confirm a bug with ES
and hopefully get a fix in.
  • Loading branch information
ymao1 authored and benakansara committed Aug 14, 2023
1 parent 19300ea commit 2384836
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ const getIndexTemplatePutBody = (opts?: GetIndexTemplatePutBodyOpts) => {
name: '.alerts-ilm-policy',
rollover_alias: `.alerts-${context ? context : 'test'}.alerts-${namespace}`,
},
'index.mapping.ignore_malformed': true,
'index.mapping.total_fields.limit': 2500,
},
mappings: {
Expand Down Expand Up @@ -641,7 +640,6 @@ describe('Alerts Service', () => {
name: '.alerts-ilm-policy',
rollover_alias: `.alerts-empty.alerts-default`,
},
'index.mapping.ignore_malformed': true,
'index.mapping.total_fields.limit': 2500,
},
mappings: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ const IndexTemplate = (namespace: string = 'default') => ({
name: 'test-ilm-policy',
rollover_alias: `.alerts-test.alerts-${namespace}`,
},
'index.mapping.ignore_malformed': true,
'index.mapping.total_fields.limit': 2500,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export const getIndexTemplate = ({
rollover_alias: indexPatterns.alias,
},
'index.mapping.total_fields.limit': totalFieldsLimit,
'index.mapping.ignore_malformed': true,
},
mappings: {
dynamic: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export default function createAlertsAsDataInstallResourcesTest({ getService }: F
rollover_alias: '.alerts-test.patternfiring.alerts-default',
},
mapping: {
ignore_malformed: 'true',
total_fields: {
limit: '2500',
},
Expand Down Expand Up @@ -197,7 +196,6 @@ export default function createAlertsAsDataInstallResourcesTest({ getService }: F
});

expect(contextIndex[indexName].settings?.index?.mapping).to.eql({
ignore_malformed: 'true',
total_fields: {
limit: '2500',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export default ({ getService }: FtrProviderContext) => {
};
};

// FAILING ES PROMOTION: https://github.com/elastic/kibana/issues/154277
describe('Non ECS fields in alert document source', () => {
before(async () => {
await esArchiver.load(
Expand Down Expand Up @@ -258,8 +259,7 @@ export default ({ getService }: FtrProviderContext) => {

// we don't validate it because geo_point is very complex type with many various representations: array, different object, string with few valid patterns
// more on geo_point type https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html
// since .alerts-* indices allow _ignore_malformed option, alert will be created for this document
it('should not fail creating alert when ECS field mapping is geo_point', async () => {
it('should fail creating alert when ECS field mapping is geo_point', async () => {
const document = {
client: {
geo: {
Expand All @@ -269,11 +269,12 @@ export default ({ getService }: FtrProviderContext) => {
},
};

const { errors, alertSource } = await indexAndCreatePreviewAlert(document);

expect(errors).toEqual([]);
const { errors } = await indexAndCreatePreviewAlert(document);

expect(alertSource).toHaveProperty('client.geo.location', 'test test');
expect(errors[0]).toContain('Bulk Indexing of signals failed');
expect(errors[0]).toContain(
'failed to parse field [client.geo.location] of type [geo_point]'
);
});

it('should strip invalid boolean values and left valid ones', async () => {
Expand Down

0 comments on commit 2384836

Please sign in to comment.