-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Discover] Add logs source and document contexts #184601
Changes from 55 commits
9e823b9
12e56b1
83f0696
0a44de8
226cdd7
4ca0e67
f64aef3
ae19e34
cbf6601
926ad4a
d8fbdb7
3afc77e
defad74
efa94d1
0115332
fbf856b
9c1b031
ff8aaeb
0964db1
35f58a1
6a871aa
2e0b76e
2db3ecc
2ea64d1
7e93b69
53ffd34
19eba4c
4321c75
faa68c2
e953b41
a5f2de8
fcc8ebc
3d8ff5a
15ea447
3ce74dd
45b618b
c05c675
bb7d439
314b033
5ecc0dd
516e996
e443c32
c2ebbde
5905c46
69d655d
2f68870
28a5d2f
08db9ae
5a32251
f657706
8ef60f2
7283f3c
9d09a4d
c3c9f61
4b925c9
326ac11
07a6ecc
6957b86
23ad7d3
aab6cb6
baf9613
db98177
9ecedd5
f2f5462
f55015f
d7c7f55
35a32ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { createRegExpPatternFrom } from './create_regexp_pattern_from'; | ||
|
||
describe('createRegExpPatternFrom should create a regular expression starting from a string that', () => { | ||
const regExpPattern = createRegExpPatternFrom('logs'); | ||
|
||
it('tests positive for single index patterns starting with the passed base pattern', () => { | ||
expect('logs*').toMatch(regExpPattern); | ||
expect('logs-*').toMatch(regExpPattern); | ||
expect('logs-*-*').toMatch(regExpPattern); | ||
expect('logs-system.syslog-*').toMatch(regExpPattern); | ||
|
||
expect('logss*').not.toMatch(regExpPattern); | ||
expect('logss-*').not.toMatch(regExpPattern); | ||
expect('metrics*').not.toMatch(regExpPattern); | ||
expect('metrics-*').not.toMatch(regExpPattern); | ||
}); | ||
|
||
it('tests positive for single index patterns containing the passed base pattern', () => { | ||
expect('foo-logs*').toMatch(regExpPattern); | ||
expect('foo-logs-*').toMatch(regExpPattern); | ||
expect('foo-logs-*-*').toMatch(regExpPattern); | ||
expect('foo-logs-system.syslog-*').toMatch(regExpPattern); | ||
|
||
expect('foo-logss*').not.toMatch(regExpPattern); | ||
expect('foo-logss-*').not.toMatch(regExpPattern); | ||
expect('foo-metrics*').not.toMatch(regExpPattern); | ||
expect('foo-metrics-*').not.toMatch(regExpPattern); | ||
}); | ||
|
||
it('tests positive for single index patterns with CCS prefixes', () => { | ||
expect('cluster1:logs-*').toMatch(regExpPattern); | ||
expect('cluster1:logs-*-*').toMatch(regExpPattern); | ||
expect('cluster1:logs-system.syslog-*').toMatch(regExpPattern); | ||
expect('cluster1:logs-system.syslog-default').toMatch(regExpPattern); | ||
|
||
expect('cluster1:logss*').not.toMatch(regExpPattern); | ||
expect('cluster1:logss-*').not.toMatch(regExpPattern); | ||
expect('cluster1:metrics*').not.toMatch(regExpPattern); | ||
expect('cluster1:metrics-*').not.toMatch(regExpPattern); | ||
}); | ||
|
||
it('tests positive for multiple index patterns comma-separated if all of them individually match the criteria', () => { | ||
expect('logs-*,cluster1:logs-*').toMatch(regExpPattern); | ||
expect('cluster1:logs-*,cluster2:logs-*').toMatch(regExpPattern); | ||
expect('*:logs-system.syslog-*,*:logs-system.errors-*').toMatch(regExpPattern); | ||
|
||
expect('*:metrics-system.syslog-*,logs-system.errors-*').not.toMatch(regExpPattern); | ||
}); | ||
|
||
it('tests positive for patterns with trialing commas', () => { | ||
tonyghiani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
expect('logs-*,').toMatch(regExpPattern); | ||
expect('cluster1:logs-*,logs-*,').toMatch(regExpPattern); | ||
}); | ||
|
||
it('tests negative for patterns with spaces and unexpected commas', () => { | ||
expect('cluster1:logs-*,clust,er2:logs-*').not.toMatch(regExpPattern); | ||
expect('cluster1:logs-*, cluster2:logs-*').not.toMatch(regExpPattern); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,17 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
export const buildIndexPatternRegExp = (basePatterns: string[]) => { | ||
export const createRegExpPatternFrom = (basePatterns: string | string[]) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note This utility function creates a regexp based on base pattern keywords. |
||
const patterns = Array.isArray(basePatterns) ? basePatterns : [basePatterns]; | ||
// Create the base patterns union with strict boundaries | ||
const basePatternGroup = `\\b(${basePatterns.join('|')})\\b([^,\\s]+)?`; | ||
const basePatternGroup = `[^,\\s]*\\b(${patterns.join('|')})\\b([^,\\s]*)?`; | ||
// Apply base patterns union for local and remote clusters | ||
const localAndRemotePatternGroup = `((${basePatternGroup})|([^:,\\s]+:${basePatternGroup}))`; | ||
const localAndRemotePatternGroup = `((${basePatternGroup})|([^:,\\s]*:${basePatternGroup}))`; | ||
// Handle trailing comma and multiple pattern concatenation | ||
return new RegExp(`^${localAndRemotePatternGroup}(,${localAndRemotePatternGroup})*(,$|$)`, 'i'); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { testPatternAgainstAllowedList } from './test_pattern_against_allowed_list'; | ||
|
||
describe('testPatternAgainstAllowedList', () => { | ||
const allowedList = ['foo-logs-bar', /^\b(logs)\b([^,\s]*)/i]; | ||
|
||
it('should return true if the passed input matches any string or regexp of the passed list', () => { | ||
expect(testPatternAgainstAllowedList(allowedList)('logs-*')).toBeTruthy(); | ||
expect(testPatternAgainstAllowedList(allowedList)('logs-*-*')).toBeTruthy(); | ||
expect(testPatternAgainstAllowedList(allowedList)('logs-system.syslog-*')).toBeTruthy(); | ||
expect(testPatternAgainstAllowedList(allowedList)('foo-logs-bar')).toBeTruthy(); | ||
|
||
expect(testPatternAgainstAllowedList(allowedList)('logss-*')).toBeFalsy(); | ||
expect(testPatternAgainstAllowedList(allowedList)('metrics*')).toBeFalsy(); | ||
expect(testPatternAgainstAllowedList(allowedList)('metrics-*')).toBeFalsy(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
export const testPatternAgainstAllowedList = | ||
(allowedList: Array<string | RegExp>) => (value: string) => { | ||
for (const allowedItem of allowedList) { | ||
const isMatchingString = typeof allowedItem === 'string' && value === allowedItem; | ||
const isMatchingRegExp = allowedItem instanceof RegExp && allowedItem.test(value); | ||
|
||
if (isMatchingString || isMatchingRegExp) { | ||
return true; | ||
} | ||
} | ||
|
||
// If no match is found in the allowedList, return false | ||
return false; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,5 @@ | |
|
||
export * from './types'; | ||
export * from './utils'; | ||
|
||
export * from './logs_context_service'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { createRegExpPatternFrom, testPatternAgainstAllowedList } from '@kbn/data-view-utils'; | ||
|
||
export interface LogsContextService { | ||
isLogsIndexPattern(indexPattern: unknown): boolean; | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-empty-interface | ||
export interface LogsContextServiceDeps { | ||
// We will probably soon add uiSettings as a dependency | ||
// to consume user configured indices | ||
} | ||
|
||
export const createLogsContextService = (_deps: LogsContextServiceDeps = {}) => { | ||
// This is initially an hard-coded set of well-known base patterns, | ||
// we can extend this allowed list with any setting coming from uiSettings | ||
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note For additional context on the upcoming setting, check https://github.com/elastic/observability-dev/issues/3498 |
||
const ALLOWED_LOGS_DATA_SOURCES = [ | ||
createRegExpPatternFrom(['log', 'logs', 'logstash', 'auditbeat', 'filebeat', 'winlogbeat']), | ||
]; | ||
|
||
const isLogsIndexPattern = (indexPattern: unknown) => { | ||
return ( | ||
typeof indexPattern === 'string' && | ||
testPatternAgainstAllowedList(ALLOWED_LOGS_DATA_SOURCES)(indexPattern) | ||
); | ||
}; | ||
|
||
return { | ||
isLogsIndexPattern, | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
export { createLogDocumentProfileProvider } from './profile'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { buildDataTableRecord } from '@kbn/discover-utils'; | ||
import { DocumentType } from '../../profiles'; | ||
import { createContextAwarenessMocks } from '../../__mocks__'; | ||
import { createLogDocumentProfileProvider } from './profile'; | ||
|
||
const mockServices = createContextAwarenessMocks().profileProviderServices; | ||
|
||
describe('logDocumentProfileProvider', () => { | ||
const logDocumentProfileProvider = createLogDocumentProfileProvider(mockServices); | ||
const RESOLUTION_MATCH = { | ||
isMatch: true, | ||
context: { | ||
type: DocumentType.Log, | ||
}, | ||
}; | ||
const RESOLUTION_MISMATCH = { | ||
isMatch: false, | ||
}; | ||
|
||
it('matches records with the correct data stream type', () => { | ||
expect( | ||
logDocumentProfileProvider.resolve({ | ||
record: buildMockRecord('logs-2000-01-01', { | ||
'data_stream.type': ['logs'], | ||
}), | ||
}) | ||
).toEqual(RESOLUTION_MATCH); | ||
}); | ||
|
||
it('matches records with fields prefixed with "log."', () => { | ||
expect( | ||
logDocumentProfileProvider.resolve({ | ||
record: buildMockRecord('logs-2000-01-01', { | ||
'log.level': ['INFO'], | ||
}), | ||
}) | ||
).toEqual(RESOLUTION_MATCH); | ||
}); | ||
|
||
it('matches records with indices matching the allowed pattern', () => { | ||
expect( | ||
logDocumentProfileProvider.resolve({ | ||
record: buildMockRecord('logs-2000-01-01'), | ||
}) | ||
).toEqual(RESOLUTION_MATCH); | ||
expect( | ||
logDocumentProfileProvider.resolve({ | ||
record: buildMockRecord('remote_cluster:filebeat'), | ||
}) | ||
).toEqual(RESOLUTION_MATCH); | ||
}); | ||
|
||
it('does not match records with neither characteristic', () => { | ||
expect( | ||
logDocumentProfileProvider.resolve({ | ||
record: buildMockRecord('another-index'), | ||
}) | ||
).toEqual(RESOLUTION_MISMATCH); | ||
}); | ||
}); | ||
|
||
const buildMockRecord = (index: string, fields: Record<string, unknown[]> = {}) => | ||
buildDataTableRecord({ | ||
_id: '', | ||
_index: index, | ||
fields: { | ||
_index: index, | ||
...fields, | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @weltenwort , I remember there was a discussion regarding the code ownership.
Thinking about that doesn't it make sense to have
olly
profile providers in a separate package without having any impact on Discover's code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my knowledge the state of discussions was to keep everything in a unified directory structure for now so it's easily discoverable. @davismcphee have there been any further conversations on this topic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay..got it. I remember @davismcphee's comment regarding keeping registeration inside Discover's directory structure but was not sure about the complete profile customization code. Sorry if i did not have full context.
The reason I ask this is I am also working on plan for security solution on similar lines and would be soon working on security related profiles providers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely have a preference for keeping the registrations inside Discover, but I'm a bit more flexible on where the provider code actually lives if solution teams have strong preferences here. The main thing is that Data Discovery is expected to be responsible for the overall experience in Discover and aware of when things change, so wherever the code lives, we should share ownership in
CODEOWNERS
.The organization in this PR makes sense to me because we only need to update
CODEOWNERS
for a single folder, but I'm open to discuss it if Security has strong opinions otherwise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I agree.
I do not have any strong opinions as I am still trying to understand and weigh in options.
Regarding the provider code, I am thinking it should not be too coupled of with discover's code. Provider code will be dependent on both
solution specific utils
(for example, o11y is separating out its utils inkbn-data-view-utils
) and discover code as well.Imo, If code remains separate, we will take care to extract discover's code in
discover-utils
package, otherwise it is very easy for provider code be dependent on discover's core code and later when it grows big, it will be difficult to take it out.May be you all have already talked about it but I am just thinking out loud here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a reasonable goal π
This is a fair concern, but I also wonder how far we can get with just being disciplined about what goes into the providers. In general I also don't think providers should have much dependence on Discover's core codebase (e.g. they shouldn't have access to internal state), and should mainly just depend on types and some utility functions, which typically should be fairly easy to pull out into packages as needed (a lot of our types and utilities already live in packages like
kbn-discover-utils
andkbn-data-view-utils
).