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

Add metrics and generic metrics backend #2355

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

hubertdeng123
Copy link
Member

No description provided.

@aldy505
Copy link
Collaborator

aldy505 commented Aug 19, 2023

10 new containers on this PR. As far as I experienced this, each Snuba container consumes around 50-100 MB of RAM, and each Sentry ingest container consumes around 60-130 MB of RAM. I'd assume this feature would require at least additional 1 GB of RAM tops (assuming every each of it consumes 100 MB of RAM). Should we bump the minimum specs on the requirements?

self-hosted/README.md

Lines 5 to 11 in 8c294f5

## Requirements
* Docker 19.03.6+
* Compose 2.0.1+
* 4 CPU Cores
* 8 GB RAM
* 20 GB Free Disk Space

Edit. Forgot to strike this issue: getsentry/snuba#1670

@BYK
Copy link
Member

BYK commented Aug 19, 2023

Or should we finally invest in a unified Snuba instance and unified ingest processes?

@hubertdeng123
Copy link
Member Author

Yep, was pretty painful adding another 10 containers here.

Or should we finally invest in a unified Snuba instance and unified ingest processes?

Let me see what I can do here in the following weeks.

@williamdes
Copy link
Contributor

Or should we finally invest in a unified Snuba instance and unified ingest processes?

That would be cool !
There is already too much containers to look after

And if you want a supervisor alternative, there is https://github.com/FedericoPonzi/Horust that I got into alpine edge testing ;)

@aldy505
Copy link
Collaborator

aldy505 commented Aug 23, 2023

Or should we finally invest in a unified Snuba instance and unified ingest processes?

That would be cool !

There is already too much containers to look after

And if you want a supervisor alternative, there is https://github.com/FedericoPonzi/Horust that I got into alpine edge testing ;)

I don't really think that trimming down the containers to a single container with some kind of supervisor would be different to the consumed resources. As most of us are more likely to have CPU and RAM usage problems instead of storage problems.

Python is a language that requires runtime. If you spawn another app (whether it's on a separate container or same container with supervisor), it will be (most likely) the same, because you'd have to have the runtime consuming some minimum CPU and RAM resource to get it running. I'd argue that it'll be better if we have snuba (and sentry ingest) to have unified consumer/ingester instead of having everything supervised.

@williamdes
Copy link
Contributor

Python is a language that requires runtime.

Yup, but using Horust, well it's Rust ;)
The consumption should be pretty minimal.

I'd argue that it'll be better if we have snuba (and sentry ingest) to have unified consumer/ingester instead of having everything supervised.

It's maybe best not to mix everything, but maybe some tasks could be unified in one container per "topic/section"

@hubertdeng123
Copy link
Member Author

Re: unified snuba/ingest instances

Will have to scope that out, on the radar for Q4:
getsentry/team-ospo#189

@hubertdeng123 hubertdeng123 changed the title add metrics backend Add metrics and generic metrics backend Aug 29, 2023
@hubertdeng123 hubertdeng123 marked this pull request as ready for review August 29, 2023 21:59
# Metrics Backend #
###################

SENTRY_RELEASE_HEALTH = "sentry.release_health.metrics.MetricsReleaseHealthBackend"
Copy link
Member Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

No this is the right place for the setting. This lets sentry know to load sessions metric data from the metrics dataset (which is the change you're trying to make).

Snuba will serve up sessions or metrics and isn't aware of what the product is doing really

Copy link

@onewland onewland left a comment

Choose a reason for hiding this comment

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

This looks right to me, assuming you got it to run locally

# Metrics Backend #
###################

SENTRY_RELEASE_HEALTH = "sentry.release_health.metrics.MetricsReleaseHealthBackend"

Choose a reason for hiding this comment

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

No this is the right place for the setting. This lets sentry know to load sessions metric data from the metrics dataset (which is the change you're trying to make).

Snuba will serve up sessions or metrics and isn't aware of what the product is doing really

@hubertdeng123
Copy link
Member Author

Looks good to me locally
Stuff in clickhouse:
Screenshot 2023-08-30 at 10 19 53 AM

Stuff in postgres:
Screenshot 2023-08-30 at 10 18 56 AM

@@ -271,6 +278,11 @@ def get_internal_network():
"organizations:session-replay",
"organizations:issue-platform",
"organizations:profiling",
"organizations:dashboards-mep",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these orgs designed to test the new snuba containers somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

those are feature flags that are set at the org level, and utilize the new snuba containers

@hubertdeng123 hubertdeng123 merged commit cad1deb into master Aug 30, 2023
8 checks passed
@hubertdeng123 hubertdeng123 deleted the hubertdeng123/add-metrics branch August 30, 2023 23:17
@hubertdeng123 hubertdeng123 mentioned this pull request Sep 5, 2023
13 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Sep 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants