Added node class to tagged metrics #3882
Conversation
|
nice! |
|
@jippi hope it would be nice -19 ;) |
client/client.go
Outdated
| if emittedNodeClass = c.config.Node.NodeClass; emittedNodeClass == "" { | ||
| emittedNodeClass = "none" | ||
| } | ||
| c.baseLabels = []metrics.Label{ |
There was a problem hiding this comment.
Can you update to
c.baseLabels = []metrics.Label{{Name: "node_id", Value: c.NodeID()}, {Name: "datacenter", Value: c.Datacenter()}}
if node := c.Node(); node.NodeClass != "" {
c.baseLabels = append(c.baseLabels, {Name: "node_class", Value: node.NodeClass})
}
There was a problem hiding this comment.
sure! original idea was to add "none" like it's listed in CLI nomad node-status
There was a problem hiding this comment.
I like none idea - having a sparse field for class will be confusing :)
There was a problem hiding this comment.
we will not add the flag to report "none" client class, right?
There was a problem hiding this comment.
Problem is all possible strings you could add are going to be valid possible node classes
There was a problem hiding this comment.
@dadgar while working on nomad monitoring tutorial I ran into a problem with grouping nodes by class when node class is not emitted from not nodes with no class configured, so I reverted the code to my previous version when none is emitted from not tagged nodes.
The issue of all possible strings are going to be valid can be addressed on config parsing stage.
Let me know if you want me to add some restrictions there.
burdandrei
force-pushed
the
telemetry-add-node-class-tag
branch
from
efd0e1d
to
2b8cb3f
Compare
|
@dadgar I rebased on master and made as you asked, but the build is failing on https://github.com/hashicorp/nomad/blob/master/nomad/structs/structs.go#L2966 |
burdandrei
force-pushed
the
telemetry-add-node-class-tag
branch
from
2b8cb3f
to
d9bb450
Compare
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
For easier monitoring nomad cluster parts