[Logs and Metrics UI] Initial setup for registering observability overview data fetchers #69999

jasonrhodes · 2020-06-25T19:34:39Z

This implements the basic skeleton for Logs and Metrics to register the Observability Homepage data retrieval functions.

x-pack/plugins/infra/public/plugin.ts

x-pack/plugins/infra/public/utils/logs_overview_fetchers.ts

jasonrhodes · 2020-06-29T14:53:03Z

src/plugins/usage_collection/public/index.ts

This is here because I missed exporting it when I updated the UsageCollection plugin start contract here: #69836

jasonrhodes · 2020-06-29T15:11:37Z

x-pack/plugins/infra/public/apps/common_providers.tsx

I changed some names here to try to make it all consistent. We now have most of our types prefaced with "InfraClient" (eventually to be LogsClient and MetricsClient, with "Client" to distinguish from the "Server" types), and then we use StartDeps/StartExports and SetupDeps/SetupExports to specify the dependencies and exports coming and going out of each lifecycle method.

jasonrhodes · 2020-06-29T15:12:18Z

x-pack/plugins/infra/public/index.ts

See this comment: https://github.com/elastic/kibana/pull/69999/files#r447046408

Fixes elastic#58014

shahzad31 · 2020-06-29T18:46:20Z

x-pack/plugins/infra/public/utils/logs_overview_fetchers.ts

+    // perform query
+    return {
+      title: 'Log rate',
+      appLink: 'TBD', // TODO: what format should this be in, relative I assume?


i think for the sake of simplicity observability plugin should handle the relative part while generating link.

simianhacker · 2020-06-29T21:04:54Z

When I try to start this PR I get these errors:

 ERROR in ./public/typings/index.ts
       │          Module not found: Error: Can't resolve './fetch_data_response' in '/Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings'
       │          resolve './fetch_data_response' in '/Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings'
       │            using description file: /Users/ccowan/Projects/kibana/x-pack/package.json (relative path: ./plugins/observability/public/typings)
       │              Field 'browser' doesn't contain a valid alias configuration
       │              using description file: /Users/ccowan/Projects/kibana/x-pack/package.json (relative path: ./plugins/observability/public/typings/fetch_data_response)
       │                no extension
       │                  Field 'browser' doesn't contain a valid alias configuration
       │                  /Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings/fetch_data_response is not a file
       │                .js
       │                  Field 'browser' doesn't contain a valid alias configuration
       │                  /Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings/fetch_data_response.js doesn't exist
       │                .ts
       │                  Field 'browser' doesn't contain a valid alias configuration
       │                  /Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings/fetch_data_response.ts doesn't exist
       │                .tsx
       │                  Field 'browser' doesn't contain a valid alias configuration
       │                  /Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings/fetch_data_response.tsx doesn't exist
       │                json
       │                  Field 'browser' doesn't contain a valid alias configuration
       │                  /Users/ccowan/Projects/kibana/x-pack/plugins/observability/public/typings/fetch_data_responsejson doesn't exist
       │                as directory
...

It looks like if we changed x-pack/plugins/observability/public/typings/fetch_data_response/index.d.ts to x-pack/plugins/observability/public/typings/fetch_data_response/index.ts it works fine.

jasonrhodes · 2020-06-29T21:38:02Z

@cauemarcondes can you take a look at the change I made in the observability plugin to properly export the new types? @simianhacker has a comment above about improving what I did so it works, not sure why we needed a d.ts file in the first place though so can you weigh in? Thanks!

afgomez · 2020-06-30T08:24:24Z

x-pack/plugins/infra/public/utils/logs_overview_fetchers.ts

+import { InfraClientCoreSetup } from '../types';
+import { LogsFetchDataResponse } from '../../../observability/public';
+
+export function getLogsHasDataFetcher(getStartServices: InfraClientCoreSetup['getStartServices']) {


@cauemarcondes I understand this function is a requirement for the dashboard. Is it a hard requirement?

I can imagine that the reason to have it is to not perform a potentially expensive query to fetch the data if there's no data to fetch.

if that's the case, how expensive would it be to call the data fetching function when there's no data at all, versus calling two functions every time?

@afgomez, the idea behind the hasData function is indeed to get a flag indicating if there's any data available in the logs indices. This exists because we have two pages in the Observability UI /landing and /overview plus /. The / URL, will call the hasData from all plugins, if one of the results is true we redirect the user to /overview otherwise /landing.

We could, of course, call the fetchData instead of hasData, but in that case, the response time would probably be twice or even thrice time slower then only checking if there's data available.

Also hasData must return false, if a user doesn't have privileges to read from the logs indices.

afgomez

PR is good 👍.

@cauemarcondes I left a comment regarding something I assume is a requirement from the observability plugin side, but it's not blocking to merge this.

cauemarcondes · 2020-06-30T08:37:04Z

x-pack/plugins/observability/public/typings/index.ts


 export * from './eui_draggable';
 export * from './eui_styled_components';
+export * from './fetch_data_response';


@jasonrhodes That looks good to me, I'm just curious to understand why the current way doesn't work for you. I am importing it on APM, and it works. https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/public/services/rest/observability_dashboard.ts#L11

Interesting, @cauemarcondes -- I'm surprised you're not getting a linting error there. Just 2 lines above that you have the "// eslint-disable-next-line @kbn/eslint/no-restricted-paths" comment which is exactly about this same thing. My understanding is you can only import from plugin/public or plugin/server when importing from another plugin.

Interesting, @cauemarcondes -- I'm surprised you're not getting a linting error there. 2 lines above you have "// eslint-disable-next-line @kbn/eslint/no-restricted-paths" which is about this very thing. My understanding is you can only import from plugin/public or plugin/server, when importing from other plugins.

For the record, I had to rename fetch_data_response/index.d.ts to index.ts (note the lack of .d). Otherwise webpack complained when trying to create the bundle

afharo

Changes for Kibana-Telemetry LGTM (only exporting UsageCollectionStart in src/plugins/usage_collection/public/index.ts)

Thanks @jasonrhodes

…odes/kibana into logs-data-for-obs-homepage_68531

elasticmachine · 2020-06-30T16:02:54Z

Pinging @elastic/apm-ui (Team:apm)

cauemarcondes

LGTM

skh

👍 from ingest management

kibanamachine · 2020-06-30T17:56:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d14e518

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
observability	21	+1	20

History

💔 Build #57497 failed 3cac145
💔 Build #57420 failed 29cf062
💔 Build #57276 failed ef9bcc6
💔 Build #57193 failed a6e24afbd3aaecd4c67292a7f85c37321cfe4140
💔 Build #56463 failed 9c64ad85c9d280c3825ca9288b03b9cf224fdbf2

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

…rview data fetchers (elastic#69999) * Switches mount callbacks to only use start deps Fixes elastic#58014 * Sets up skeleton logs data fetchers for overview * Fixes type hacks for logs fetcher * Prevent kibana from crashing on initial load * Fixes types and linting errors * Fixes some linting import/export issues Co-authored-by: Alejandro Fernández Gómez <alejandro.fernandez@elastic.co>

…rview data fetchers (#69999) (#70351) * Switches mount callbacks to only use start deps Fixes #58014 * Sets up skeleton logs data fetchers for overview * Fixes type hacks for logs fetcher * Prevent kibana from crashing on initial load * Fixes types and linting errors * Fixes some linting import/export issues Co-authored-by: Alejandro Fernández Gómez <alejandro.fernandez@elastic.co> Co-authored-by: Alejandro Fernández Gómez <alejandro.fernandez@elastic.co>

…rview data fetchers (elastic#69999) * Switches mount callbacks to only use start deps Fixes elastic#58014 * Sets up skeleton logs data fetchers for overview * Fixes type hacks for logs fetcher * Prevent kibana from crashing on initial load * Fixes types and linting errors * Fixes some linting import/export issues Co-authored-by: Alejandro Fernández Gómez <alejandro.fernandez@elastic.co>

jasonrhodes force-pushed the logs-data-for-obs-homepage_68531 branch from 4c885aa to 9c64ad8 Compare June 25, 2020 19:51

jasonrhodes requested review from a team, afgomez, cauemarcondes, simianhacker and weltenwort June 25, 2020 19:58

cauemarcondes reviewed Jun 26, 2020

View reviewed changes

x-pack/plugins/infra/public/plugin.ts Outdated Show resolved Hide resolved

cauemarcondes reviewed Jun 26, 2020

View reviewed changes

x-pack/plugins/infra/public/utils/logs_overview_fetchers.ts Outdated Show resolved Hide resolved

cauemarcondes reviewed Jun 26, 2020

View reviewed changes

x-pack/plugins/infra/public/utils/logs_overview_fetchers.ts Outdated Show resolved Hide resolved

jasonrhodes commented Jun 29, 2020

View reviewed changes

jasonrhodes added 2 commits June 29, 2020 11:26

Switches mount callbacks to only use start deps

a59fbef

Fixes elastic#58014

Sets up skeleton logs data fetchers for overview

98e205f

jasonrhodes force-pushed the logs-data-for-obs-homepage_68531 branch from 9c64ad8 to a6e24af Compare June 29, 2020 15:26

jasonrhodes marked this pull request as ready for review June 29, 2020 16:54

jasonrhodes requested a review from a team as a code owner June 29, 2020 16:54

jasonrhodes requested a review from a team June 29, 2020 16:54

jasonrhodes requested review from a team as code owners June 29, 2020 16:54

Fixes type hacks for logs fetcher

ef9bcc6

jasonrhodes force-pushed the logs-data-for-obs-homepage_68531 branch from a6e24af to ef9bcc6 Compare June 29, 2020 18:38

shahzad31 reviewed Jun 29, 2020

View reviewed changes

jasonrhodes added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jun 29, 2020

Prevent kibana from crashing on initial load

29cf062

afgomez reviewed Jun 30, 2020

View reviewed changes

afgomez approved these changes Jun 30, 2020

View reviewed changes

cauemarcondes reviewed Jun 30, 2020

View reviewed changes

afharo approved these changes Jun 30, 2020

View reviewed changes

jasonrhodes added 2 commits June 30, 2020 09:47

Fixes types and linting errors

8abdf6c

Merge branch 'logs-data-for-obs-homepage_68531' of github.com:jasonrh…

3cac145

…odes/kibana into logs-data-for-obs-homepage_68531

jasonrhodes removed the request for review from weltenwort June 30, 2020 13:52

Fixes some linting import/export issues

d14e518

botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Jun 30, 2020

cauemarcondes approved these changes Jun 30, 2020

View reviewed changes

skh approved these changes Jun 30, 2020

View reviewed changes

jasonrhodes merged commit bb7bc78 into elastic:master Jun 30, 2020

jasonrhodes deleted the logs-data-for-obs-homepage_68531 branch June 30, 2020 18:21

jasonrhodes mentioned this pull request Jun 30, 2020

[7.x] [Logs and Metrics UI] Initial setup for registering observability overview data fetchers (#69999) #70351

Merged

shahzad31 mentioned this pull request Jul 1, 2020

[Uptime] Fix type issue cause by conflicting merged PR's #70424

Merged

[Logs and Metrics UI] Initial setup for registering observability overview data fetchers #69999

[Logs and Metrics UI] Initial setup for registering observability overview data fetchers #69999

Uh oh!

Conversation

jasonrhodes commented Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simianhacker commented Jun 29, 2020

Uh oh!

jasonrhodes commented Jun 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afgomez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afgomez Jun 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afharo left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jun 30, 2020

Uh oh!

cauemarcondes left a comment

Choose a reason for hiding this comment

Uh oh!

skh left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 30, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

jasonrhodes commented Jun 25, 2020 •

edited

Loading

afgomez Jun 30, 2020 •

edited

Loading