Metrics does not count requests on ingresses without host

[jonaz]

Is this a request for help? Yes , bug with possible solution.

What keywords did you search in NGINX Ingress controller issues before filing this one? missing metrics (#3053 looked promising, but wrong version. according to git commit this bug was introduced in 0.20.0)

---

Is this a BUG REPORT or FEATURE REQUEST? (choose one): BUG REPORT

NGINX Ingress controller version: 0.22.0 (bug introduced in 0.20.0)

Kubernetes version (use kubectl version): 1.7.14

Environment:

• Cloud provider or hardware configuration: bare metal
• OS (e.g. from /etc/os-release): coreos 1967.4.0
• Kernel (e.g. uname -a): 4.14.96-coreos
• Install tools:
• Others:

What happened: ingresses without specific host does not report metrics.

What you expected to happen: All ingresses should have metrics.

How to reproduce it (as minimally and precisely as possible):
Create an ingress without host set.

Anything else we need to know:

Hi we just upgraded to 0.22.0 from 0.15.0 and now have the issue with dissapearing metrics.

I looked a the code and found this commit which introduces the problem:
9766ad8

If you have an ingress that does not specify host it will never find a match and not increase the metric counter.

So all ingresses without host will be without metrics now :(

I'm not 100% familiar with the codebase and not quite sure how to solve it since we cannot know if ingress is missing host at that time. Perhaps hosts field on SocketCollector could be a

map[ingressName]struct{
    Wildcard bool
    Hosts sets.String
}

In that case we can know when not to skip because its a wildcard?

msg in handleMessage in SocketCollector looks like on a request on an ingress that does not have host. It looks korrect.

[{
    "requestLength": 1190,
    "ingress": "kafka-http-ingress",
    "status": "201",
    "service": "kafka-http",
    "requestTime": 0.024,
    "namespace": "default",
    "host": "route-utv.fnox.se",
    "method": "POST",
    "upstreamResponseTime": 0.024,
    "upstreamResponseLength": 4,
    "upstreamLatency": 0.014,
    "path": "\/internalapi\/kafka",
    "responseLength": 213
}]

[aledbf]

@jonaz actually this change was intentional. Please check #3116 . How we can differentiate a valid request from a DOS?

[jonaz]

@aledbf Perhaps we could match the prefix of the request path against the paths on the ingress? In addition to the host filtering. In that case we will not have more metrics series than host+path combinations of all ingresses?

Right now we have lost our ability to monitor/alert on a majority of our services request failuere rate... which is really bad for us in the ops department. We use a single domain (for CORS reasons) and just have ingresses routing on path for about 120 microservices.

[jonaz]

@aledbf would you accept PR with proposed fix or do we need to maintain a fork or move to another ingress controller?

[komljen]

@jonaz Did you find a solution for this?

[jonaz]

@komljen no, still waiting for answer frmo @aledbf if my proposed solution would be accepted.

I guess we have to go back to Traefik in the mean time.

[adamelliotfields]

Just wanted to thank @jonaz for opening this issue, as this was the reason Grafana wasn't showing anything for me.

It might be worth mentioning this in the monitoring section of the docs. I'd imagine others might assume a fan-out without a host would generate metrics.

[dotLou]

Would it make sense to change the code here to export all ingress metrics when per-host metrics are disabled though? We're losing a lot of metrics that are essential to monitoring this in production because the metrics are just not passed on.

Here's the flag I'm talking about: #3594

Here's the code in question I'm suggesting we change:

ingress-nginx/internal/ingress/metric/collectors/socket.go

Lines 224 to 227 in ddedd16

	if !sc.hosts.Has(stats.Host) {
	klog.V(3).Infof("skiping metric for host %v that is not being served", stats.Host)
	continue
	}

[jonaz]

My solution above would protect against DDOS and also support metrics on all paths+hosts. But no word from maintainers here yet.

[dotLou]

My solution above would protect against DDOS and also support metrics on all paths+hosts. But no word from maintainers here yet.

I like your solution in terms of host labels being present on the metrics, as it definitely protects against high cardinality in case of a DDOS. Having said that though, when metrics-per-host=false, the benefit IMO is that there's no longer a DDOS risk in terms of metrics label cardinality. At least from my point of view, in that case anyway, I would like to get all metrics, including those from any potential DDOS; In fact, a DDOS would become identifiable by a big increase of 404s on the metrics with this capability.

My thinking is that maybe we need both; if metrics-per-host=true, regex check to determine whether to export or not. If metrics-per-host=false, export everything. Alternatively, even if the path doesn't match and metrics-per-host=true, maybe we could remove the host label for things that don't match a known host (this way, we get all the metrics in all cases). Thoughts? Did I understand your suggestion properly?

It would certainly be useful to get some thoughts from some of the maintainers here; I'm interested in submitting at least one PR to do one or the other approach, but I don't want to do that work if there's no interest in accepting such a change.

[choffmeister]

Are there any concrete plans on the timeline for this? We also have the issue, that we basically are blind in terms of nginx metrics, as we give every customer a custom subdomain (so we use a wildcard domain and hence have not metrics). IMHO it would be a good solution, to just record the wildcard host. For example:

We have an ingress for host *.domain.com. If a customer accesses our system via customer1.domain.com, then the nginx could just record this request for *.domain.com. The same for every other xxx.domain.com. All requests handled by the one wildcard ingress go into one bucket. Hence there would be no problem with unbounded cardinality in the metrics and so no DDOS problem. But we could still distinguish requests for the different ingresses.

I would try to help improve this, but I have no experience with Go :(

[choffmeister]

I created a PR #4139 to fix that one gets metrics at least when running with --metrics-per-host=false.

[alexgavrisco]

I totally agree with @choffmeister - there are quite a few use cases where you have to use wildcards and still get metrics (without tagging them with host).
There must be a way to get metrics even for ingress with a wildcard, without getting a DOS of the Prometheus/etc.

However, I think that this behavior must be controlled via a separate option/flag for several reasons:

• for folks that use this metrics-per-host, changing its behavior is a breaking change
• there are use cases when you might want to get metrics for the wildcard and tag them with the hostname (e.g. another LB in front of nginx).

My suggestion would be a flag that disables filtering metrics for ingress with wildcard and keep the metrics-per-host with its current behavior.

@aledbf What do you think about this approach?

[mosheBelostotsky]

Hi @jonaz Any news with this issue?
It's very important for our use-case...

[rlees85]

Shame the PR seems to have gone quiet.... its also blocking us from upgrading from 0.18 so looking forward to it being merged

[hairyhenderson]

@jonaz @aledbf any progress on this? We're essentially flying blind with our ingress right now because of this bug...

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[essh]

/remove-lifecycle stale

[jonaz]

@jonaz @aledbf any progress on this? We're essentially flying blind with our ingress right now because of this bug...

@hairyhenderson
since no reply from @aledbf regarding my implementation suggestion we are migrating to traefik 2.0 instead.

[jacksontj]

@aledbf I also just ran into this issue (today as a matter-of-fact). IMO the linked PR (#4139) is a reasonable way to go. In the case that someone has a wildcard ingress IMO it makes sense to just exclude the host label -- since then I can still get metrics based on the ingress name (which is also a fixed cardinality).

Is there anything remaining to get the PR merged in?

[rlees85]

@aledbf I also just ran into this issue (today as a matter-of-fact). IMO the linked PR (#4139) is a reasonable way to go. In the case that someone has a wildcard ingress IMO it makes sense to just exclude the host label -- since then I can still get metrics based on the ingress name (which is also a fixed cardinality).

Is there anything remaining to get the PR merged in?

Very unlikely to ever happen. I'd recommend you name all of your ingresses (this is the approach I took to get around this issue). It isn't that hard, especially when using some form of automation to deploy Kubernetes apps (Helm or in my case Terraform). Sticky Session also starts working again when doing this :)

Long term, traefik might be the way to go, depending on if or not ingress-nginx picks up a new maintainer with aledbf stepping down.

Disclaimer: this isn't a dig, ingress-nginx is a great piece of work and I highly appreciate the work aledbf has done on it. Just trying to point out a workaround and the fact things are bleak right now but hopefully someone/some others will step up.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[jacksontj]

/remove-lifecycle stale

[shivpathak]

I am also experiencing the same issue not getting nginx_ingress_controller_requests metrics if I don't define the host
e.g.

This doesn't generate metrics for nginx_ingress_controller_requests

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    kubernetes.io/ingress.class: service-nginx
  name: observability-es
  namespace: capabilities
spec:
  rules:
  - http:
      paths:
      - backend:
          serviceName: elasticsearch-es-http
          servicePort: 9200
        path: /

This generate metrics for nginx_ingress_controller_requests

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    kubernetes.io/ingress.class: service-nginx
  name: observability-es
  namespace: capabilities
spec:
  rules:
  - host: dev.microservices.test.test
    http:
      paths:
      - backend:
          serviceName: elasticsearch-es-http
          servicePort: 9200
        path: /

Would be great if we could get the fix for this.

[moljor]

I even lose my metrics when I made my host a wildcard.
So the host exists it just changed from e.g. "prod.something.com" to "*.something.com".

[DanOfir]

I also cannot see metrics of hosts with wildcard, there is a workaround?

[hairyhenderson]

@DanOfir no, this has effectively stalled, and caused more than a few people to simply switch to a different ingress controller. See #4139 (comment) for some related conversation.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[essh]

/remove-lifecycle stale

[aledbf]

Closing. Fixed in master #4139