Skip to content

Tribe node support#9132

Merged
jbudz merged 47 commits intomasterfrom
tribe-node-support
Dec 30, 2016
Merged

Tribe node support#9132
jbudz merged 47 commits intomasterfrom
tribe-node-support

Conversation

@tylersmalley
Copy link
Member

@tylersmalley tylersmalley commented Nov 17, 2016

This PR adds support for configuring Kibana to pull data from a tribe node. At a high level, this works by pointing kibana to an administration node for managing state and a tribe node for data.

Changes

Server API

Previously we exposed the Elasticsearch connection on the Hapi plugin (client, ElasticsearchClientLogging, createClient, callWithRequestFactory, callWithRequest).

The Elasticsearch plugin now exposes getCluster and createCluster on server.plugins.elasticsearch. getCluster returns a cluster object containing config(), createClient, callWithRequest and callWithInternalUser.

There are two cluster's already created. 'admin' for any kibana state changes(modifying .kibana, etc), and 'data' for all other requests. If a tribe node is configured, 'admin' points to elasticsearch.url, and 'data' points to elasticsearch.tribe.url. If a tribe node is not configured, both point to elasticsearch.url

An example of making an admin request as the configured server user in kibana.yml:

const { callWithInternalUser } = server.plugins.elasticsearch.getCluster('admin')
callWithInternalUser('bulk', options)

And a non kibana state change passing through the current request:

const { callWithRequest } = server.plugins.elasticsearch.getCluster('data')
callWithRequest(req, 'bulk', options)

Browser API

Kibana state management requests in the browser will need to use the esAdmin angular service, or to use the /es_admin proxy endpoint. All other requests can continue to use the es service and /elasticsearch proxy endpoint.

Testing

Quick start:

  • ./node_modules/.bin/grunt esvm:tribe:keepalive will start several nodes
    • localhost:9200 tribe node, points to cluster 1 and 2
    • localhost:9201 data cluster 1
    • localhost:9202 data cluster 2
    • localhost:9203 administration
  • In kibana.yml:
    elasticsearch.tribe.url: "http://localhost:9203"
    

Closes #8495

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@jbudz jbudz self-requested a review December 9, 2016 19:56
@jbudz jbudz changed the title WIP: Tribe node support Tribe node support Dec 9, 2016
@jbudz jbudz added the review label Dec 9, 2016
@tylersmalley tylersmalley requested a review from spalger December 9, 2016 20:24
@epixa epixa self-assigned this Dec 9, 2016
@epixa epixa self-requested a review December 9, 2016 21:14
it('returns true with single a node that matches', async () => {
setNodes('5.1.0');
const result = await checkEsVersion(server, KIBANA_VERSION);
const result = await checkEsVersion(server, KIBANA_VERSION, callAsKibanaUser);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like checkEsVersion takes callAsKibanaUser as an arg

module.exports = _.memoize(function (server) {
const config = server.config();
const target = url.parse(config.get('elasticsearch.url'));
module.exports = _.memoize(function (config) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be memoized per server, but I don't think memoizing on the config makes sense. Since we are tracking the instances another way I don't think we need to memoize here.

* @param {boolean} options.wrap401Errors
* @returns {Promise}
*/
callAsKibanaUserFactory(client) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like making this a factory is an unnecessary layer of indirection. Wouldn't a method that accesses this._client do the trick?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. We are currently utilizing it in X-Pack but we should be able to go about creating the client with elasticsearch-js plugins another way.

const Logger = server.plugins.elasticsearch.ElasticsearchClientLogging;

class AdminClientLogging extends Logger {
get tags() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

es6 properties are available so this can just be

class AdminClientLogging extends Logger {
  tags = ['admin'];
  logQueries = Boolean(config.get('elasticsearch.logQueries'));
  // ...
}

server.expose('ElasticsearchClientLogging', clientLogger(server));

const dataCluster = createDataCluster(server);
server.on('close', bindKey(dataCluster, 'close'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that clients should probably automatically register their close handler on creation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to create{Data,Admin}Cluster

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still seems to be here, though?

let SourceAbstract = Private(AbstractDataSourceProvider);
let DocRequest = Private(DocRequestProvider);
export default function DocSourceFactory(Private) {
let DocSourceAbstract = Private(AbstractDocSource);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about let AbstractDocSource = Private(AbstractDocSourceProvider);

let docStrategy = Private(DocStrategyProvider);
let DocRequest = Private(DocRequestProvider);

_.class(AdminDocSource).inherits(DocSourceAbstract);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update any new classes to use es6 syntax

import chrome from 'ui/chrome';
import modules from 'ui/modules';

modules.get('kibana').run(function ($rootScope, $location) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel like the right way to do this, what other methods were ruled out? A route resolve feels like a more appropriate way to add the precondition, and it should probably be 1) paired with a notify.warning() and 2) kbnUrl.redirect()

Copy link
Contributor

@spalger spalger Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also prefer if the devtool app wasn't even in the navbar, but maybe there's a good reason it was left.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed in favor of disabling console and hiding the dev tools application when there aren't any tools registered.

module.exports = function (server) {
const config = server.config();
const client = server.plugins.elasticsearch.client;
const callAsKibanaUser = server.plugins.elasticsearch.getCluster('admin').callAsKibanaUser;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destructuring the callAsKibanaUser property here, and other places, could trim down the line length a bit. Something I would personally appreciate.

}

config(path) {
return cloneDeep(path ? get(this._config, path) : this._config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of providing an alternate config api here. What do you think of exposing the Config class from the kibana server so we could wrap this object in that and expose its api from cluster.config()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not against it but have some concerns, it will couple cluster to the config class and the server, and it's a relatively heavy class (we just need a getter). an alternative could be to rename the config method, or using defineProperty. thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the config class is too much for here, but I think we still need to do something about the underlying ambiguity.

My issue is mostly with this._config. I'm unclear what this value does/should contain, and what values the consumers of this cluster are expecting it to have. cluster.config() adds to that ambiguity by encouraging external code to depend on arbitrary, potentially deeply-nested, properties of this._config.

The config class tries to mitigate this ambiguity by strictly validating the config with Joi and then throwing errors anytime external code requests a key that isn't in the schema. This makes the class larger, but helps me (and I hope others) feel confident about what values are available and what they should look like.

What if we moved to more specific methods; rather than a config(key) function we exposed a series of functions for the discrete values external code is consuming: getRequestHeadersWhitelist(), getUrl() and so on.

tylersmalley and others added 4 commits December 11, 2016 18:50
* Close create{Admin,Data}Cluster handles closing the connection
* Remove callAsKibanaUser argument from tests
* ClientLogger uses ES6 properties for tags and logQueries
* Ensure were destructuring cluster to access callAsKibanaUser

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@epixa
Copy link
Contributor

epixa commented Dec 14, 2016

This branch has conflicts, FYI

tylersmalley and others added 2 commits December 14, 2016 08:45
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@epixa
Copy link
Contributor

epixa commented Dec 14, 2016

Still has conflicts even after previous merge

@tylersmalley
Copy link
Member Author

@epixa - we're conflict free.

tylersmalley and others added 3 commits December 14, 2016 14:39
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Class properties are still in the very eary stages and not widely supported.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
const plugins = [function (Client, config) {
// esFactory automatically injects the AngularConnector to the config
// https://github.com/elastic/elasticsearch-js/blob/master/src/lib/connectors/angular.js
_.class(CustomAngularConnector).inherits(config.connectionClass);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a native class here?


getSsl = () => getClonedProperty(this._config, 'ssl');

getClient = () => this._client;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't matter, and I'm not advocating for changing it, but I am curious: why not use getters here?

@stacey-gammon
Copy link

stacey-gammon commented Dec 28, 2016

I can trigger a shard failure by creating a scripted field where I have doc['field-from-index-on-cluster-1'].value + doc['field-from-index-on-cluster-2'].value but perhaps that is a known limitation and not supported atm?

Update: Looks like it's simpler than that. A scripted field on my index * with the value doc['system.memory.actual.free'].value triggers a failure, but the same scripted field on my index of metr* (which exists only on a single cluster) works fine. Strangely, if I try the same thing with doc['source.stats.net_packets_total'].value and for my pack* index, it works just fine on both. I'm not sure what is special about the first one. Maybe this is an unrelated bug - I haven't tried in a non-tribe node environment.

Copy link

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not too much of a pain in the A can you switch to named exports for the new code (re: #8641)

Copy link

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not too much of a pain in the A can you switch to named exports for the new code (re: #8641)?

client: {
nodes: {}
},
getCluster: sinon.stub().withArgs('admin').returns({ callWithInternalUser: sinon.stub() }), // Fix

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the // Fix comment for? You want to get rid of this stub? or is the comment old? If intentional, maybe add a little more information.

Copy link
Member Author

@tylersmalley tylersmalley Dec 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment appears to be leftover and is no longer valid. Removing.

@@ -0,0 +1,18 @@
import { get } from 'lodash';

export default function checkForTribe(callWithInternalUser) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the return values to be a little unexpected. I would have guessed true if tribe found, false otherwise, based on the name alone. Something like assertNoTribeSettings would be a better name, IMO. Or ensureNoTribe, throwErrorIfTribeFound.

Or, even better, since this is only used in one place aside from testing, maybe leave it as checkForTribe but return true or false from the promise, and change where it is being called now to throw an error if tribe was found.

Also maybe add a function comment and specify input and return values?

Copy link
Member Author

@tylersmalley tylersmalley Dec 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stacey-gammon, I pushed a commit which renames the check to ensureNotTribe and did the same to checkEsVersion (making it ensureEsVersion). While returning a boolean would be a better interface, it wouldn't be consistent with the ES check and would require more code in the health check which is the only place it's used. Will this work?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome, much clearer, thanks!


info() {}

debug() {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looked weird to me too. Maybe just add a comment so future readers of this code will understand these functions shouldn't be removed, and why. (Maybe bring the close() {} function up here too so they are all together).

I understand why we can't get rid of them, but I'm surprised they aren't implemented at all. Since they are being used, seems like that would mean we are losing messages. I wonder why we wouldn't have a simple server.log(['debug', 'elasticsearch'].concat(this.tags), message);.

I understand this isn't your original code though, so no need to go digging in, or fixing it here.

}

/**
* callWithRequest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this was copied over or you added, but fwiw, I find comments like this to be pointless. We know the name of the function below so what is the benefit of adding it in the comment? Just another place that can get out of sync if the function name was ever refactored. Unless there is a reason for it that I am unaware of?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added, but your right - it's rather pointless documentation and the method names and arguments should speak for themselves. I have remove the comment blocks.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever you decide is fine, but personally I love the argument specifications. :) I just don't like repeating the name of the function. I like it when input parameter types are explicitly defined because I find it difficult to discover since js is not strongly typed. Additionally you can get some visual indication (depending on your IDE) that you are doing something wrong if you pass in a type that isn't expected.

* @param {string} endpoint
* @param {Object} clientParams
* @param {Object} options
* @param {boolean} options.wrap401Errors

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be syntax I'm unaware of but this isn't an input parameter to the function, so shouldn't it be removed? If we wanted to specify that options should have a wrap401Errors field, I think we should typedef the options Object

Also, should clientParams and options also be | undefined because they are given default values as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's simply a way of documenting the options. This comment block has been removed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documenting the fields in options object or documenting the input parameters to the function? I get the input parameters (and I actually really like them, as per above), it's just the very last one that doesn't look right to me because it's not an input parameter, and I don't think thats the right way to document options. Instead I would expect something like this:

/**

/**

  • ... other stuff
  • @param {Object} clientParams
  • @param {RequestOptions} options
    */
    callWithRequest = ....

Anyway, fine whatever you decide, I'm just curious if this is an official JSDoc way of doing the above that I was unaware of.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tylersmalley - I just ran into this JSDOC - https://code.google.com/archive/p/jsdoc-toolkit/wikis/TagParam.wiki
See the Parameters With Properties section
Apparently

@param {Object} options
@param {boolean} options.wrap401Errors

is the legit way to say you expect the options Object to have that particular boolean parameter, without having to use typedef. Maybe you already knew that, but in case you didn't, wanted to share. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah I had used it previously but good to know it's actually part of the JSDOC spec. Thanks for the follow-up!

import { bindKey } from 'lodash';
import clientLogger from './client_logger';

export default function (server) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use named export? Would be helpful to giving the function a name so it's clearer what is going on without having to look at the file name.

Also I think comments would be helpful, e.g. is this what is used for creating test clusters for the esvm command stuff to work? Is it only used for testing situations?

server.expose('ElasticsearchClientLogging', clientLogger(server));

createDataCluster(server);
createAdminCluster(server);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the code that gets the testing stuff to work and set up the test data and admin clusters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't necessary for testing to work but it isolated the code the made it easier to test.

Both of these call server.plugins.elasticsearch.createCluster() which expose the data and admin clusters to the rest of the application. They are then accessible via server.plugins.elasticsearch.getCluster()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah okay makes sense, I initially thought they were actually creating the clusters (booting the instances up). Resolved.

config.ssl.ca = serverConfig.ssl.ca.map(readFile);
}

config.defer = () => Bluebird.defer();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the defer here? Not possible to use native promises?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, this is necessary because the elasticsearch-js client used to return bluebird promises and our codebase is using .finally() in a bunch of places as a result. However, the latest version of elasticsearc-js now returns native promises, so we need to provide a defer function to preserve the existing behaviors in Kibana.

The long term solution would be to move our codebase away from .finally(), but that's not really a tribe thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epixa is correct - this keeps the code functionally the same as it used to be. We could remove the Bluebird promise here but we would need to remove our usage of finally which is only in one place within health_check.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, maybe not something that should be part of this PR, but will be nice to eventually get consistency around our promise usage. Thanks for the explanation!

if (es) return es;

es = esFactory({
.service('es', function (esFactory, esUrl, esApiVersion, esRequestTimeout) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some comments explaining the difference between es and esAdmin. If I'm understanding this correctly, es should be used to fetch data, while esAdmin should be used to fetch kibana stuff (like saved object meta data)? So esAdmin will contain the .kibana index, and es will have the data stuff? Maybe it should be called esData to better differentiate.

It looks like everything got a bit more complicated now because we have to be aware of which is the right service to use. I suppose there is no way to abstract that out so it would pick the right service?

When tribe node is off, do es and esAdmin point to the same cluster? If so, it seems like it would be very easy for someone developing in non-tribe mode to break tribe mode without realizing it if they pick the wrong service, because either es or esAdmin would work in non-tribe node land. Or I could be completely wrong, if that isn't how it works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to comments

It's es to preserve backwards compatibility with the old implementation, unfortunately. It is more complicated now, but Kibana has been more complicated in this way when xpack is in the mix for awhile since monitoring also supports an entirely separate cluster. This change is just formalizing the concept. The two are the same under the default configuration.

We need sufficient integration testing with tribe node to help us avoid making cluster-choice based mistakes. That's in the follow up meta ticket to address after feature freeze: #9640. CI builds should run all possible selenium tests against a tribe configuration to help verify behaviors here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with making it smarter is plugins - with core kibana we know it's only the .kibana index, but with x-pack we have .monitoring-*, and more. Maybe plugins could register administration indices.

👍 comment being added, and I'll make sure it's in the elastic.co plugin docs too

export default function DocSourceFactory(Private) {
const AbstractDocSource = Private(AbstractDocSourceProvider);
const docStrategy = Private(DocStrategyProvider);
const DocRequest = Private(DocRequestProvider);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is docStrategy a function and DocRequest a class, hence the different capitalizations? Or should they be consistent?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. docStrategy is an object, DocRequest is a class.

jbudz and others added 2 commits December 28, 2016 15:21
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@jbudz
Copy link
Contributor

jbudz commented Dec 28, 2016

@stacey-gammon switched to named exports for most new files with the exception of courier files, for consistency with sibling implementations. Okay if we defer on that area until all of courier is addressed?

@jbudz
Copy link
Contributor

jbudz commented Dec 28, 2016

re: #9132 (comment) I would expect it to work if those fields are present on every document in both clusters. Do null checks help? something like

if (doc['source.stats.net_packets_total'].value != null) {
  return doc['source.stats.net_packets_total'].value;
}
return null;

jbudz and others added 3 commits December 28, 2016 16:31
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley
Copy link
Member Author

@stacey-gammon, it won't let me respond to your comment regarding client_logger.js so I will respond here. elasticsearch-js expects those functions to exist and this is how they were implemented prior to this PR. Your right, we do need to see if there is anything we are dropping due to the NOOP but this maintains functionality. In the meantime I have pushed a commit to organize those methods together and comment why.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
})

//Elasticsearch client used for managing Kibana's state. Connects to the /es-admin proxy,
//Always uses the base elasticsearch configuartion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, but configuartion => configuration.

@stacey-gammon
Copy link

@jbudz re: the scripted fields issue, I think this is happening on master as well, so it doesn't look related to this PR. FWIW, adding the null check didn't seem to work (I tried your suggestion as well as

if (doc['system.memory.actual.free'] != null) {
  return doc['system.memory.actual.free'].value;
}
return null;

since I think you meant to drop the .value on the first check, but neither version worked. I'll file a separate issue.

@epixa
Copy link
Contributor

epixa commented Dec 29, 2016

I didn't run the few updates, but the code changes LGTM

@epixa epixa dismissed spalger’s stale review December 29, 2016 16:19

He's out for a few days, so other people reviewed instead

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@jbudz jbudz merged commit 024c816 into master Dec 30, 2016
@tylersmalley tylersmalley deleted the tribe-node-support branch December 30, 2016 16:09
jbudz pushed a commit to jbudz/kibana that referenced this pull request Dec 30, 2016
* Adds support for Tribe nodes

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* @spalger review feedback

* Close create{Admin,Data}Cluster handles closing the connection
* Remove callAsKibanaUser argument from tests
* ClientLogger uses ES6 properties for tags and logQueries
* Ensure were destructuring cluster to access callAsKibanaUser

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Use class syntax on new data sources

* [tribe] Use includes instead of indexOf in call_client

* [tribe] DocRequest --> AbstractDocRequest

* [tribe] Fix AbstractDoc test rename

* Removes factory objects and adds addClientPlugin to Cluster (elastic#9467)

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Resolves eslint error

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Use properties on the instance instead of class properties

Class properties are still in the very eary stages and not widely supported.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Remove disabled dev tools app, do not bundle console when tribe is enabled

* [tribe] Use destructuring, don't reassign args

* [tribe] Use class syntax for client request wrapper

* [tribe] callAsKibanaUser -> callWithInternalUser

* [tribe] Remove clients from module context, service is a singleton

* [tribe] Use instance property shorthand for admin and data DocRequests

* Removes questions

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Fixes typo in tests

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Correctly names test case

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Revert "Use properties on the instance instead of class properties"

This reverts commit ebd06ae.

* Adds tests for create_{admin,data}_cluster

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Persists clusters to server

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Move cluster config requests to distinct getters

* Adds getClient and removes addClientPlugin

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Expose createClient, consolidate config parsing

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Removes createClients from Cluster

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Prevent status change from red to red

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Updates esvm:tribe ports to be consistant with dev

9200 is admin
9201:9202 are both data clusters
9203 is a tribe node connecting to both data clusters

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Get ssl.ca from serverConfig

* [tribe/esvm] Remove plugin configuration

* Removes unused variable

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Named exports for creating clusters

* [tribe] Named exports for client logger, cluster

* [tribe] Named exports for health check

* Remove invalid comment

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Comment explaining difference between admin and data browser clients

* Rename ES checks to be consistant with functionality

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Organize NOOP functions

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Removing function comments

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Explicitly check for presence of url in tribe

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
tylersmalley pushed a commit that referenced this pull request Jan 3, 2017
* Adds support for Tribe nodes

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* @spalger review feedback

* Close create{Admin,Data}Cluster handles closing the connection
* Remove callAsKibanaUser argument from tests
* ClientLogger uses ES6 properties for tags and logQueries
* Ensure were destructuring cluster to access callAsKibanaUser

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Use class syntax on new data sources

* [tribe] Use includes instead of indexOf in call_client

* [tribe] DocRequest --> AbstractDocRequest

* [tribe] Fix AbstractDoc test rename

* Removes factory objects and adds addClientPlugin to Cluster (#9467)

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Resolves eslint error

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Use properties on the instance instead of class properties

Class properties are still in the very eary stages and not widely supported.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Remove disabled dev tools app, do not bundle console when tribe is enabled

* [tribe] Use destructuring, don't reassign args

* [tribe] Use class syntax for client request wrapper

* [tribe] callAsKibanaUser -> callWithInternalUser

* [tribe] Remove clients from module context, service is a singleton

* [tribe] Use instance property shorthand for admin and data DocRequests

* Removes questions

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Fixes typo in tests

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Correctly names test case

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Revert "Use properties on the instance instead of class properties"

This reverts commit ebd06ae.

* Adds tests for create_{admin,data}_cluster

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Persists clusters to server

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Move cluster config requests to distinct getters

* Adds getClient and removes addClientPlugin

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Expose createClient, consolidate config parsing

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Removes createClients from Cluster

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Prevent status change from red to red

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Updates esvm:tribe ports to be consistant with dev

9200 is admin
9201:9202 are both data clusters
9203 is a tribe node connecting to both data clusters

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Get ssl.ca from serverConfig

* [tribe/esvm] Remove plugin configuration

* Removes unused variable

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Named exports for creating clusters

* [tribe] Named exports for client logger, cluster

* [tribe] Named exports for health check

* Remove invalid comment

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* [tribe] Comment explaining difference between admin and data browser clients

* Rename ES checks to be consistant with functionality

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Organize NOOP functions

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Removing function comments

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

* Explicitly check for presence of url in tribe

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
weronikaolejniczak added a commit that referenced this pull request Jan 14, 2026
## Dependency updates

- `@elastic/eui`: `v111.0.0` ⏩ `v111.1.0`
- `@elastic/eui-theme-borealis`: `v5.2.0` ⏩ `v5.3.0`

---

## Changes

- Removed `euiBasicTable.tableCaptionWithPagination`,
`euiBasicTable.tableAutoCaptionWithPagination`,
`euiBasicTable.tableSimpleAutoCaptionWithPagination`,
`euiBasicTable.tableAutoCaptionWithoutPagination` i18n tokens
- Added `euiBasicTable.caption.itemCountPart.withTotalItemCount`,
`euiBasicTable.caption.paginationPart.withPageCount`,
`euiBasicTable.caption.tableName`, `euiBasicTable.caption.emptyState`
i18n tokens
- Updated snapshot tests
- Updated a couple of Jest assertions due to [this EUI
change](https://github.com/elastic/eui/pull/9254/changes)), see
127ab80

## Package updates

### `@elastic/eui`
[v111.1.0](https://github.com/elastic/eui/releases/tag/v111.1.0)

- Added `dashedCircle` icon
([#9278](elastic/eui#9278))
- Added `crossProjectSearch` icon
([#9275](elastic/eui#9275))
- Added component token `components.tourStepIndicatorInactiveColor` and
`components.tourStepIndicatorActiveColor`
([#9271](elastic/eui#9271))
- Remapped `EuiBeacon` component `success` variant to use `success`
color token instead of `accentSecondary`
([#9271](elastic/eui#9271))
- Added `EuiSplitButton` and its respective sub-components
`EuiSplitButton.ActionPrimary` and `EuiSplitButton.ActionSecondary`
([#9269](elastic/eui#9269))
- Added `productRobot` icon
([#9259](elastic/eui#9259))
- Added beta `euiContainer()`, `euiContainerCSS()`, and
`euiContainerQuery()` Emotion utilities to help work with CSS Container
Queries ([#9264](elastic/eui#9264))
- Added `useEuiContainerQuery` hook to observe container query changes
in JavaScript ([#9251](elastic/eui#9251))
- Updated EuiFlexGroup's `gutterSize` from `l` to `m`
([#9132](elastic/eui#9132))
- Updated EuiSpacer's `size` from `l` to `m`
([#9132](elastic/eui#9132))
- Updated EuiHorizontalRule's `margin` from `l` to `m`
([#9132](elastic/eui#9132))
- Updated EuiPageHeader's tab `size` from `l` to `m`
([#9132](elastic/eui#9132))
- Updated EuiEmptyPrompt's spacer `size` between title and text from `m`
to `s` ([#9132](elastic/eui#9132))
- Updated EuiSearchBar's `gutterSize` from `m` to `s`
([#9132](elastic/eui#9132))

<img width="2158" height="392" alt="image"
src="https://github.com/user-attachments/assets/e217f5a5-5b4f-48df-830d-a60861939945"
/>
<img width="1692" height="608" alt="image"
src="https://github.com/user-attachments/assets/d1f49e86-ad8c-4d80-9d02-54c73baae616"
/>
<img width="2182" height="302" alt="image"
src="https://github.com/user-attachments/assets/295c768c-a2df-48f5-80cb-1a1ce5b19e00"
/>
<img width="1904" height="1066" alt="image"
src="https://github.com/user-attachments/assets/f96cbde6-0529-4f5b-be5b-b56f28c5d2b7"
/>
<img width="1566" height="128" alt="image"
src="https://github.com/user-attachments/assets/be4df105-9e32-4a9d-89f0-fe973f441495"
/>

**Bug fixes**

- Fixed flyout overlay masks not being visible for `EuiDataGrid`'s
fullscreen mode by reducing the `z-index` of the fullscreen mode overlay
([#9267](elastic/eui#9267))

**Accessibility**

- Added information about the empty state of `EuiBasicTable` in the
table caption ([#9265](elastic/eui#9265))
- Improved `EuiBasicTable` accessibility by ensuring a fallback
`tableCaption` is applied if none is provided
([#9254](elastic/eui#9254))

### `@elastic/eui-theme-borealis` v5.3.0

- Added component token `components.tourStepIndicatorInactiveColor` and
`components.tourStepIndicatorActiveColor`
([#9271](elastic/eui#9271))
smith pushed a commit to smith/kibana that referenced this pull request Jan 16, 2026
## Dependency updates

- `@elastic/eui`: `v111.0.0` ⏩ `v111.1.0`
- `@elastic/eui-theme-borealis`: `v5.2.0` ⏩ `v5.3.0`

---

## Changes

- Removed `euiBasicTable.tableCaptionWithPagination`,
`euiBasicTable.tableAutoCaptionWithPagination`,
`euiBasicTable.tableSimpleAutoCaptionWithPagination`,
`euiBasicTable.tableAutoCaptionWithoutPagination` i18n tokens
- Added `euiBasicTable.caption.itemCountPart.withTotalItemCount`,
`euiBasicTable.caption.paginationPart.withPageCount`,
`euiBasicTable.caption.tableName`, `euiBasicTable.caption.emptyState`
i18n tokens
- Updated snapshot tests
- Updated a couple of Jest assertions due to [this EUI
change](https://github.com/elastic/eui/pull/9254/changes)), see
elastic@127ab80

## Package updates

### `@elastic/eui`
[v111.1.0](https://github.com/elastic/eui/releases/tag/v111.1.0)

- Added `dashedCircle` icon
([elastic#9278](elastic/eui#9278))
- Added `crossProjectSearch` icon
([elastic#9275](elastic/eui#9275))
- Added component token `components.tourStepIndicatorInactiveColor` and
`components.tourStepIndicatorActiveColor`
([elastic#9271](elastic/eui#9271))
- Remapped `EuiBeacon` component `success` variant to use `success`
color token instead of `accentSecondary`
([elastic#9271](elastic/eui#9271))
- Added `EuiSplitButton` and its respective sub-components
`EuiSplitButton.ActionPrimary` and `EuiSplitButton.ActionSecondary`
([elastic#9269](elastic/eui#9269))
- Added `productRobot` icon
([elastic#9259](elastic/eui#9259))
- Added beta `euiContainer()`, `euiContainerCSS()`, and
`euiContainerQuery()` Emotion utilities to help work with CSS Container
Queries ([elastic#9264](elastic/eui#9264))
- Added `useEuiContainerQuery` hook to observe container query changes
in JavaScript ([elastic#9251](elastic/eui#9251))
- Updated EuiFlexGroup's `gutterSize` from `l` to `m`
([elastic#9132](elastic/eui#9132))
- Updated EuiSpacer's `size` from `l` to `m`
([elastic#9132](elastic/eui#9132))
- Updated EuiHorizontalRule's `margin` from `l` to `m`
([elastic#9132](elastic/eui#9132))
- Updated EuiPageHeader's tab `size` from `l` to `m`
([elastic#9132](elastic/eui#9132))
- Updated EuiEmptyPrompt's spacer `size` between title and text from `m`
to `s` ([elastic#9132](elastic/eui#9132))
- Updated EuiSearchBar's `gutterSize` from `m` to `s`
([elastic#9132](elastic/eui#9132))

<img width="2158" height="392" alt="image"
src="https://github.com/user-attachments/assets/e217f5a5-5b4f-48df-830d-a60861939945"
/>
<img width="1692" height="608" alt="image"
src="https://github.com/user-attachments/assets/d1f49e86-ad8c-4d80-9d02-54c73baae616"
/>
<img width="2182" height="302" alt="image"
src="https://github.com/user-attachments/assets/295c768c-a2df-48f5-80cb-1a1ce5b19e00"
/>
<img width="1904" height="1066" alt="image"
src="https://github.com/user-attachments/assets/f96cbde6-0529-4f5b-be5b-b56f28c5d2b7"
/>
<img width="1566" height="128" alt="image"
src="https://github.com/user-attachments/assets/be4df105-9e32-4a9d-89f0-fe973f441495"
/>

**Bug fixes**

- Fixed flyout overlay masks not being visible for `EuiDataGrid`'s
fullscreen mode by reducing the `z-index` of the fullscreen mode overlay
([elastic#9267](elastic/eui#9267))

**Accessibility**

- Added information about the empty state of `EuiBasicTable` in the
table caption ([elastic#9265](elastic/eui#9265))
- Improved `EuiBasicTable` accessibility by ensuring a fallback
`tableCaption` is applied if none is provided
([elastic#9254](elastic/eui#9254))

### `@elastic/eui-theme-borealis` v5.3.0

- Added component token `components.tourStepIndicatorInactiveColor` and
`components.tourStepIndicatorActiveColor`
([elastic#9271](elastic/eui#9271))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants