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

docs: clarify the behavior of prepending hostname to metrics #17521

Merged
merged 4 commits into from
Jun 5, 2023

Conversation

huikang
Copy link
Collaborator

@huikang huikang commented May 30, 2023

Description

The prepended name to the gauge metrics causes some confusion on how to query the metrics. According to the go-metrics' godoc, EnableHostname is only applicable to gauge metrics:

EnableHostname   	bool // Enable prefixing gauge values with hostname”

Consul initializes the metrics.config with the agent configuration value “telemetry.disable_hostname”.

consul/lib/telemetry.go

Lines 327 to 329 in 720dda7

func configureSinks(cfg TelemetryConfig, memSink metrics.MetricSink) (metrics.FanoutSink, error) {
metricsConf := metrics.DefaultConfig(cfg.MetricsPrefix)
metricsConf.EnableHostname = !cfg.DisableHostname

Therefore, if disable_hostname is false, hostname will be added to gauge metric names (the default behavior); otherwise, no hostname will be prepended to the metrics name of gauge type. That said, disable_hostname has no impact on other types of metrics.

This PR clarifies this behavior in consul's doc.

Links

fix #17320

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@huikang huikang added pr/no-changelog PR does not need a corresponding .changelog entry backport/1.14 backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. labels May 30, 2023
@huikang huikang requested a review from a team as a code owner May 30, 2023 21:30
@github-actions github-actions bot added the type/docs Documentation needs to be created/updated/clarified label May 30, 2023
@huikang huikang requested review from a team and xwa153 and removed request for a team May 30, 2023 21:31
@huikang huikang requested a review from jjti May 30, 2023 21:34
Copy link
Member

@xwa153 xwa153 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Comment on lines 1821 to 1822
This controls whether or not to prepend runtime telemetry of gauge type with the
machine's hostname, defaults to false (prepend hostname).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This controls whether or not to prepend runtime telemetry of gauge type with the
machine's hostname, defaults to false (prepend hostname).
If enabled this disables prepending the name of gauge type metrics with the
machine's hostname. By default, hostname is prepended.

Copy link
Contributor

@jjti jjti Jun 5, 2023

Choose a reason for hiding this comment

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

possible rephrasing. I still find it a bit tricky to read as is. The section below is very helpful, though

Copy link
Contributor

@trujillo-adam trujillo-adam left a comment

Choose a reason for hiding this comment

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

I added a suggestion that I think will make the setting clearer.

website/content/docs/agent/config/config-files.mdx Outdated Show resolved Hide resolved
Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
@huikang
Copy link
Collaborator Author

huikang commented Jun 5, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

consul_server_isLeader gauge always zero
4 participants