[Discover] Add logs source and document contexts #184601
Conversation
| // this.rootProfileService.registerProvider(o11yRootProfileProvider); | ||
| // this.dataSourceProfileService.registerProvider(logsDataSourceProfileProvider); | ||
| // this.documentProfileService.registerProvider(logDocumentProfileProvider); | ||
| const providerServices = createProfileProviderServices(); |
There was a problem hiding this comment.
Note
Profiles are created using factory functions which receive the providerServices. This services can be passed anything required from core, pluginsStart or any dependencies, crafting the external services a profile might use to resolve its context
botelastic
bot
added
the
ci:project-deploy-observability
botelastic
bot
added
the
Team:obs-ux-infra_services
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
packages/kbn-data-view-utils/src/utils/create_regexp_pattern_from.test.ts
Outdated
Show resolved
Hide resolved
…rom.test.ts Co-authored-by: Kerry Gallagher <[email protected]>
There was a problem hiding this comment.
Nice work! I'm sure we'll find ways to refine the heuristics over time, but this seems like a great initial implementation. I also like the way the code is organized and think it sets a good example of patterns to follow for future implementations. I left one minor comment, but otherwise this all LGTM 👍
There's an increase to the page load bundle that I'd like to avoid, but that will only be temporary until #185905 is merged where we'll start importing the profile providers async when they're needed.
| const getIndices = getFieldValues('_index'); | ||
|
|
||
| const hasFieldsWithPrefix = (prefix: string) => (record: DataTableRecord) => { | ||
| return Object.keys(record.flattened).some((field) => field.startsWith(prefix)); |
There was a problem hiding this comment.
We sometimes deal with records in Discover containing thousands of fields. We're deferring document context resolution until needed anyway, but a for...of loops would be more efficient here vs allocating a new array for each document with Object.keys.
There was a problem hiding this comment.
I think you meant using a for...in loop over the properties in record.flattened to check whether any of them starts with the passed prefix, for...of is only for iterables.
In the case of for...in, it is natively faster but requires an additional check on the object's own properties only, which makes the performance almost the same.
For readability I'd rather leave it like this, here you can also find a performance benchmark for the different approaches which highlights a relatively small difference.
There was a problem hiding this comment.
Yes, definitely meant for...in 🙂 and I was thinking a bit more about allocations when we unfortunately encounter docs with something like 80,000 fields, but this optimization might be premature, so it makes sense to leave it as is and look into it further if it ever becomes an issue.
src/plugins/discover/public/types.ts
Outdated
There was a problem hiding this comment.
Nice 👌 about time these types were moved out of plugin.tsx.
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
kibanamachine
added
v8.15.0
backport:skip
📝 Summary
This PR adds basic implementations for resolving "logs" data source and document contexts to their respective profiles. Due to the limited set of profile customization points the new profiles are empty.
🔍 Implementation details
profile_providersfolder incontext_awareness.@kbn/discover-utilsand@kbn/data-view-utilspackages.Document Level Logs Resolution
The document logs context resolution is performed with the following criteria, as far as one complies, the context will be evaluated as a match:
data_stream.typefield exists on the document and it's equal tologslog.)_indexfield exists and tests positive against the allowed indices from the built-in definition/ settings.Data Source Logs Resolution
The data source logs context resolution is performed with the following criteria, as far as one complies, the context will be evaluated as a match:
🕵️♀️ Review notes
Note
Notes in this format have been left through the PR to give additional context about some choices, any further feedback is welcome