[Discover][APM] Trace badge icons for traces summary cell #209824

Bluefinger · 2025-02-05T16:28:22Z

Summary

Adds badges for the Summary cell in the Traces Data Source Profile mode for Discover. Shows service.name, event.outcome, transaction.name, transaction.duration.us, span.name and span.duration.us depending on whether the fields are available on the log document.

Closes #208692

Trace Badges - Screenshot 2025-02-07 123457

How to Test

Run synthtrace with the following profile/scenario: traces_logs_entitities.ts
Ensure there is a Data View enabled for Logs Explorer that has the following id structure: apm_static_data_view_id with index patterns that match on traces-*, and add the following to your kibana.dev.yml and:

discover.experimental.enabledProfiles:
  - traces-data-source-profile

Go to Logs Explorer (Discover) and there should be badges that show for trace data, corresponding to service.name, event.outcome, transaction.name and transaction.duration.us or span.name and span.duration.us
Each badge should be clickable and show a popover for applying a filter

Bluefinger · 2025-02-06T12:05:38Z

/ci

Bluefinger · 2025-02-06T13:27:48Z

/ci

Bluefinger · 2025-02-06T17:34:17Z

/ci

elasticmachine · 2025-02-07T11:48:31Z

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

...d/kbn-discover-contextual-components/src/data_types/logs/components/summary_column/utils.tsx

...ages/shared/kbn-discover-utils/src/data_types/logs/utils/get_available_trace_badge_fields.ts

iblancof

Tested locally and really liked the quick filtering with highlighted attributes!

I noticed that badges are being added to all documents in the table, not just those with data_stream.type:"traces". Is this intentional and aligned with the original goal?

I get that impacting other documents comes at no extra cost, but I just want to ensure we're all aware and that it still makes sense in the context of improving the traces experience, especially since we won't rely on the APM Data View as an enabler forever.

Metrics	Logs

...es/shared/kbn-discover-contextual-components/src/data_types/logs/components/translations.tsx

src/platform/packages/shared/kbn-discover-utils/src/data_types/logs/types.ts

iblancof · 2025-02-10T10:33:01Z

...blic/context_awareness/profile_providers/observability/traces_data_source_profile/profile.ts

  },
  resolve: ({ dataSource }) => {
    if (
      isDataSourceType(dataSource, DataSourceType.DataView) &&
-      dataSource.dataViewId === 'apm_static_data_view_id_default'
+      dataSource.dataViewId.includes('apm_static_data_view_id')


Thanks for adding support for multiple spaces!

src/platform/packages/shared/kbn-discover-utils/src/field_constants.ts

davismcphee

Left a bit of feedback, but the Data Discovery changes look good overall. I just noticed one issue when testing. It looks like filtering doesn't work for formatted fields -- are we able to pass the unformatted values instead?

filters.mp4

davismcphee · 2025-02-11T01:29:22Z

...cover-contextual-components/src/data_types/logs/components/summary_column/summary_column.tsx

@@ -38,7 +43,7 @@ export interface SummaryColumnFactoryDeps {
  share?: SharePluginStart;
 }

-export type SummaryColumnProps = DataGridCellValueElementProps;
+export type SummaryColumnProps = DataGridCellValueElementProps & { isTracesSummary?: boolean };


I partly wonder if it would be better to either decompose these components or make them more generic vs adding a flag, but I guess it depends on how many variants we expect to have, and Logs UX would know better.

...d/kbn-discover-contextual-components/src/data_types/logs/components/summary_column/utils.tsx

...context_awareness/profile_providers/observability/traces_data_source_profile/profile.test.ts

kpatticha · 2025-02-11T16:11:53Z

Similar to @davismcphee comment around the formatted filters I've noticed this one

Screen.Recording.2025-02-11.at.6.09.22.PM.mov

kpatticha · 2025-02-11T16:18:13Z

Another observation—I initially found this a bit confusing but later realized that the documents below represent aggregated metrics. This approach might work for the first iterations, but it's worth checkin.

We display badges only for raw transactions, while the APM data view can have the aggregated metrics.

Bluefinger · 2025-02-11T16:29:24Z

while testing it locally for the transaction.duration.us badge I've noticed this

@kpatticha Which synthtrace dataset are you testing with or with real data?

...d/kbn-discover-contextual-components/src/data_types/logs/components/summary_column/utils.tsx

kpatticha · 2025-02-12T08:40:36Z

while testing it locally for the transaction.duration.us badge I've noticed this

@kpatticha Which synthtrace dataset are you testing with or with real data?

I used synthrace. First run node scripts/synthtrace simple_trace and then node scripts/synthtrace traces_logs_entities.ts

iblancof · 2025-02-12T11:20:58Z

Hey, I was exploring a bit and noticed this, some durations don't seem to display their units.

This data comes from the traces_logs_entities.ts synthrace scenario.

Bluefinger · 2025-02-12T14:05:37Z

So, I refactored a bunch of the code, so it should be a bit more robust and also more reusable in additional contexts. However, I decided to not try to force the microsecond formatting, because there's a lot of machinery already for deciding how something gets formatted, and I don't want to second guess what it is doing. I brought back the translated labels for the Event Outcome badge, but it is only translated for the badge itself. On the popover, it shows the raw value, so to make it clear that the actual value that is being filtered on is what is represented by the raw value.

~~Also, I hope the generics aren't too hairy.~~ I erased the generics, wasn't needed.

Bluefinger · 2025-02-13T16:22:19Z

For today, I removed the translations for event.outcome as the designs no longer reflect this, and we'll be relying on the field formatting going forward.

elasticmachine · 2025-02-13T18:35:17Z

💚 Build Succeeded

Buildkite Build
Commit: a391f62

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1703	1706	+3
`canvas`	1227	1230	+3
`cloudSecurityPosture`	794	797	+3
`discover`	1101	1107	+6
`esqlDataGrid`	519	522	+3
`eventAnnotationListing`	746	749	+3
`lens`	1823	1826	+3
`logsShared`	363	366	+3
`observability`	1445	1448	+3
`searchPlayground`	296	299	+3
`securitySolution`	6791	6794	+3
`slo`	994	997	+3
`unifiedDocViewer`	281	284	+3
`unifiedHistogram`	288	291	+3
total			+45

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-utils`	230	240	+10

Async chunks

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

id	before	after	diff
`discover`	847.9KB	851.8KB	+3.9KB
`unifiedDocViewer`	148.5KB	149.0KB	+527.0B
total			+4.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/discover-utils`	4	3	-1

Page load bundle

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

id	before	after	diff
`discover`	44.8KB	44.8KB	+1.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/discover-contextual-components`	39	38	-1
`@kbn/discover-utils`	280	290	+10
total			+9

History

💔 Build #275705 failed f6cee4d
💔 Build #275629 failed e9a86d7
💔 Build #275594 failed 3a152ba
💛 Build #275253 was flaky 181362d
💚 Build #274859 succeeded a25bde7

davismcphee

Filtering is now working as expected, and the Data Discovery changes LGTM 👍 I left a couple more comments around field formatting, but I think this is good to approve on our end now. Thanks for working on it!

davismcphee · 2025-02-14T05:38:55Z

...d/kbn-discover-contextual-components/src/data_types/logs/components/summary_column/utils.tsx


  return availableResourceFields.map((name) => ({
    name,
-    value: resourceDoc[name],
+    rawValue: resourceDoc[name],
+    value: getFormattedValue(name, resourceDoc[name], dataView),


Suggested change

value: getFormattedValue(name, resourceDoc[name], dataView),

value: formatFieldValue(

resourceDoc[name],

row.raw,

fieldFormats,

dataView,

dataView.getFieldByName(name),

'html'

),

Since there's no longer any special handling going on here, I'd definitely recommend dropping the getFormattedValue function and using formatFieldValue since it's the standard util and will produce the best output.

I've made the change to make use of formatFieldValue, but as a note, there's a soundness hole with it in that in some circumstances, it doesn't actually return a string value. This is due to FieldFormat.convert not actually turning a value into string if there is no formatter present for it, instead it simply casts it to string. As such, I put TODOs on the places I've changed to using formatFieldValue to remove the string guards once the method no longer has a soundness hole.

Thanks for updating it 👍 And interesting, TIL. I suppose that would be an issue regardless for any use of getFormatterForField, but I don't think it's an actual scenario. It looks like converter never returns undefined and we we just have a redundant null check there. We should be able to drop it and get rid of value as string. Our team owns that code so we can take a look at it as a followup and update here too.

src/platform/plugins/shared/discover/public/components/data_types/logs/service_name_cell.tsx

## Summary It was [pointed out](#209824 (comment)) during a recent review that the field format `convert()` value does some odd casting. It looks like this probably isn't necessary since `converter` should never be undefined, and we can just return the result directly. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Elastic Machine <[email protected]>

…#211342) ## Summary It was [pointed out](elastic#209824 (comment)) during a recent review that the field format `convert()` value does some odd casting. It looks like this probably isn't necessary since `converter` should never be undefined, and we can just return the result directly. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Elastic Machine <[email protected]>

Bluefinger added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Feb 5, 2025

Bluefinger marked this pull request as ready for review February 7, 2025 11:48

Bluefinger requested review from a team as code owners February 7, 2025 11:48

cauemarcondes reviewed Feb 7, 2025

View reviewed changes

iblancof reviewed Feb 10, 2025

View reviewed changes

crespocarlos reviewed Feb 10, 2025

View reviewed changes

src/platform/packages/shared/kbn-discover-utils/src/field_constants.ts Outdated Show resolved Hide resolved

src/platform/packages/shared/kbn-discover-utils/src/field_constants.ts Outdated Show resolved Hide resolved

davismcphee reviewed Feb 11, 2025

View reviewed changes

Bluefinger and others added 14 commits February 11, 2025 11:09

[Discover][APM] Trace badge icons for traces summary cell

18fd34f

Include span fields for the badges

01a2530

[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'

9d091de

Make isTracesSummary optional

7c42be4

Tidy up constants and undo erroneous import changes

cafc4eb

Event outcome badge and translations

fcb923f

Add a test case for covering the traces summary mode

13d1f1c

Probs don't mess too much with mocks

d254d57

Refactoring field formatting and resolution

0639a9d

Seperate out trace types from logs

1db2c06

Don't forget to save changed files

d71ad02

Don't forget to rename function

95525f2

Better generic name for getUnformattedFields

b414262

Fix filtering for event.outcome badge

a195f39

Bluefinger force-pushed the discover-summary-badges branch from a25bde7 to a195f39 Compare February 11, 2025 11:13

Optimise get available field utils, fix nit on profile.test.ts

a89779d

Remove translated event.outcome badge, fix formatting cases

181362d

davismcphee reviewed Feb 12, 2025

View reviewed changes

Bluefinger added 2 commits February 12, 2025 14:58

Refactor and genericify resource field creation

0a040dc

Merge branch 'main' into discover-summary-badges

0236b0f

Bluefinger added 4 commits February 12, 2025 15:18

Erase the generics!

3a152ba

Fix typing for renderValue

e9a86d7

Fix ServiceNameCell

570b71d

Actually use traceLabel

f6cee4d

Bluefinger mentioned this pull request Feb 12, 2025

[Discover][APM] Custom overview tab for data_stream.type: "traces" in the new trace data source profile detail flyout #210612

Merged

Bluefinger added 3 commits February 13, 2025 11:38

Degenerify getUnformattedFields

9c27448

Merge branch 'main' into discover-summary-badges

c2bd614

Remove translations from event.outcome as it is no longer part of design

122a194

Bluefinger and others added 2 commits February 13, 2025 17:43

Make checking for trace data more robust

fcf297e

[CI] Auto-commit changed files from 'node scripts/notice'

a391f62

davismcphee approved these changes Feb 14, 2025

View reviewed changes

Make use of formatFieldValue instead of custom logic

b64f09b

Bluefinger merged commit a95f903 into elastic:main Feb 14, 2025
9 checks passed

kibanamachine added the v9.1.0 label Feb 14, 2025

This was referenced Feb 15, 2025

[Field Formatters] Fix field format convert() return value #211342

Merged

[Discover] Fix resource badge text rendering #211885

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover][APM] Trace badge icons for traces summary cell #209824

[Discover][APM] Trace badge icons for traces summary cell #209824

Bluefinger commented Feb 5, 2025 •

edited

Loading

Bluefinger commented Feb 6, 2025

Bluefinger commented Feb 6, 2025

Bluefinger commented Feb 6, 2025

elasticmachine commented Feb 7, 2025

iblancof left a comment

iblancof Feb 10, 2025

davismcphee left a comment

davismcphee Feb 11, 2025

kpatticha commented Feb 11, 2025

kpatticha commented Feb 11, 2025 •

edited

Loading

Bluefinger commented Feb 11, 2025

kpatticha commented Feb 12, 2025

iblancof commented Feb 12, 2025

Bluefinger commented Feb 12, 2025 •

edited

Loading

Bluefinger commented Feb 13, 2025

elasticmachine commented Feb 13, 2025

API count

davismcphee left a comment

davismcphee Feb 14, 2025

Bluefinger Feb 14, 2025

davismcphee Feb 15, 2025

-    value: getFormattedValue(name, resourceDoc[name], dataView),
+    value: formatFieldValue(
+      resourceDoc[name],
+      row.raw,
+      fieldFormats,
+      dataView,
+      dataView.getFieldByName(name),
+      'html'
+    ),

[Discover][APM] Trace badge icons for traces summary cell #209824

[Discover][APM] Trace badge icons for traces summary cell #209824

Conversation

Bluefinger commented Feb 5, 2025 • edited Loading

Summary

How to Test

Bluefinger commented Feb 6, 2025

Bluefinger commented Feb 6, 2025

Bluefinger commented Feb 6, 2025

elasticmachine commented Feb 7, 2025

iblancof left a comment

Choose a reason for hiding this comment

iblancof Feb 10, 2025

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

davismcphee Feb 11, 2025

Choose a reason for hiding this comment

kpatticha commented Feb 11, 2025

kpatticha commented Feb 11, 2025 • edited Loading

Bluefinger commented Feb 11, 2025

kpatticha commented Feb 12, 2025

iblancof commented Feb 12, 2025

Bluefinger commented Feb 12, 2025 • edited Loading

Bluefinger commented Feb 13, 2025

elasticmachine commented Feb 13, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

davismcphee left a comment

Choose a reason for hiding this comment

davismcphee Feb 14, 2025

Choose a reason for hiding this comment

Bluefinger Feb 14, 2025

Choose a reason for hiding this comment

davismcphee Feb 15, 2025

Choose a reason for hiding this comment

Bluefinger commented Feb 5, 2025 •

edited

Loading

kpatticha commented Feb 11, 2025 •

edited

Loading

Bluefinger commented Feb 12, 2025 •

edited

Loading