Skip to content

[Discover] Exclude Elasticsearch metadata fields from Display in Content Column#213255

Merged
rStelmach merged 9 commits intomainfrom
204771-discover-summary-column-should-not-filter-out-mapped-fields-which-start-with-_-in-observability-profile
Mar 10, 2025
Merged

[Discover] Exclude Elasticsearch metadata fields from Display in Content Column#213255
rStelmach merged 9 commits intomainfrom
204771-discover-summary-column-should-not-filter-out-mapped-fields-which-start-with-_-in-observability-profile

Conversation

@rStelmach
Copy link
Contributor

@rStelmach rStelmach commented Mar 5, 2025

Removed "_" from fields to exclude in the Display and replaced it with Elasticsearch's metadata fields.

@rStelmach rStelmach added bug Fixes for quality problems that affect the customer experience Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:obs-onboarding Observability Onboarding Team labels Mar 5, 2025
@rStelmach rStelmach requested a review from a team as a code owner March 5, 2025 15:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

'_ignored',
'_routing',
'_meta',
'_tier',
Copy link
Contributor

@jughosta jughosta Mar 6, 2025

Choose a reason for hiding this comment

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

Hi @rStelmach,

Looks like FILTER_OUT_FIELDS_PREFIXES_FOR_CONTENT is used for checking whether a field name starts with one of the array items or not. Since we would like to exclude only specific fields, extending this list would not work correctly and we need (for example) to define another array and extend the logic in isFieldAllowed util accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @jughosta, I changed the logic for filtering, let me know if that's what you had in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Now we can remove the new fields from FILTER_OUT_FIELDS_PREFIXES_FOR_CONTENT.

Copy link
Contributor Author

@rStelmach rStelmach Mar 6, 2025

Choose a reason for hiding this comment

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

Oh you are right, I forgot to change it 😄 .

@rStelmach rStelmach requested a review from a team as a code owner March 6, 2025 14:08
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, @rStelmach!

Since it's a bug fix, would be great to backport to the latest versions too. It can be achieved by replacing labels backport:skip and release_note:skip with backport:version, v9.1.0, v9.0.0, v8.19.0, v8.18.0 and release_note:fix.

Another suggestion is to create branches in your own fork instead of pushing to Kibana repo. You can find more info here.

@rStelmach rStelmach added backport:version Backport to applied version labels v9.1.0 v9.0.0 v8.18.0 v8.19.0 release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Mar 7, 2025
…lter-out-mapped-fields-which-start-with-_-in-observability-profile
…lter-out-mapped-fields-which-start-with-_-in-observability-profile
…lter-out-mapped-fields-which-start-with-_-in-observability-profile
…lter-out-mapped-fields-which-start-with-_-in-observability-profile
…lter-out-mapped-fields-which-start-with-_-in-observability-profile
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

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
@kbn/discover-contextual-components 36 38 +2
@kbn/discover-utils 277 278 +1
total +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
discover 921.6KB 921.7KB +149.0B
Unknown metric groups

API count

id before after diff
@kbn/discover-contextual-components 38 40 +2
@kbn/discover-utils 327 328 +1
total +3

History

@rStelmach rStelmach merged commit 49ebf9e into main Mar 10, 2025
9 checks passed
@rStelmach rStelmach deleted the 204771-discover-summary-column-should-not-filter-out-mapped-fields-which-start-with-_-in-observability-profile branch March 10, 2025 12:55
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13765185172

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.18 Backport failed because of merge conflicts
8.x Backport failed because of merge conflicts
9.0 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 213255

Questions ?

Please refer to the Backport tool documentation

@gbamparop
Copy link
Contributor

@rStelmach I think this just needs a backport to 8.19.0 (8.x).

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 12, 2025
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 213255 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 213255 locally

3 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 213255 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 213255 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 213255 locally

rStelmach added a commit to rStelmach/kibana that referenced this pull request Mar 19, 2025
…ent Column (elastic#213255)

Removed "_" from fields to exclude in the Display and replaced it with
Elasticsearch's metadata fields.

(cherry picked from commit 49ebf9e)

# Conflicts:
#	src/platform/packages/shared/kbn-discover-contextual-components/src/data_types/logs/components/summary_column/utils.tsx
@rStelmach
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@jughosta
Copy link
Contributor

9.0.0 needs a backport too

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
…ent Column (elastic#213255)

Removed "_" from fields to exclude in the Display and replaced it with
Elasticsearch's metadata fields.
rStelmach added a commit that referenced this pull request Mar 24, 2025
…n Content Column (#213255) (#215132)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Discover] Exclude Elasticsearch metadata fields from Display in
Content Column (#213255)](#213255)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Robert
Stelmach","email":"60304951+rStelmach@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-03-10T12:55:35Z","message":"[Discover]
Exclude Elasticsearch metadata fields from Display in Content Column
(#213255)\n\nRemoved \"_\" from fields to exclude in the Display and
replaced it with\nElasticsearch's metadata
fields.","sha":"49ebf9e43e7848666fe720afd4365385aee7628e","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Feature:Discover","release_note:fix","backport
missing","Team:obs-ux-logs","backport:version","v9.1.0","v8.19.0"],"title":"[Discover]
Exclude Elasticsearch metadata fields from Display in Content
Column","number":213255,"url":"https://github.com/elastic/kibana/pull/213255","mergeCommit":{"message":"[Discover]
Exclude Elasticsearch metadata fields from Display in Content Column
(#213255)\n\nRemoved \"_\" from fields to exclude in the Display and
replaced it with\nElasticsearch's metadata
fields.","sha":"49ebf9e43e7848666fe720afd4365385aee7628e"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/213255","number":213255,"mergeCommit":{"message":"[Discover]
Exclude Elasticsearch metadata fields from Display in Content Column
(#213255)\n\nRemoved \"_\" from fields to exclude in the Display and
replaced it with\nElasticsearch's metadata
fields.","sha":"49ebf9e43e7848666fe720afd4365385aee7628e"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 24, 2025
@gbamparop
Copy link
Contributor

9.0.0 needs a backport too

@jughosta we're just backporting it to new versions (8.x for 8.19).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels bug Fixes for quality problems that affect the customer experience Feature:Discover Discover Application release_note:fix Team:obs-onboarding Observability Onboarding Team v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Discover] Summary column should not filter out mapped fields which start with _ in Observability profile

5 participants