Skipper: Support custom HTTP handlers added to the support listener

[azarakovskiy]

Is your feature request related to a problem? Please describe.
I am trying to add some custom listeners that would work on the support port.

As a specific example, I want to change a health check endpoint from being just a normal route returned in my custom data client to the support listener. Because of that, two things happen:

1. Every request to /healthcheck is logged in access log.
2. Healthchecks happen super often, and metrics for other "real" requests are therefore messed up.

Describe the solution you would like
I suggest having a field in the configuration that allows passing a list/map of HTTP handlers which are added to SupportListener.

Describe alternatives you've considered (optional)
To fix what I have stated as a problem, I considered creating a new listener on the third port (additional to SKIPPER_PORT and SUPPORT_PORT), but it just feels wrong.

Additional context (optional)
This draft PR shows the main idea. It is not complete in any sense, it was only created to show the proposed approach.

Would you like to work on it?
Yes

[szuecs]

@azarakovskiy Can you add more context regards to other useful examples?
I don't think that you should have a health check on a support listener, because the main listener could just crash and support listener happily replies healthy to all the health checks. (think of it as a general recommendation, that you can happily ignore)
I understand that the feature could make a lot of sense for other use cases and I would like to have a brief idea of it.

@aryszka @AlexanderYastrebov do you see any blockers, why we should not want such a feature?

[AlexanderYastrebov]

I want to change a health check endpoint from being just a normal route returned in my custom data client to the support listener

I honestly did not get why. Also @szuecs comment about having health check on main listener seems very reasonable if not a best practice.

A route:

health: Path("/healthz") -> disableAccessLog() -> inlineContent("ok") -> <shunt>;

should not produce any access logs and I am not sure skipper writes access logs for support listeners at all. IMO clarification is needed.

[AlexanderYastrebov]

@szuecs I dont see any blocker but would like to better understand the need.

[azarakovskiy]

Hi @szuecs @AlexanderYastrebov thanks for your replies.

I don't think that you should have a health check on a support listener, because the main listener could just crash and support listener happily replies healthy to all the health checks.

This is a super valid concern, I have not thought about it. I believe this is a very good reason _not_ to put the healthcheck on support port.

health: Path("/healthz") -> disableAccessLog() -> inlineContent("ok") -> <shunt>;

This actually might solve the issue I am having and reduce "spam" in access logs. The second issue is that access to the healthcheck also counts in Prometheus metrics., and since healthchecks are often I don't see the correct number of real requests. I will check if disableAccessLog() removes this as well. If not, I can always just exclude this route from Prometheus query, so not a real issue.

Thank you both, I don't think anymore that I need this change for the healthcheck.

---

On the other hand, this might be a useful feature to be able to pass some custom support handlers. I don't have a concrete use case at the moment though. IDK, a random example, maybe one wants to get a list of microservices around that are alive/dead to know the overall health of the system? Well, you'd do this outside Skipper, I think...

[szuecs]

Just to add with these filters [1], you can select status code you like to have access logs or not.
Do you want to close the issue and PR?

[1]
https://opensource.zalando.com/skipper/reference/filters/#enableaccesslog
https://opensource.zalando.com/skipper/reference/filters/#disableaccesslog

[aryszka]

In general, I like the idea of the custom support listeners, but as it was already discussed, the healthcheck is just the wrong example.

I am closing this PR and issue for now. Please feel free to reopen or create a new issue to discuss the custom handlers topic, and same for the PR.