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

reverseproxy: Implement admin API for reporting upstream statuses #4125

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

francislavoie
Copy link
Member

As discussed in https://caddy.community/t/cli-to-query-for-upstream-server-status/12210

$ curl localhost:2019/reverse_proxy/upstreams
[
  {"address": "localhost:10002", "healthy": true, "num_requests": 0, "fails": 0},
  {"address": "localhost:10000", "healthy": true, "num_requests": 0, "fails": 0},
  {"address": "localhost:10001", "healthy": true, "num_requests": 0, "fails": 0}
]

Tested with a Caddyfile like this:

{
        auto_https off
}

:10000 {
        respond "10000"
}

:10001 {
        respond "10001"
}

:10002 {
        respond "10002"
}

:9999 {
        reverse_proxy localhost:10000 localhost:10001 localhost:10002
}

@francislavoie francislavoie added this to the 2.x milestone Apr 21, 2021
@francislavoie francislavoie requested a review from mholt April 21, 2021 06:12
@francislavoie
Copy link
Member Author

francislavoie commented Apr 21, 2021

Something funky's going on with github actions... the tests pass but the jobs hard-fail with no results sometimes. I'm very confused. https://www.githubstatus.com/ seems to say everything is fine right now though.

Edit: Re-ran a couple times, finished fine this time 🤷‍♂️

@mholt mholt modified the milestones: 2.x, v2.4.0 Apr 21, 2021
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Cool, this is basically exactly what I had in mind. I'll spare a few little nits and just go with it. Thanks! This should be pretty useful.

modules/caddyhttp/reverseproxy/admin.go Show resolved Hide resolved
@mholt mholt merged commit 4d0474e into caddyserver:master Apr 21, 2021
@francislavoie francislavoie deleted the proxy-admin branch April 21, 2021 20:16
@ttys3
Copy link
Contributor

ttys3 commented Sep 13, 2021

I see this not work.

The num_requests always 0, and the healthy is always true even it is not healthy.

about the num_requests, I saw the code:

defer di.Upstream.Host.CountRequest(-1)

don't understand if we need the num_requests, then why decrease it in defer ?

@francislavoie


Sorry for the distrub, I've read the document explanation carefully and now see it.
I was wrong, I assume that it means total sum without reading the doc.

But, anyway, the num_requests field is confusing,
I think maybe name it to active_requests better.

@francislavoie
Copy link
Member Author

francislavoie commented Sep 13, 2021

@ttys3 it depends on how you have your health checking configured. Please ask on the forums! https://caddy.community. Also see the docs, which have an explanation of how it works https://caddyserver.com/docs/api#get-reverse-proxyupstreams

@ttys3
Copy link
Contributor

ttys3 commented Sep 13, 2021

Sorry for the distrub, I've read the document explanation carefully and now see it.
I was wrong, I assume that it means total sum without reading the doc.

But, anyway, the num_requests field is confusing,
I think maybe name it to active_requests better.

@francislavoie
Copy link
Member Author

The admin API matches the field names in the internal structs. Changing it now would be a breaking change. The documentation explains it sufficiently IMO, so no need to change it.

@ttys3
Copy link
Contributor

ttys3 commented Sep 13, 2021

Ok. got it.

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.

4 participants