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

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Oct 2, 2018

Relates to #23720

STILL A WIP

This PR removes the dependency on source_node.uuid in the monitoring codebase. We want to move away from indexing this and this PR is the first step of the effort to shift our queries and response logic to look for and use this data elsewhere in the document.

Blockers

There are some metrics that rely on using the source_node.uuid to filter the data appropriately but the document type in which these metrics run doesn't have a 1-1 replacement for the uuid, notably metrics that rely on index_stats document types. Some document types (node_stats and shards) have a 1-1 replacement so those are fine.

@@ -26,6 +26,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

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

[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?

@@ -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

@chrisronline
Copy link
Contributor Author

@ycombinator and I synced about this and have decided to NOT pursue this effort. We're going to continue to rely on the source_node field as Metricbeat is currently still indexing it. We will take the time to remove this dependency once the UI is reading from Metricbeat indices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants