Skip to content

Add GroupChangeLog feature gate to fix es indexing cardinality #1361

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

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

94DanielBrown
Copy link
Contributor

Tested in kind

FYI I got error when using flux install to test

[kind-kind|flux-system] ➜  flux-bootstrap git:(main) ✗ k logs kustomize-controller-6d8bf96598-4t595                                                                                                                    [dev/flux-bootstrap]
{"level":"error","ts":"2025-02-12T15:55:13.993Z","logger":"setup","msg":"unable to start manager","error":"failed to determine if *v1.Kustomization is namespaced: failed to get restmapping: unable to retrieve the complete list of server APIs: kustomize.toolkit.fluxcd.io/v1: no matches for kustomize.toolkit.fluxcd.io/v1, Resource="}

But installing with this worked fine as crd was v1 not v1beta2
k apply -f https://github.com/fluxcd/flux2/releases/latest/download/install.yaml

example logs from applying kustomization after

{“level":"info","ts":"2025-02-12T17:20:23.527Z","msg":"server-side apply for cluster definitions completed","controller":"kustomization","controllerGroup":"kustomize.toolkit.fluxcd.io","controllerKind":"Kustomization","Kustomization":{"name":"podinfo","namespace":"flux-system"},"namespace":"flux-system","name":"podinfo","reconcileID":"c78a46e3-5d45-4c01-8cfa-87ed9cf93e3a","output":{"unchanged":["Service/default/podinfo","Deployment/default/podinfo","HorizontalPodAutoscaler/default/podinfo”]}}

main.go Outdated
Comment on lines 112 to 113
flag.BoolVar(&esLogging, "es-logging", false,
"Reduces cardinality in Elasticsearch by not using the kubernetes object as field name in logs")
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this and add a feature flag called GroupedChangelog here: internal/features/features.go, also please use this name in all code instead of EsLogging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated and tested
{"level":"info","ts":"2025-02-13T12:07:31.518Z","logger":"setup","msg":"loading feature gate","GroupChangeLog":true}

@@ -59,6 +63,9 @@ var features = map[string]bool{
// StrictPostBuildSubstitutions
// opt-in from v1.3
StrictPostBuildSubstitutions: false,
// GroupChangeLog
// opt-in from v2.5
Copy link
Member

Choose a reason for hiding this comment

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

This refers to the kustomize-controller version, should be v1.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah sorry, fixed

@stefanprodan stefanprodan changed the title chore: add grouped field logging feature for es indexing cardinality chore: add GroupChangeLog feature gate to fix es indexing cardinality Feb 13, 2025
@stefanprodan
Copy link
Member

@94DanielBrown can you please squash all commits into one, then I can merge. Thanks!

@stefanprodan
Copy link
Member

@94DanielBrown can you please signoff you commit

@stefanprodan
Copy link
Member

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @94DanielBrown 🏅

@stefanprodan
Copy link
Member

@stefanprodan stefanprodan merged commit 947be82 into fluxcd:main Feb 17, 2025
7 checks passed
@matheuscscp matheuscscp changed the title chore: add GroupChangeLog feature gate to fix es indexing cardinality Add GroupChangeLog feature gate to fix es indexing cardinality Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants