Remaining work attributes table#224723
Conversation
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
e38a45a to
6e7c325
Compare
3b39533 to
13ed0bc
Compare
| <EuiFlexItem grow={false}> | ||
| <EuiFlexGroup | ||
| responsive={false} | ||
| wrap={true} |
There was a problem hiding this comment.
Nit
| wrap={true} | |
| wrap |
| const noFields = | ||
| groupedFields.attributesFields.length === 0 && | ||
| groupedFields.resourceAttributesFields.length === 0 && | ||
| groupedFields.scopeAttributesFields.length === 0; | ||
|
|
There was a problem hiding this comment.
Nit: to avoid repeating groupedFields, we can do:
| const noFields = | |
| groupedFields.attributesFields.length === 0 && | |
| groupedFields.resourceAttributesFields.length === 0 && | |
| groupedFields.scopeAttributesFields.length === 0; | |
| const {attributesFields, resourceAttributesFields, scopeAttributesFields} = groupedFields | |
| const noFields = | |
| attributesFields.length === 0 && | |
| resourceAttributesFields.length === 0 && | |
| scopeAttributesFields.length === 0; | |
Maybe define them on top, and use them everywhere
There was a problem hiding this comment.
it can be simplified a bit more
const noFields = Object.values(groupedFields).every(
(arr) => Array.isArray(arr) && arr.length === 0
);
...c/components/observability/attributes/doc_viewer_attributes_overview/attributes_overview.tsx
Show resolved
Hide resolved
| const fieldDisplayName = displayName | ||
| ? displayName | ||
| : fieldMapping && fieldMapping.displayName | ||
| ? fieldMapping.displayName | ||
| : fieldName; |
There was a problem hiding this comment.
Nit we can simplify this to avoid chaining conditions, something like:
| const fieldDisplayName = displayName | |
| ? displayName | |
| : fieldMapping && fieldMapping.displayName | |
| ? fieldMapping.displayName | |
| : fieldName; | |
| const dipslayNameFromFieldMapping = fieldMapping?.displayName ? fieldMapping.displayName : fieldName; | |
| const fieldDisplayName = displayName ?? dipslayNameFromFieldMapping; |
davismcphee
left a comment
There was a problem hiding this comment.
Code-only review. Data Discovery changes look good for the most part, but I noticed an issue related to field display names in the Table tab.
src/platform/packages/shared/kbn-unified-doc-viewer/src/components/field_name/field_name.tsx
Outdated
Show resolved
Hide resolved
src/platform/plugins/shared/unified_doc_viewer/public/components/doc_viewer_table/field_row.ts
Outdated
Show resolved
Hide resolved
src/platform/plugins/shared/unified_doc_viewer/public/components/doc_viewer_table/field_row.ts
Outdated
Show resolved
Hide resolved
|
@jennypavlova No idea what was wrong with the position of the tooltip, I just changed position and now looks better |
| [allFields, shouldShowFieldHandler] | ||
| ); | ||
|
|
||
| const groupedFields = useMemo(() => { |
There was a problem hiding this comment.
Can we move the field grouping functionality into a separate utility file to keep things cleaner and add some unit tests?
|
|
||
| if (lowerFieldName.startsWith('resource.attributes.')) { | ||
| resourceAttributesFields.push(fieldName); | ||
| resourceAttributesFields.push({ |
There was a problem hiding this comment.
There’s some repetition in this approach.
Can we improve it a bit? We could use a mapping something like or something similar
const fieldTypeMap = [
{ prefix: 'resource.attributes.', target: resourceAttributesFields },
{ prefix: 'scope.attributes.', target: scopeAttributesFields },
{ prefix: 'attributes.', target: attributesFields },
];
for (const { prefix, target } of fieldTypeMap) {
if (lowerFieldName.startsWith(prefix)) {
target.push({
name: fieldName,
displayName: getAttributeDisplayName(fieldName),
});
}
}
```
wdyt?
There was a problem hiding this comment.
A forEach() will work better here
| return acc; | ||
| } | ||
|
|
||
| if (isEsqlMode && areNullValuesHidden && flattened[fieldName] == null) { |
There was a problem hiding this comment.
in lines 85-95 all return acc. Can we combine them?
There was a problem hiding this comment.
yeah, I think in this case is better to use a forEach() I will modify and extract the function in to a separate file + unit tests
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| export function getAttributeDisplayName(fieldName: string): string { |
There was a problem hiding this comment.
Could you please add some tests for this function?
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
davismcphee
left a comment
There was a problem hiding this comment.
Confirmed the custom label issue I was seeing is resolved now, Data Discovery changes LGTM 👍
kpatticha
left a comment
There was a problem hiding this comment.
Thanks for the quick fixes, great work 💯
|
Starting backport for target branches: 8.19 https://github.com/elastic/kibana/actions/runs/15901517587 |
Closes elastic#221928 #### Add ES|QL logic https://github.com/user-attachments/assets/d29f939a-7b82-4873-92d4-8210c2202339 #### Empty message for accordion - Empty message when there are no attributes fields at all - For now we kept the accordion closed when fields count is zero, with an empty message inside, waiting for UI/UX team to review this implementation <img width="524" alt="Screenshot 2025-06-24 at 12 27 18" src="https://github.com/user-attachments/assets/4015ed6a-5977-486d-93e6-d8b5714af9fd" /> #### Simplify attribute display names - the field name should not show the full field name. The tooltip will show both, simplify and full name, this is part of the implementation `FieldName` component from platform <img width="624" alt="Screenshot 2025-06-24 at 12 19 48" src="https://github.com/user-attachments/assets/634b4ef0-0934-4721-9217-334286b6464a" /> <img width="624" alt="Screenshot 2025-06-24 at 12 20 07" src="https://github.com/user-attachments/assets/bdc6de9c-784f-4c78-bf18-1f37b645429d" /> #### Filtering controls use full field name https://github.com/user-attachments/assets/7858d803-271e-4913-9aae-385dd7bc9e25 #### Add explanatory tooltip for attribute namespaces <img width="525" alt="Screenshot 2025-06-24 at 12 24 33" src="https://github.com/user-attachments/assets/a76b1419-c1d9-4e46-a289-a819b7533b18" /> <img width="525" alt="Screenshot 2025-06-24 at 12 24 51" src="https://github.com/user-attachments/assets/e48b19a3-85a8-4a13-b527-3a4494aef2af" /> <img width="525" alt="Screenshot 2025-06-24 at 12 24 57" src="https://github.com/user-attachments/assets/50501672-4d75-43ce-b61b-646108b4b14a" /> ### Test: #### How to generate OTel data - Follow https://github.com/smith/elastic-stack-docker-compose?tab=readme-ov-file#elastic-stack-docker-compose #### How to test - Make sure your solution view is Observability - update your `kibana.yml` ``` discover.experimental.enabledProfiles: - observability-root-profile-with-attributes-tab # if you want to test it with the additional profiles add the following to your `kibana.yaml` - observability-traces-data-source-profile - observability-traces-transaction-document-profile - observability-traces-span-document-profile ``` (cherry picked from commit 75ba373)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.19`: - [Remaining work attributes table (#224723)](#224723) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Miriam","email":"31922082+MiriamAparicio@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-06-26T12:13:27Z","message":"Remaining work attributes table (#224723)\n\nCloses https://github.com/elastic/kibana/issues/221928\n\n#### Add ES|QL logic\n\n\nhttps://github.com/user-attachments/assets/d29f939a-7b82-4873-92d4-8210c2202339\n\n#### Empty message for accordion\n\n- Empty message when there are no attributes fields at all\n- For now we kept the accordion closed when fields count is zero, with\nan empty message inside, waiting for UI/UX team to review this\nimplementation\n\n<img width=\"524\" alt=\"Screenshot 2025-06-24 at 12 27 18\"\nsrc=\"https://github.com/user-attachments/assets/4015ed6a-5977-486d-93e6-d8b5714af9fd\"\n/>\n\n#### Simplify attribute display names\n\n- the field name should not show the full field name. The tooltip will\nshow both, simplify and full name, this is part of the implementation\n`FieldName` component from platform\n\n<img width=\"624\" alt=\"Screenshot 2025-06-24 at 12 19 48\"\nsrc=\"https://github.com/user-attachments/assets/634b4ef0-0934-4721-9217-334286b6464a\"\n/>\n\n<img width=\"624\" alt=\"Screenshot 2025-06-24 at 12 20 07\"\nsrc=\"https://github.com/user-attachments/assets/bdc6de9c-784f-4c78-bf18-1f37b645429d\"\n/>\n\n#### Filtering controls use full field name\n\n\nhttps://github.com/user-attachments/assets/7858d803-271e-4913-9aae-385dd7bc9e25\n\n#### Add explanatory tooltip for attribute namespaces\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 33\"\nsrc=\"https://github.com/user-attachments/assets/a76b1419-c1d9-4e46-a289-a819b7533b18\"\n/>\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 51\"\nsrc=\"https://github.com/user-attachments/assets/e48b19a3-85a8-4a13-b527-3a4494aef2af\"\n/>\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 57\"\nsrc=\"https://github.com/user-attachments/assets/50501672-4d75-43ce-b61b-646108b4b14a\"\n/>\n\n\n### Test:\n#### How to generate OTel data\n- Follow\nhttps://github.com/smith/elastic-stack-docker-compose?tab=readme-ov-file#elastic-stack-docker-compose\n\n#### How to test\n- Make sure your solution view is Observability\n- update your `kibana.yml` \n\n```\ndiscover.experimental.enabledProfiles:\n - observability-root-profile-with-attributes-tab\n # if you want to test it with the additional profiles add the following to your `kibana.yaml` \n - observability-traces-data-source-profile\n - observability-traces-transaction-document-profile\n - observability-traces-span-document-profile\n```","sha":"75ba373fbdd6a2ee52da6a1e9604c6ff7036b704","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:obs-ux-infra_services","backport:version","v9.1.0","v8.19.0"],"title":"Remaining work attributes table","number":224723,"url":"https://github.com/elastic/kibana/pull/224723","mergeCommit":{"message":"Remaining work attributes table (#224723)\n\nCloses https://github.com/elastic/kibana/issues/221928\n\n#### Add ES|QL logic\n\n\nhttps://github.com/user-attachments/assets/d29f939a-7b82-4873-92d4-8210c2202339\n\n#### Empty message for accordion\n\n- Empty message when there are no attributes fields at all\n- For now we kept the accordion closed when fields count is zero, with\nan empty message inside, waiting for UI/UX team to review this\nimplementation\n\n<img width=\"524\" alt=\"Screenshot 2025-06-24 at 12 27 18\"\nsrc=\"https://github.com/user-attachments/assets/4015ed6a-5977-486d-93e6-d8b5714af9fd\"\n/>\n\n#### Simplify attribute display names\n\n- the field name should not show the full field name. The tooltip will\nshow both, simplify and full name, this is part of the implementation\n`FieldName` component from platform\n\n<img width=\"624\" alt=\"Screenshot 2025-06-24 at 12 19 48\"\nsrc=\"https://github.com/user-attachments/assets/634b4ef0-0934-4721-9217-334286b6464a\"\n/>\n\n<img width=\"624\" alt=\"Screenshot 2025-06-24 at 12 20 07\"\nsrc=\"https://github.com/user-attachments/assets/bdc6de9c-784f-4c78-bf18-1f37b645429d\"\n/>\n\n#### Filtering controls use full field name\n\n\nhttps://github.com/user-attachments/assets/7858d803-271e-4913-9aae-385dd7bc9e25\n\n#### Add explanatory tooltip for attribute namespaces\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 33\"\nsrc=\"https://github.com/user-attachments/assets/a76b1419-c1d9-4e46-a289-a819b7533b18\"\n/>\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 51\"\nsrc=\"https://github.com/user-attachments/assets/e48b19a3-85a8-4a13-b527-3a4494aef2af\"\n/>\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 57\"\nsrc=\"https://github.com/user-attachments/assets/50501672-4d75-43ce-b61b-646108b4b14a\"\n/>\n\n\n### Test:\n#### How to generate OTel data\n- Follow\nhttps://github.com/smith/elastic-stack-docker-compose?tab=readme-ov-file#elastic-stack-docker-compose\n\n#### How to test\n- Make sure your solution view is Observability\n- update your `kibana.yml` \n\n```\ndiscover.experimental.enabledProfiles:\n - observability-root-profile-with-attributes-tab\n # if you want to test it with the additional profiles add the following to your `kibana.yaml` \n - observability-traces-data-source-profile\n - observability-traces-transaction-document-profile\n - observability-traces-span-document-profile\n```","sha":"75ba373fbdd6a2ee52da6a1e9604c6ff7036b704"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/224723","number":224723,"mergeCommit":{"message":"Remaining work attributes table (#224723)\n\nCloses https://github.com/elastic/kibana/issues/221928\n\n#### Add ES|QL logic\n\n\nhttps://github.com/user-attachments/assets/d29f939a-7b82-4873-92d4-8210c2202339\n\n#### Empty message for accordion\n\n- Empty message when there are no attributes fields at all\n- For now we kept the accordion closed when fields count is zero, with\nan empty message inside, waiting for UI/UX team to review this\nimplementation\n\n<img width=\"524\" alt=\"Screenshot 2025-06-24 at 12 27 18\"\nsrc=\"https://github.com/user-attachments/assets/4015ed6a-5977-486d-93e6-d8b5714af9fd\"\n/>\n\n#### Simplify attribute display names\n\n- the field name should not show the full field name. The tooltip will\nshow both, simplify and full name, this is part of the implementation\n`FieldName` component from platform\n\n<img width=\"624\" alt=\"Screenshot 2025-06-24 at 12 19 48\"\nsrc=\"https://github.com/user-attachments/assets/634b4ef0-0934-4721-9217-334286b6464a\"\n/>\n\n<img width=\"624\" alt=\"Screenshot 2025-06-24 at 12 20 07\"\nsrc=\"https://github.com/user-attachments/assets/bdc6de9c-784f-4c78-bf18-1f37b645429d\"\n/>\n\n#### Filtering controls use full field name\n\n\nhttps://github.com/user-attachments/assets/7858d803-271e-4913-9aae-385dd7bc9e25\n\n#### Add explanatory tooltip for attribute namespaces\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 33\"\nsrc=\"https://github.com/user-attachments/assets/a76b1419-c1d9-4e46-a289-a819b7533b18\"\n/>\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 51\"\nsrc=\"https://github.com/user-attachments/assets/e48b19a3-85a8-4a13-b527-3a4494aef2af\"\n/>\n\n<img width=\"525\" alt=\"Screenshot 2025-06-24 at 12 24 57\"\nsrc=\"https://github.com/user-attachments/assets/50501672-4d75-43ce-b61b-646108b4b14a\"\n/>\n\n\n### Test:\n#### How to generate OTel data\n- Follow\nhttps://github.com/smith/elastic-stack-docker-compose?tab=readme-ov-file#elastic-stack-docker-compose\n\n#### How to test\n- Make sure your solution view is Observability\n- update your `kibana.yml` \n\n```\ndiscover.experimental.enabledProfiles:\n - observability-root-profile-with-attributes-tab\n # if you want to test it with the additional profiles add the following to your `kibana.yaml` \n - observability-traces-data-source-profile\n - observability-traces-transaction-document-profile\n - observability-traces-span-document-profile\n```","sha":"75ba373fbdd6a2ee52da6a1e9604c6ff7036b704"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Miriam <31922082+MiriamAparicio@users.noreply.github.com>








Closes #221928
Add ES|QL logic
Screen.Recording.2025-06-24.at.12.15.17.mov
Empty message for accordion
Simplify attribute display names
FieldNamecomponent from platformFiltering controls use full field name
Screen.Recording.2025-06-24.at.12.23.23.mov
Add explanatory tooltip for attribute namespaces
Test:
How to generate OTel data
How to test
kibana.yml