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

caddyhttp: Make metrics opt-in #5042

Merged
merged 2 commits into from
Sep 16, 2022
Merged

caddyhttp: Make metrics opt-in #5042

merged 2 commits into from
Sep 16, 2022

Conversation

mholt
Copy link
Member

@mholt mholt commented Sep 16, 2022

Related to #4644

Metrics have accumulated an unfortunate performance regression and have some functionality limitations. Since we don't have the time or expertise on hand currently to address these issues in the more ideal way, I think we need to make metrics opt-in for now.

In the JSON, metrics are enabled by setting "metrics": {} in your server config, or in the Caddyfile, use global options:

{
	servers {
		metrics
	}
}

This is EXPERIMENTAL and subject to change.

Note that metrics for handle_response handlers (etc) will not be able to have metrics enabled, at least for now.

I think going forward a better long-term fix might be to have the metrics directive enable metrics, rather than a server-wide config. I'd also like to make certain aspects about metrics actually configurable, rather than having an empty struct.

@mholt mholt added this to the v2.6.0 milestone Sep 16, 2022
Copy link
Collaborator

@hairyhenderson hairyhenderson left a comment

Choose a reason for hiding this comment

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

Worth it until we can get the perf issues sorted out!

Copy link
Member

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

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

I think I agree with this as well, as unfortunate as it is.

I guess we only end up with like admin metrics if this is off? What does the metrics endpoint end up showing? Will it just show "no requests" for the vectors it tracks? That could be confusing/surprising to people who upgrade.

@mholt
Copy link
Member Author

mholt commented Sep 16, 2022

Yeah, I'll make it very clear in the release notes that metrics are opt-in now.

@mholt mholt merged commit 74547f5 into master Sep 16, 2022
@mholt mholt deleted the metrics-toggle branch September 16, 2022 19:32
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.

3 participants