Skip to content
This repository was archived by the owner on Feb 19, 2025. It is now read-only.

Remove remote_port label from flow_traffic metrics#95

Open
slrtbtfs wants to merge 1 commit into
cloudflare:masterfrom
slrtbtfs:flow_explosion
Open

Remove remote_port label from flow_traffic metrics#95
slrtbtfs wants to merge 1 commit into
cloudflare:masterfrom
slrtbtfs:flow_explosion

Conversation

@slrtbtfs

Copy link
Copy Markdown

Fixes #94.

However it theoretically has the potential to break some existing usecases, where the
distiction between remote ports is relevant.

Signed-off-by: Tobias Guggenmos tobias.guggenmos@uni-ulm.de

Fixes cloudflare#94.

However it theoretically has the potential to break some existing usecases, where the
distiction between remote ports is relevant.

Signed-off-by: Tobias Guggenmos <tobias.guggenmos@uni-ulm.de>
@debugloop

Copy link
Copy Markdown
Contributor

I was a bit alarmed by this, but the IOS-XR flow export we use does not randomize the source port regularly, may be even only on chassis reload. Still, this seems like a good precaution, I don't really see what you'd use that label for.

@SuperQ SuperQ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, I agree, tracking remote_port is too high a cardinality risk.

@SuperQ

SuperQ commented Apr 3, 2021

Copy link
Copy Markdown
Contributor

Ping @lspgn. 😄 Any chance we can get some reviews and a release?

@lspgn

lspgn commented Apr 3, 2021

Copy link
Copy Markdown
Contributor

Hello,
Thank you for the PR, looks good to me.
But I won't be able to merge yet (see #97).
Thank you for your patience.

tgragnato referenced this pull request in tgragnato/goflow Aug 14, 2024
* Add MPLS decoding for IPFIX
Co-authored-by: David Roy <door7302@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cardinality explosion in flow_traffic metrics

4 participants