Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Monitoring] Remove dependency on source_node.uuid #23721

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions x-pack/plugins/monitoring/server/lib/__tests__/create_query.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ describe('Create Type Filter', () => {

describe('Create Query', () => {
beforeEach(() => {
metric = ElasticsearchMetric.getMetricFields();
metric = {
...ElasticsearchMetric.getMetricFields(),
field: 'node_stats.some_field'
};
});

it('Allows UUID to not be passed', () => {
Expand All @@ -42,7 +45,7 @@ describe('Create Query', () => {
const result = createQuery(options);
let expected = {};
expected = set(expected, 'bool.filter[0].term', {
'source_node.uuid': 'abc123'
'node_stats.node_id': 'abc123'
});
expected = set(expected, 'bool.filter[1].range.timestamp', {
format: 'epoch_millis',
Expand Down Expand Up @@ -86,6 +89,7 @@ describe('Create Query', () => {
function callCreateQuery() {
const options = { uuid: 'abc123', metric };
delete options.metric.uuidField;
delete options.metric.field;
return createQuery(options);
}
expect(callCreateQuery).to.throwException((e) => {
Expand Down Expand Up @@ -113,7 +117,7 @@ describe('Create Query', () => {
let expected = {};
expected = set(expected, 'bool.filter[0].bool.should', [ { term: { _type: 'test-type-yay' } }, { term: { type: 'test-type-yay' } } ]);
expected = set(expected, 'bool.filter[1].term', {
'source_node.uuid': 'abc123'
'node_stats.node_id': 'abc123'
});
expected = set(expected, 'bool.filter[2].range.timestamp', {
format: 'epoch_millis',
Expand Down
17 changes: 16 additions & 1 deletion x-pack/plugins/monitoring/server/lib/create_query.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ export const createTypeFilter = (type) => {
};
};

function getUuidFieldFromType(type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to either implicitly figure this out or explicitly require it for each metric. It's an issue once this is fixed

switch (type) {
case 'node_stats':
return 'node_stats.node_id';
}
throw 'Unable to infer uuid field from type: ' + type;
}

/*
* Creates the boilerplace for querying monitoring data, including filling in
* document UUIDs, start time and end time, and injecting additional filters.
Expand Down Expand Up @@ -63,7 +71,14 @@ export function createQuery(options) {
let uuidFilter;
// options.uuid can be null, for example getting all the clusters
if (uuid) {
const uuidField = get(options, 'metric.uuidField');
let uuidField = get(options, 'metric.uuidField');
if (!uuidField) {
const field = get(options, 'metric.field');
if (field) {
const documentType = field.split('.')[0];
uuidField = getUuidFieldFromType(documentType);
}
}
if (!uuidField) {
throw new MissingRequiredError('options.uuid given but options.metric.uuidField is false');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ export function handleResponse(clusterState, shardStats, nodeUuid) {
return response => {
let nodeSummary = {};
const nodeStatsHits = get(response, 'hits.hits', []);
const nodes = nodeStatsHits.map(hit => hit._source.source_node); // using [0] value because query results are sorted desc per timestamp
const nodes = nodeStatsHits.map(hit => hit._source.node_stats); // using [0] value because query results are sorted desc per timestamp
const node = nodes[0] || getDefaultNodeFromId(nodeUuid);
const sourceStats = get(response, 'hits.hits[0]._source.node_stats');
const clusterNode = get(clusterState, [ 'nodes', nodeUuid ]);
const stats = {
resolver: nodeUuid,
node_ids: nodes.map(node => node.uuid),
node_ids: nodes.map(node => node.node_id),
attributes: node.attributes,
transport_address: node.transport_address,
name: node.name,
Expand Down Expand Up @@ -72,7 +72,7 @@ export function getNodeSummary(req, esIndexPattern, clusterState, shardStats, {
// Build up the Elasticsearch request
const metric = ElasticsearchMetric.getMetricFields();
const filters = [{
term: { 'source_node.uuid': nodeUuid }
term: { 'node_stats.node_id': nodeUuid }
}];
const params = {
index: esIndexPattern,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ export async function getNodes(req, esIndexPattern, clusterStats, shardStats) {
metric: metricFields
}),
collapse: {
field: 'source_node.uuid'
field: 'node_stats.node_id'
},
aggs: {
nodes: {
terms: {
field: `source_node.uuid`,
field: `node_stats.node_id`,
size: config.get('xpack.monitoring.max_bucket_size')
},
aggs: getMetricAggs(LISTING_METRICS_NAMES, bucketSize)
Expand All @@ -78,7 +78,7 @@ export async function getNodes(req, esIndexPattern, clusterStats, shardStats) {
sort: [ { timestamp: { order: 'desc' } } ]
},
filterPath: [
'hits.hits._source.source_node',
'hits.hits._source.node_stats',
'aggregations.nodes.buckets.key',
...LISTING_METRICS_PATHS,
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ export function mapNodesInfo(nodeHits, clusterStats, shardStats) {
const clusterState = get(clusterStats, 'cluster_state', { nodes: {} });

return nodeHits.reduce((prev, node) => {
const sourceNode = get(node, '_source.source_node');
const sourceNode = get(node, '_source.node_stats');

const calculatedNodeType = calculateNodeType(sourceNode, get(clusterState, 'master_node'));
const { nodeType, nodeTypeLabel, nodeTypeClass } = getNodeTypeClassLabel(sourceNode, calculatedNodeType);
const isOnline = !isUndefined(get(clusterState, [ 'nodes', sourceNode.uuid ]));
const isOnline = !isUndefined(get(clusterState, [ 'nodes', sourceNode.node_id ]));

return {
...prev,
[sourceNode.uuid]: {
name: sourceNode.name,
[sourceNode.node_id]: {
name: sourceNode.node_id,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pinging @ycombinator. Do we have a replacement plan for this? I can't remember from our earlier discussions, but are we able to inject the node's name (whatever node the beat is talking to) into the document shipped by MB? Or what was our plan for this?

transport_address: sourceNode.transport_address,
type: nodeType,
isOnline,
nodeTypeLabel: nodeTypeLabel,
nodeTypeClass: nodeTypeClass,
shardCount: get(shardStats, `nodes[${sourceNode.uuid}].shardCount`, 0),
shardCount: get(shardStats, `nodes[${sourceNode.node_id}].shardCount`, 0),
}
};
}, {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ export function getShardAggs(config, includeNodes) {
aggs: {
index_count: { cardinality: { field: 'shard.index' } },
node_names: {
terms: { field: 'source_node.name', size: aggSize }
terms: { field: 'node_stats.name', size: aggSize }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This field doesn't seem to exist so this won't work

},
node_ids: {
terms: { field: 'source_node.uuid', size: 1 } // node can only have 1 id
terms: { field: 'shard.node', size: 1 } // node can only have 1 id
}
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export class ElasticsearchMetric extends Metric {
super({
...opts,
app: 'elasticsearch',
uuidField: 'source_node.uuid',
timestampField: 'timestamp'
});

Expand All @@ -28,7 +27,6 @@ export class ElasticsearchMetric extends Metric {

static getMetricFields() {
return {
uuidField: 'source_node.uuid', // ???
timestampField: 'timestamp'
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ export const metrics = {
// CGroup CPU Utilization Fields
const quotaMetricConfig = {
app: 'elasticsearch',
uuidField: 'source_node.uuid',
uuidField: 'node_stats.node_id',
timestampField: 'timestamp',
fieldSource: 'node_stats.os.cgroup',
usageField: 'cpuacct.usage_nanos',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export function esNodeRoute(server) {
const clusterState = get(cluster, 'cluster_state', { nodes: {} });
const shardStats = await getShardStats(req, esIndexPattern, cluster, { includeIndices: true, includeNodes: true });
const nodeSummary = await getNodeSummary(req, esIndexPattern, clusterState, shardStats, { clusterUuid, nodeUuid, start, end });
const metrics = await getMetrics(req, esIndexPattern, metricSet, [{ term: { 'source_node.uuid': nodeUuid } }]);
const metrics = await getMetrics(req, esIndexPattern, metricSet, [{ term: { 'node_stats.node_id': nodeUuid } }]);

let shardAllocation;
if (!isAdvanced) {
Expand Down