Skip to content

Conversation

rexagod
Copy link
Contributor

@rexagod rexagod commented Jul 6, 2025

Signed-off-by: Pranshu Srivastava rexagod@gmail.com

Description

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.

KSM v2.16 introduced look around support for regexes (kubernetes/kube-state-metrics#2616), and since namespaces are an extremely common native resource, it might make sense to have their creation metrics OOTB for downstream consumers.

Type of change

What type of changes does your code introduce to the kube-prometheus? Put an x in the box that apply.

  • CHANGE (fix or feature that would cause existing functionality to not work as expected)
  • FEATURE (non-breaking change which adds functionality)
  • BUGFIX (non-breaking change which fixes an issue)
  • ENHANCEMENT (non-breaking change which improves existing functionality)
  • NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Please put a one-line changelog entry below. Later this will be copied to the changelog file.

Replaces the `^kube_.+_created$` in KSM's metrics deny-list with `^kube_(?=namespace).*_created$` to allow generating namespace creation metrics.

generate-versions.sh seems to return null for KSM, but I can update versions.json if needed.

@rexagod rexagod requested a review from a team as a code owner July 6, 2025 23:14
@rexagod
Copy link
Contributor Author

rexagod commented Jul 6, 2025

Do we maintain any stability guarantees (or backward-compatibilities) for the deny-list? I've assumed that the deny-list, with adequate reason, is subject to change based on observations seen over prolonged periods, but I'm happy to do this in a safer manner (say, in a separate ksm-lite-v216, for example) to maintain stability.

@rexagod
Copy link
Contributor Author

rexagod commented Jul 6, 2025

If this still seems odd, I'll try to put together a more makeshift solution downstream and close this out.

@simonpasquier
Copy link
Contributor

I suspect that cluster-monitoring-operator is the only downstream user for his addon. I'm not sure that it's worth keeping it in this repository given that it's not documented and it provides no configuration option. If we want to keep it here, I'd advocate for 1) adding arguments to customize the deny list and 2) minimal documentation.

WDYT @slashpai @philipgough?

@philipgough
Copy link
Contributor

@simonpasquier yep, agree that makes sense

containers,
);

local defaultDenylist = [
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use the "standard" values pattern?

{
    values+:: {
        ksmDenyList: [
           ...
        ],
    },

    kubeStateMetrics+: {
        ....
    }
}
```

@rexagod rexagod force-pushed the allow-namespaces branch from b222bf7 to 787bc20 Compare July 29, 2025 19:46
@rexagod rexagod changed the title change: don't deny namespaces creation metrics change: relax kube_.+_created to allow namespaces Jul 29, 2025
@rexagod rexagod force-pushed the allow-namespaces branch 3 times, most recently from 185a97b to 150d07a Compare July 29, 2025 20:13
@rexagod
Copy link
Contributor Author

rexagod commented Jul 29, 2025

Not sure why the test expects kubePrometheus to be present in the resulting object?

@rexagod
Copy link
Contributor Author

rexagod commented Aug 13, 2025

Tests seem to pass on my fork: https://github.com/rexagod/kube-prometheus/actions/runs/16932879487 for the same commit.

Triggering a retest.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
@rexagod
Copy link
Contributor Author

rexagod commented Aug 18, 2025

cc @simonpasquier for a review 🙇

@rexagod
Copy link
Contributor Author

rexagod commented Aug 20, 2025

Ping @simonpasquier for a look here 🙂

@simonpasquier
Copy link
Contributor

Thanks! #2684 (comment) is still valid.

@rexagod
Copy link
Contributor Author

rexagod commented Aug 31, 2025

So just to make sure I'm understanding this correctly, do you mean instead of ksmConfig this should use values (from the example above) so it can be integrated into the values-based model used in CMO?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants