Skip to content

feat: expire stale advanced metrics after not being updated for some time - #1921

Merged
Quang Nguyen (nddq) merged 1 commit into
microsoft:mainfrom
mmckeen:removeStaleMetrics
Feb 17, 2026
Merged

feat: expire stale advanced metrics after not being updated for some time#1921
Quang Nguyen (nddq) merged 1 commit into
microsoft:mainfrom
mmckeen:removeStaleMetrics

Conversation

@mmckeen

@mmckeen Matthew McKeen (mmckeen) commented Oct 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

With advanced metrics, high-cardinality labels can cause the metrics export to bloat leading to unbounded memory and resource usage.

This PR sets up an optional TTL for advanced metrics defined in the MetricsConfiguration CRD. By default, the TTL is infinite and cleanup is not tracked or done.

When defined, on a period equal to the TTL metrics which have not been updated within the last TTL duration will be removed from the metrics export.

For counters and gauges that look like counters, this will be treated by Prometheus similar to any other missing metric (for example from an application restart). As long as functions like rate or increase are used, calculations will remain accurate.

This also includes a refactor of the base metrics object as an interface, allowing mocking in tests and a clean API contract with child objects that compose it.

Related Issue

#1692

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Deployed, modified CRD to enable/disable and change the TTL.

Metrics are re-initialized as expected.

Made sure that CRD validation rejects invalid TTL values.

Also, added comprehensive tests for the TTL-based cleanup logic.

Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@mmckeen
Matthew McKeen (mmckeen) requested a review from a team as a code owner October 28, 2025 19:12
@github-actions

github-actions Bot commented Nov 29, 2025

Copy link
Copy Markdown

This PR will be closed in 7 days due to inactivity.

@github-actions github-actions Bot added the meta/waiting-for-author Blocked and waiting on the author label Nov 29, 2025
@mmckeen

Matthew McKeen (mmckeen) commented Dec 2, 2025

Copy link
Copy Markdown
Contributor Author

Quang Nguyen (@nddq) this isn't waiting on me, any way to prevent it from being auto-closed?

@nddq Quang Nguyen (nddq) removed the meta/waiting-for-author Blocked and waiting on the author label Dec 2, 2025
@nddq

Quang Nguyen (nddq) commented Dec 2, 2025

Copy link
Copy Markdown
Member

Matthew McKeen (@mmckeen) removed the stale tag

@mmckeen Matthew McKeen (mmckeen) mentioned this pull request Dec 16, 2025
Closed
@github-actions

github-actions Bot commented Jan 2, 2026

Copy link
Copy Markdown

This PR will be closed in 7 days due to inactivity.

@github-actions github-actions Bot added the meta/waiting-for-author Blocked and waiting on the author label Jan 2, 2026
@nddq Quang Nguyen (nddq) removed the meta/waiting-for-author Blocked and waiting on the author label Jan 8, 2026
nddq
Quang Nguyen (nddq) requested changes Jan 28, 2026
View reviewed changes

@nddq Quang Nguyen (nddq) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for taking a while to respond 😅 Gave this some thoughts

Comment thread pkg/module/metrics/basemetricsobject.go
Comment thread pkg/module/metrics/basemetricsobject.go Outdated
Comment thread pkg/module/metrics/basemetricsobject.go Outdated
@mmckeen
Matthew McKeen (mmckeen) force-pushed the removeStaleMetrics branch 4 times, most recently from 2abfcbe to 8c5bf85 Compare February 3, 2026 17:28
@mmckeen
Matthew McKeen (mmckeen) marked this pull request as draft February 6, 2026 01:24
@mmckeen

Matthew McKeen (mmckeen) commented Feb 6, 2026

Copy link
Copy Markdown
Contributor Author

I'm working on refactoring this a bit to be able to add some better testing for the expiration case.

@mmckeen
Matthew McKeen (mmckeen) marked this pull request as ready for review February 10, 2026 19:56
@mmckeen

Matthew McKeen (mmckeen) commented Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

Quang Nguyen (@nddq) this includes a bunch of test coverage (including making sure that the goroutine doesn't leak) as well as a refactor of the base metrics object in order to support that testing.

@mmckeen

Matthew McKeen (mmckeen) commented Feb 13, 2026

Copy link
Copy Markdown
Contributor Author

Quang Nguyen (@nddq) appreciate a review on this when you get the chance :)

It's the last PR away from us being able to run the upstream version of Retina rather than our fork.

nddq
Quang Nguyen (nddq) reviewed Feb 14, 2026
View reviewed changes

@nddq Quang Nguyen (nddq) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good to me, managed to test the feature in my local cluster and verified that it is working correctly! Just a few nit comments and also the markdown linter is failing (although we might consider disabling it in the future or make it less strict)

Comment thread pkg/module/metrics/basemetricsobject.go Outdated
Comment thread pkg/module/metrics/basemetricsobject.go Outdated
Comment thread crd/api/v1alpha1/validations/validate_metricconfiguration_test.go
Comment thread pkg/module/metrics/dns_test.go Outdated
Comment thread pkg/module/metrics/drops_test.go Outdated
@nddq Quang Nguyen (nddq) linked an issue Feb 14, 2026 that may be closed by this pull request
Expire stale exported metrics #1692
Closed
@mmckeen
feat: expire stale advanced metrics after not being updated for some …
444fb85 
…time

Signed-off-by: Matthew McKeen <matthew.mckeen@fastly.com>
@mmckeen

Matthew McKeen (mmckeen) commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

Overall looks good to me, managed to test the feature in my local cluster and verified that it is working correctly! Just a few nit comments and also the markdown linter is failing (although we might consider disabling it in the future or make it less strict)

Fixed the nits, looks like the Markdown linter is failing on main though.

nddq
Quang Nguyen (nddq) approved these changes Feb 17, 2026
View reviewed changes

@nddq Quang Nguyen (nddq) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm, thanks! Just realized that markdown lint is not required to merge PRs

@nddq
Quang Nguyen (nddq) added this pull request to the merge queue Feb 17, 2026
Merged via the queue into microsoft:main with commit adc6406 Feb 17, 2026
32 of 33 checks passed
Laksh (lakshk98) pushed a commit to lakshk98/retina that referenced this pull request Aug 4, 2026
@mmckeen @lakshk98
feat: expire stale advanced metrics after not being updated for some …
28e3303 
…time (microsoft#1921)

# Description

With advanced metrics, high-cardinality labels can cause the metrics
export to bloat leading to unbounded memory and resource usage.

This PR sets up an optional TTL for advanced metrics defined in the
`MetricsConfiguration` CRD. By default, the TTL is infinite and cleanup
is not tracked or done.

When defined, on a period equal to the TTL metrics which have not been
updated within the last TTL duration will be removed from the metrics
export.

For counters and gauges that look like counters, this will be treated by
Prometheus similar to any other missing metric (for example from an
application restart). As long as functions like `rate` or `increase` are
used, calculations will remain accurate.

This also includes a refactor of the base metrics object as an
interface, allowing mocking in tests and a clean API contract with child
objects that compose it.

## Related Issue

microsoft#1692

## Checklist

- [X] I have read the [contributing
documentation](https://retina.sh/docs/Contributing/overview).
- [X] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [X] I have correctly attributed the author(s) of the code.
- [X] I have tested the changes locally.
- [X] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Deployed, modified CRD to enable/disable and change the TTL.

Metrics are re-initialized as expected.

Made sure that CRD validation rejects invalid TTL values.

Also, added comprehensive tests for the TTL-based cleanup logic.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

Signed-off-by: Matthew McKeen <matthew.mckeen@fastly.com>
Laksh (lakshk98) pushed a commit to lakshk98/retina that referenced this pull request Aug 4, 2026
@mmckeen @lakshk98
feat: expire stale advanced metrics after not being updated for some …
3cafb4b 
…time (microsoft#1921)

# Description

With advanced metrics, high-cardinality labels can cause the metrics
export to bloat leading to unbounded memory and resource usage.

This PR sets up an optional TTL for advanced metrics defined in the
`MetricsConfiguration` CRD. By default, the TTL is infinite and cleanup
is not tracked or done.

When defined, on a period equal to the TTL metrics which have not been
updated within the last TTL duration will be removed from the metrics
export.

For counters and gauges that look like counters, this will be treated by
Prometheus similar to any other missing metric (for example from an
application restart). As long as functions like `rate` or `increase` are
used, calculations will remain accurate.

This also includes a refactor of the base metrics object as an
interface, allowing mocking in tests and a clean API contract with child
objects that compose it.

## Related Issue

microsoft#1692

## Checklist

- [X] I have read the [contributing
documentation](https://retina.sh/docs/Contributing/overview).
- [X] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [X] I have correctly attributed the author(s) of the code.
- [X] I have tested the changes locally.
- [X] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Deployed, modified CRD to enable/disable and change the TTL.

Metrics are re-initialized as expected.

Made sure that CRD validation rejects invalid TTL values.

Also, added comprehensive tests for the TTL-based cleanup logic.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

Signed-off-by: Matthew McKeen <matthew.mckeen@fastly.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nddq Quang Nguyen (nddq) nddq approved these changes
@ibezrukavyi ibezrukavyi Awaiting requested review from ibezrukavyi ibezrukavyi is a code owner automatically assigned from microsoft/retina
@MikeZappa87 Mike Zappa (MikeZappa87) Awaiting requested review from MikeZappa87 MikeZappa87 is a code owner automatically assigned from microsoft/retina

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Expire stale exported metrics

2 participants

@mmckeen @nddq