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

Add ECS data to data views #180543

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add ECS data to data views #180543

wants to merge 1 commit into from

Conversation

kertal
Copy link
Member

@kertal kertal commented Apr 10, 2024

Summary

Exploration what happens if we enrich data view fields data server side with ECS information to display descriptions in the UI:

image

Note: this can't be shipped as it would add the description to every data view fields request, which would lead to a lot of redundant data being transfered

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@kertal
Copy link
Member Author

kertal commented Apr 10, 2024

/ci

@kibana-ci
Copy link
Collaborator

kibana-ci commented Apr 10, 2024

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #68 / apis transform basic license transform /internal/transform/schedule_now_transforms bulk schedule "after each" hook for "should schedule multiple transforms by transformIds, even if one of the transformIds is invalid"
  • [job] [logs] FTR Configs #68 / apis transform basic license transform /internal/transform/schedule_now_transforms bulk schedule "after each" hook for "should schedule multiple transforms by transformIds"

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
data 2619 2620 +1
dataViews 370 371 +1
total +2

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
data 31 32 +1
dataViews 0 1 +1
total +2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViews 61.4KB 61.5KB +56.0B
Unknown metric groups

API count

id before after diff
data 3285 3286 +1
dataViews 1070 1071 +1
total +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kertal kertal changed the title Add ESC data to data views Add ECS data to data views Apr 16, 2024
EcsFlat[fieldName as keyof typeof EcsFlat]?.description
) {
field.ecsDescription = EcsFlat[fieldName as keyof typeof EcsFlat].description;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make it optional for the field caps request to control the response size better? For example enrich with ecs descriptions only if includeEcsMeta param is passed to the API call.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, because adding it every time would make it too expensive. maybe it could be even more granular, something like
includeEcsMeta=all -> return all ECS Meta data?
includeEcsMeta=description,short,... -> return selected ECS Meta data?

Then it would be flexible, e.g. for the showing the description in the fields popover just single field + description could be requested. By default, when a data view requests all the fields, just a single small field could be requested, sufficient to add priority, if set

Copy link
Member Author

Choose a reason for hiding this comment

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

wdyt @mattkime ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will create an issue for that

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect that ECS data will be loaded for a small number of fields at a time? I think you said so much but I thought I'd verify.

Will the ECS data be loaded for fields individually or for a small number at a time?

Does field_caps data inform how ECS data is returned?

Copy link
Member Author

@kertal kertal May 3, 2024

Choose a reason for hiding this comment

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

Do we expect that ECS data will be loaded for a small number of fields at a time? I think you said so much but I thought I'd verify.

it depends on the use case, when showing the description of a field, yes, just a single field. But for prioritization, telemetry, this number of fields can be big (that's why the API needs to be flexible, to just serve the ECS data that's needed)

Will the ECS data be loaded for fields individually or for a small number at a time?

Depends on the use case, in this PR it added for all fields when ecs.version is part of the field list. this is very simple but would lead to redundant information, and therefore more network traffic in ECS use cases. This is something we should omit, that's why ECS enrichment needs to be granular

Does field_caps data inform how ECS data is returned?

No, ECS data is being added / enriched on top of the field_caps result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants