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

hubble/metrics: Add support for fallback labels, ip addresses and dns names #14848

Merged
merged 5 commits into from
Feb 9, 2021

Conversation

gandro
Copy link
Member

@gandro gandro commented Feb 3, 2021

This adds a two context option to the Hubble metrics, dns and ip.

dns adds the DNS name as the source or destination label. If multiple DNS names are associated with an IP address, all of them are added.
ip adds the IPv4 or IPv6 address as the source or destination label.

In addition, this PR also fixes some missing cases around policy verdict and capture events. It also add support for specifying multiple contexts to support fall-back. For example, a metric configuration of flow:destinationContext=dns|ip will first try to use the DNS name of the target for the label, but if no DNS name is known, it will fall back and use the IP address of the target instead.

Except from --hubble-metrics=flow:destinationContext=dns|ip against the connectivity-check.yaml:

hubble_flows_processed_total{destination="1.1.1.1",protocol="TCP",subtype="to-stack",type="Trace",verdict="FORWARDED"} 42
hubble_flows_processed_total{destination="www.google.com",protocol="TCP",subtype="to-stack",type="Trace",verdict="FORWARDED"} 63

@gandro gandro added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/hubble Impacts hubble server or relay labels Feb 3, 2021
@gandro gandro requested a review from a team as a code owner February 3, 2021 13:22
@gandro gandro requested review from a team and qmonnet February 3, 2021 13:22
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Feb 3, 2021
@gandro gandro requested a review from glibsm February 3, 2021 13:22
Flow events can originate from trace, drop, capture or policy verdict
events. The metrics code did not have the necessary cases to deal with
the latter two and treated them as unknown events instead.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This adds a new context option to the Hubble metrics, which adds the DNS
name as the source or destination label. If multiple DNS names are
associated with an IP address, all of them are added.

Example metric for `--hubble-metrics=flow:destinationContext=dns` in the
`connectivity-check.yaml`:

```
hubble_flows_processed_total{destination="www.google.com",protocol="TCP",subtype="",type="PolicyVerdict",verdict="FORWARDED"} 57
```

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/hubble-metrics-dns-context branch from cc37ac1 to 292595e Compare February 3, 2021 13:24
@qmonnet qmonnet removed their assignment Feb 3, 2021
@gandro gandro removed the request for review from glibsm February 3, 2021 16:27
@gandro
Copy link
Member Author

gandro commented Feb 3, 2021

test-me-please

@michi-covalent
Copy link
Contributor

@gandro it might be useful to fall back to other context if dns name is not set? for example, if the destination is inside the cluster, i'd still like to see namespace / pod name, etc 🤔 not sure if it makes sense, but i was thinking something like:

--hubble-metrics=flow:destinationContext=namespace;prefer-dns=true

or something

@gandro
Copy link
Member Author

gandro commented Feb 4, 2021

@gandro it might be useful to fall back to other context if dns name is not set? for example, if the destination is inside the cluster, i'd still like to see namespace / pod name, etc thinking not sure if it makes sense, but i was thinking something like:

--hubble-metrics=flow:destinationContext=namespace;prefer-dns=true

or something

I'd like to avoid making DNS a special case. All the other context options (except maybe identity) have the same problem in that the target might be empty. But maybe we can support a list of targets that are tried in sequence.

@gandro gandro marked this pull request as draft February 4, 2021 13:09
This extends the `sourceContext` and `destinationContext` option in such
a way that multiple contexts can be specified. For example:

    flow:sourceContext=dns|pod;destinationContext=identity

Implies that Hubble first tries to use the DNS name as the source
context. If this yields an empty result, it tries to use an identity
instead. If that still yields an empty result, the source context
remains empty. In other words: The first non-empty context is added as
the label. The `destinationContext` in the above example still works as
before: The context label contains the destination identity, otherwise
an empty string.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This adds a new source and destination context which adds the source or
destination IP as a label to metrics which support context options.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This documents the newly added `dns` and `ip` context options, as well
as the ability to specify multiple fallback contexts.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
@gandro gandro force-pushed the pr/gandro/hubble-metrics-dns-context branch from 292595e to 824db3f Compare February 4, 2021 14:25
@gandro gandro changed the title hubble/metrics: Add dns context option hubble/metrics: Add support for fallback labels, ip addresses and dns names Feb 4, 2021
@gandro gandro marked this pull request as ready for review February 4, 2021 14:37
@gandro
Copy link
Member Author

gandro commented Feb 4, 2021

I have reworked the PR a bit, please see the PR description for details. @michi-covalent your use-case should now be supported by the following syntax:

--hubble-metrics=flow:destinationContext=dns|namespace

Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

🚀 lgtm a big 👍 just one question regarding the dns name field.

pkg/hubble/metrics/api/context.go Show resolved Hide resolved
@gandro
Copy link
Member Author

gandro commented Feb 8, 2021

test-me-please

@gandro gandro added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 8, 2021
@borkmann borkmann merged commit b8a698b into cilium:master Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/hubble Impacts hubble server or relay
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants