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

Remove reporting of unqueried metrics. #4076

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

sarahjmiller
Copy link
Contributor

Note: Samson is a public repo, do not include Zendesk-internal information, urls, etc.

The metrics cost optimization project requested that we remove the process stats being reported by Samson, which are not queried by any existing monitors. I've commented it out so that it will be easy to reinstate if we do need to monitor the process stats.

References

Risks

  • Low: only removing stats reporting that aren't being monitored.

@sarahjmiller sarahjmiller requested a review from a team October 31, 2023 17:18
Copy link
Contributor

@grosser grosser left a comment

Choose a reason for hiding this comment

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

  • why is this even an issue when these have ~10 total series ?
  • we should monitor periodicals taking too long because that means they are doing something funky (either consuming too many resources or stuck in a loop)

@sarahjmiller
Copy link
Contributor Author

Thanks for the feedback, @grosser!

why is this even an issue when these have ~10 total series ?

In the metrics cost optimization request they mention both an improvement to ingest cost and cardinality cost, so even a small number of series might make a difference?

we should monitor periodicals taking too long because that means they are doing something funky (either consuming too many resources or stuck in a loop)

The request also says that these metrics aren't queried by any existing dashboards, monitors, or users. If Compute is monitoring these metrics, would you be willing to respond in the Slack thread with the rationale for keeping them?

@grosser
Copy link
Contributor

grosser commented Nov 1, 2023

ah nvm I read the code wrong ... this is just process list
that would have big cardinality and be not generally useful 🎉

@sarahjmiller sarahjmiller merged commit faef15f into master Nov 2, 2023
10 checks passed
@sarahjmiller sarahjmiller deleted the DPLY-3582/remove-unqueried-metrics branch November 2, 2023 21:16
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.

2 participants