Add Uptime Monitoring by justinkambic · Pull Request #25480 · elastic/kibana

justinkambic · 2018-11-09T18:47:06Z

Summary

Add a new plugin to x-pack for Uptime Monitoring.

At the moment there are common, public, and server directories. The majority of changes here are in server. There's also a scripts directory that is just set up to derive static types from the GraphQL schema.

Common

There are constants and domain_types that will be shared by both public and server. These are straightforward. The third subfolder, graphql, contains generated files that will provide typings and introspection for the GraphQL client once it's been added.

Public

At the moment the client does not do much. In the lib folder you'll find somewhat typical Kibana boilerplate to hook into Angular and render a React component in the root of the application section of the page. The apps dir contains the actual startup code.

Server

The majority of the new code lives here. There is a new REST API, a GraphQL Endpoint, and a lib folder that contains the business logic.

lib

This is the code responsible for facilitating connection to/from elasticsearch, graphql, the REST API. This is also where the backend framework is mocked out for testing.

The adapter section is responsible for connections, i.e. querying ES, hooking the Kibana server to the REST paths, or establishing the GraphQL endpoint.

The compose directory contains the startup code where the various framework adapters are invoked.

The domain folder contains code designed for the creation of libs, which expose adapter functionality to consumers. libs enable mocking of the backend, as they can take any adapter matching their expected interface.

graphql

Uses the apollo server. Declares a schema, the required resolver (only 1 at the moment), and hooks into Kibana via a hapi plugin that is declared in lib.

rest_api

This is pretty straightforward; each part of the API is split into domains, at the moment we only have pings, with one route, which gets all of the pings. These are hooked into hapi by the Kibana adapter.

Recommendations for reviewing

Test the API/GraphQL endpoints.

For REST, simply use Postman or a similar product to query api/uptime_monitoring/pings.
For GraphQL, use GraphiQL to query api/uptime_monitoring/graphql. An example query:

{
  pings: allPings(sort: "desc", size: 2) {
    timestamp,
    beat {
      name,
      timezone
    },
    http {
    	response {
        status_code
      }
    },
    monitor {
      id,
      ip,
      name,
      scheme,
      status,
      type
    }
  }
}

Ensure that the React component loads properly by using the new sidebar button for Uptime Monitoring
Review the code. Comments are great but if anything is unclear ask to pair and we will discuss.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

there is no code that will render defined in this PR, aside from the Angular hookup code which is based on existing plugins that are cross-browser tested already
~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~

there is no text that will be user-facing defined in this PR
~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~

The project should have a README added, tracked in #25479
~~- [ ] Documentation was added for features that require explanation or tutorials~~

Unit or functional tests were updated or added to match the most common scenarios

irrelevant, no UI functionality in this PR
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2018-11-09T19:05:57Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mattapperson

Overall I would say this Is looking good, just the one major thing with regards to memory leaking but otherwise I like it

mattapperson · 2018-11-09T20:32:09Z

+    this.register(component);
+  };
+
+  private register = (rootComponent: React.ReactElement<any>) => {


The mounting you do here is good, but it does not take into consideration the needed unmounting. Otherwise angular will keep this in memory and navigating away and back will result in multiple allocations. To avoid this you will need something like the code we have in BeatsCM:

private manageAngularLifecycle($scope: any, $route: any, elem: any) { const lastRoute = $route.current; const deregister = $scope.$on('$locationChangeSuccess', () => { const currentRoute = $route.current; // if templates are the same we are on the same route if (lastRoute.$$route.template === currentRoute.$$route.template) { // this prevents angular from destroying scope $route.current = lastRoute; } }); $scope.$on('$destroy', () => { if (deregister) { deregister(); } // manually unmount component when scope is destroyed if (elem) { ReactDOM.unmountComponentAtNode(elem); } }); }

Does 42a087e look like a step in the right direction?

@mattapperson any further thoughts on this issue, or is it resolved now?

elasticmachine · 2018-11-10T00:33:48Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-11-10T00:58:07Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-11-12T13:54:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

chrisronline

Looks pretty good. I haven't really seen a typescript/graphql plugin code before (I know we have them, but I haven't looked). It does seem like a lot of boilerplate, but maybe I'm misunderstanding. How much of this would you say is pure copy/paste boilerplate?

justinkambic · 2018-11-14T15:08:11Z

@chrisronline fair question.

The angular lifecycle code that you've pointed out already is complete boilerplate copied from a few other plugins.
The contents of scripts is copied from infra, but needs here may diverge in the future; in my experience it's not uncommon for plugins to maintain their own scripts but I'm open to alternatives.
Some additional things on the client like feature registration.
The definition of a hapi plugin for the GraphQL endpoint is mostly copied from infra's implementation with little variation, and I wouldn't expect the needs of this code to change much. I'd say it's probably the biggest example of boilerplate on the server.
A lot of the setup code for the plugin server (init functions, REST route definition, GraphQL resolvers) is based on the same pattern followed by infra and beats-cm, but I wouldn't call it copy/paste. It follows the same pattern but all three have different domains so the behavior is largely unique, and changes I'm already working on upstream of this have begun diverging a bit from what's here.
There may be little things here and there outside of what I've mentioned but that is a large portion of the duplicate code.

* Initial commit. * Add heartbeat app to Kibana. * Add routing architecture. * Add graphQL endpoint. * Rename app to be 'uptime monitoring' instead of 'heartbeat'. * Add server tests. * Standardize interface/class/route naming. * Clean up GraphQL type inference script. * Update gql schema. * Rename classes/interfaces. Add tests for ES ping adapter. * Rename more symbols. * Improve angular lifecycle management. * Move angular lifecycle management code to framework adapter.