[Perfomance] Add Inline documentation for TTFMP #212393

kpatticha · 2025-02-25T15:13:31Z

Summary

closes https://github.com/elastic/observability-dev/issues/4101

⚠️ Instrumentation

Pass the description as metadata. The prefix [TTFMP] is required.

How to test

Checkout the PR
make sure you run yarn kbn bootstrap
go to any page that has onPageReady function instrumented (ex services)

elasticmachine · 2025-02-25T15:14:13Z

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

yngrdyn · 2025-02-25T15:36:49Z

This is a nice addition 👏🏼
What we also do for dataset quality is to explicitly mark visually what is the component reporting TTFMP

but of course, this is possible because we have a dedicated dashboard, might be messy to have something like this in an overview dashboard for all obs apps

bryce-b · 2025-02-25T18:29:06Z

src/core/packages/analytics/browser-internal/src/track_performance_measure_entries.ts

+          if (description?.length > MAX_DESCRIPTION_LENGTH) {
+            // eslint-disable-next-line no-console
+            console.error(
+              `The description for the measure: ${target} is too long. The maximum length is ${MAX_DESCRIPTION_LENGTH}. Strings longer than ${MAX_DESCRIPTION_LENGTH} will not be indexed or stored`


Is there a discrepancy here when considering the added length of the prefix [TTFMP] resulting in the actual acceptable length of text entered being 248? Is that worth being denoted in the error message?

Since the prefix is mandatory, the total allowed length remains 256 characters.

The console error will only be visible to developers, but I want to include a warning because any input exceeding 256 characters will be ignored by the indexer due to ignore_above 256 , as long keywords tend to cause issues in ES.

console.warn probably fit better

Do we only want to warn, or do we also want to truncate the description to the max size?
Maybe in some cases, the warning is missed, and we send a very long description.

The string will no be indexed above 256 but it's a good point. We could truncate and avoid sending the long text which won't be indexed

updated here 1cb6e66

src/core/packages/analytics/browser-internal/src/track_performance_measure_entries.ts

...m/packages/shared/kbn-ebt-tools/src/performance_metrics/context/measure_interaction/index.ts

cauemarcondes

LGTM, just some open questions.

kpatticha · 2025-02-27T15:31:09Z

@elastic/obs-ui-devex-team

…01-inline-doc

awahab07

LGTM!

Tested and it works as expected. Thanks for adding description in meta.

awahab07 · 2025-02-28T14:28:12Z

src/platform/packages/shared/kbn-ebt-tools/src/performance_metrics/context/types.ts

+type ApmPageId = 'services' | 'traces' | 'dependencies';
+type InfraPageId = 'hosts';
+
+export type Key = `${ApmPageId}` | `${InfraPageId}`;


Will all plugins have to define their page keys explicitly here? Is this to discourage changing prefixes?

Yes, the idea is also to encourage consistency, discoverability and avoid duplicate keys.

src/platform/packages/shared/kbn-ebt-tools/src/performance_metrics/context/types.ts

maryam-saeidi

Code LGTM!

Added a comment about truncating the description if it is long.

maryam-saeidi · 2025-03-03T08:57:55Z

src/core/packages/analytics/browser-internal/src/track_performance_measure_entries.ts

+          if (description?.length > MAX_DESCRIPTION_LENGTH) {
+            // eslint-disable-next-line no-console
+            console.error(
+              `The description for the measure: ${target} is too long. The maximum length is ${MAX_DESCRIPTION_LENGTH}. Strings longer than ${MAX_DESCRIPTION_LENGTH} will not be indexed or stored`


Do we only want to warn, or do we also want to truncate the description to the max size?
Maybe in some cases, the warning is missed, and we send a very long description.

…01-inline-doc

elasticmachine · 2025-03-04T14:34:07Z

💚 Build Succeeded

Buildkite Build
Commit: 20f5ce6

Metrics [docs]

Async chunks

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

id	before	after	diff
`apm`	2.3MB	2.3MB	+96.0B

Page load bundle

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

id	before	after	diff
`core`	474.7KB	474.9KB	+247.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`@kbn/core-analytics-browser-internal`	8	9	+1

Total ESLint disabled count

id	before	after	diff
`@kbn/core-analytics-browser-internal`	8	9	+1

History

💔 Build #280656 failed ce230e3
💔 Build #280012 failed 545e0a3
💚 Build #279746 succeeded af4b0ec
💚 Build #279396 succeeded dad69d8
💔 Build #279152 failed 2467bb5

kpatticha added 3 commits February 25, 2025 16:55

Pass description metadata

23edd98

Add description for top dependencies

2875852

Add test

bd9f6a7

kpatticha requested review from a team as code owners February 25, 2025 15:13

botelastic bot added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Feb 25, 2025

kpatticha added v9.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Feb 25, 2025

Fix lint

2467bb5

kpatticha added the backport:skip This commit does not require backporting label Feb 25, 2025

bryce-b reviewed Feb 25, 2025

View reviewed changes

cauemarcondes reviewed Feb 25, 2025

View reviewed changes

src/core/packages/analytics/browser-internal/src/track_performance_measure_entries.ts Outdated Show resolved Hide resolved

cauemarcondes reviewed Feb 25, 2025

View reviewed changes

...m/packages/shared/kbn-ebt-tools/src/performance_metrics/context/measure_interaction/index.ts Outdated Show resolved Hide resolved

cauemarcondes approved these changes Feb 25, 2025

View reviewed changes

kpatticha added 4 commits February 26, 2025 12:05

Merge branch 'main' of github.com:elastic/kibana into 4101-inline-doc

9f03a5f

Address PR comments

1ee626c

Replace error with warn

3c0c353

Merge branch 'main' into 4101-inline-doc

dad69d8

kpatticha enabled auto-merge (squash) February 26, 2025 11:30

kpatticha disabled auto-merge February 27, 2025 11:59

Add type safety for the key

6859b06

Merge branch '4101-inline-doc' of github.com:kpatticha/kibana into 41…

af4b0ec

…01-inline-doc

kpatticha requested review from awahab07 and maryam-saeidi February 27, 2025 15:34

awahab07 approved these changes Feb 28, 2025

View reviewed changes

kpatticha added 2 commits February 28, 2025 17:19

Update description type

feb5233

Merge branch 'main' of github.com:elastic/kibana into 4101-inline-doc

545e0a3

kpatticha enabled auto-merge (squash) February 28, 2025 15:29

maryam-saeidi approved these changes Mar 3, 2025

View reviewed changes

kpatticha added 2 commits March 4, 2025 12:12

Truncate description

1cb6e66

Merge branch 'main' into 4101-inline-doc

ce230e3

jloleysens approved these changes Mar 4, 2025

View reviewed changes

kpatticha added 2 commits March 4, 2025 14:37

fix eslint

3401cdb

Merge branch '4101-inline-doc' of github.com:kpatticha/kibana into 41…

20f5ce6

…01-inline-doc

kpatticha merged commit a16dc71 into elastic:main Mar 4, 2025
9 checks passed

kibanamachine added the v9.1.0 label Mar 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Perfomance] Add Inline documentation for TTFMP #212393

[Perfomance] Add Inline documentation for TTFMP #212393

kpatticha commented Feb 25, 2025

elasticmachine commented Feb 25, 2025

yngrdyn commented Feb 25, 2025

bryce-b Feb 25, 2025 •

edited

Loading

kpatticha Feb 26, 2025 •

edited

Loading

kpatticha Feb 26, 2025

maryam-saeidi Mar 3, 2025

kpatticha Mar 4, 2025

kpatticha Mar 4, 2025

cauemarcondes left a comment

kpatticha commented Feb 27, 2025

awahab07 left a comment

awahab07 Feb 28, 2025

kpatticha Feb 28, 2025

maryam-saeidi left a comment

maryam-saeidi Mar 3, 2025

elasticmachine commented Mar 4, 2025

ESLint disabled line counts

Total ESLint disabled count

[Perfomance] Add Inline documentation for TTFMP #212393

[Perfomance] Add Inline documentation for TTFMP #212393

Conversation

kpatticha commented Feb 25, 2025

Summary

⚠️ Instrumentation

How to test

elasticmachine commented Feb 25, 2025

yngrdyn commented Feb 25, 2025

bryce-b Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

kpatticha Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

kpatticha Feb 26, 2025

Choose a reason for hiding this comment

maryam-saeidi Mar 3, 2025

Choose a reason for hiding this comment

kpatticha Mar 4, 2025

Choose a reason for hiding this comment

kpatticha Mar 4, 2025

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

kpatticha commented Feb 27, 2025

awahab07 left a comment

Choose a reason for hiding this comment

awahab07 Feb 28, 2025

Choose a reason for hiding this comment

kpatticha Feb 28, 2025

Choose a reason for hiding this comment

maryam-saeidi left a comment

Choose a reason for hiding this comment

maryam-saeidi Mar 3, 2025

Choose a reason for hiding this comment

elasticmachine commented Mar 4, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

bryce-b Feb 25, 2025 •

edited

Loading

kpatticha Feb 26, 2025 •

edited

Loading