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

Automatically include `node` label for every `instance` #1217

Closed
SuperQ opened this Issue Nov 13, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@SuperQ
Copy link
Member

SuperQ commented Nov 13, 2015

It's highly useful to be able to metrics comparisons between targets on the same node. For example getting the rate of disk IO from the node_exporter and normalizing that with request counters from mysqld_exporter.

Creating a node label from instance works via metrics relabeling, but it can be very messy. It would be nice to just have this be a default option.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Nov 13, 2015

This is the sort of thing that's going to vary a lot between sites, and not going to work if you've things like an overlay network or multiple DNS names for a given machine in play. As with all cases where you're trying to add labels to targets that are site specific, relabelling is the way to go.

@SuperQ

This comment has been minimized.

Copy link
Member Author

SuperQ commented Nov 13, 2015

Except that a node is a very common labeling method for a lot of network designs. Even in something like kubernetes/docker where you have per-container IP address, you may have multiple targets that have relevant metrics to share with each other. Forcing the node:port as the instance just makes it more difficult to aggregate and combine metrics. These should never have been a single label to begin with. instance_node and instance_port would have been better.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Nov 14, 2015

I think you're confusing __address__ and instance. While __address__ can currently only be a host:port (and it's conceivable we'll have unix domain sockets some day), there's no such restriction on instance. It could be an ip:port, a dnsname:port, a dnsname, an IP, one of the various formattings of IPv6 addresses, a URL, a zookeeper path, a consul path, a EC2 instance ID or anything else that makes sense in the context of your deployment and in particular your service discovery.

This is something that's only make sense in some deployments, and adding in labels like this will cause problems for users as it'll require additional special handling by users to strip off as we improve label handling in promql.

This is not something we can safely do by default due to both safety, and because it's not sane from a maintenance/usability standpoint to offer options, let alone defaults, for the multitude of common scenarios.

@SuperQ

This comment has been minimized.

Copy link
Member Author

SuperQ commented Nov 14, 2015

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Nov 14, 2015

Hmm, I can see that working in some cases but not working in most cases when you consider things like the blackbox exporter which require different handling. In general I'd also expect the node exporter and applications to have different relabel rules. I think this is probably better handled by a for loop in configuration management, as there's likely lots of other aspects that are similar if relabelling is.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Nov 14, 2015

I agree here. This requires a single relabeling rule – way less than needed for other tweaks to the standard behavior.
Overly noisy repetition generated by such loops can often be avoided by making better use of the existing SD capabilities.

Relabeling is not trivial in itself, layering it will make configuration unnecessarily complicated.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Dec 11, 2015

Closing since we solved this accordingly for our use case.

@fabxc fabxc closed this Dec 11, 2015

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.