feat(profiling): Run profiling on self-hosted #2154

phacops · 2023-05-17T13:40:58Z

This PR aims to add support for profiling on self-hosted.

aldy505

I don't see any configuration for integrating it with the web or snuba containers? Or is it meant to be just like this? I'd like to try it on my own server 😄

docker-compose.yml

Co-authored-by: Reinaldy Rafli <aldy505@proton.me>

phacops · 2023-05-17T14:28:02Z

@aldy505 Good point, I added those.

phacops · 2023-05-17T14:36:07Z

We might need to wait for getsentry/snuba#4195.

docker-compose.yml

aldy505

Actually I'm a bit curious about the SENTRY_DSN environment variable that can be applied to snuba and vroom container. If they're left empty, I suppose no error message will be sent to the internal project of people's self-hosted instance.

Vroom's defined here: https://github.com/getsentry/vroom/blob/c2cf9653f5bb9b2e4a02f0548956b790547e9048/cmd/vroom/config.go#L8

aldy505 · 2023-05-19T01:11:10Z

docker-compose.yml

+  snuba-profiling-profiles-consumer:
+    <<: *snuba_defaults
+    command: consumer --storage profiles --auto-offset-reset=latest --max-batch-time-ms 1000 --no-strict-offset-reset
+  snuba-profiling-functions-consumer:
+    <<: *snuba_defaults
+    command: consumer --storage functions --auto-offset-reset=latest --max-batch-time-ms 1000 --no-strict-offset-reset


Should these be added to the list of sentry image's depends_on field?

Here:

self-hosted/docker-compose.yml

Lines 39 to 50 in 8aa571c

snuba-consumer:

<<: *depends_on-default

snuba-outcomes-consumer:

<<: *depends_on-default

snuba-sessions-consumer:

<<: *depends_on-default

snuba-transactions-consumer:

<<: *depends_on-default

snuba-subscription-consumer-events:

<<: *depends_on-default

snuba-subscription-consumer-transactions:

<<: *depends_on-default

I added vroom as a depends_on since web needs to POST to vroom, but then the consumers only depends on kafka technically. I think it's good like this.

docker-compose.yml

hubertdeng123

Will profiling be released under CalVer? If so, would need to add VROOM to

self-hosted/scripts/bump-version.sh

Line 10 in 1fa7734

    
           sed -i -e "s/^\(SENTRY\|SNUBA\|RELAY\|SYMBOLICATOR\)_IMAGE=\([^:]\+\):.\+\$/\1_IMAGE=\2:$NEW_VERSION/" .env

docker-compose.yml

phacops · 2023-05-23T17:12:56Z

Actually I'm a bit curious about the SENTRY_DSN environment variable that can be applied to snuba and vroom container. If they're left empty, I suppose no error message will be sent to the internal project of people's self-hosted instance.

That's correct, the Sentry SDK won't be activated and nothing will be sent. I was expecting the user to pass the right DSN via an environment variable. Do you know how other services are handling that?

aldy505 · 2023-05-23T22:48:51Z

That's correct, the Sentry SDK won't be activated and nothing will be sent. I was expecting the user to pass the right DSN via an environment variable. Do you know how other services are handling that?

We might as well need to ping @hubertdeng123 for help.

There is an "internal" project that should've been set up during the first install of Sentry, but I don't know on how to acquire the DSN and put it into the environment variable / configuration that can be consumed by Snuba and Vroom.

hubertdeng123 · 2023-05-23T23:06:56Z

There is an "internal" project that should've been set up during the first install of Sentry, but I don't know on how to acquire the DSN and put it into the environment variable / configuration that can be consumed by Snuba and Vroom.

Snuba is very similar, retrieves SENTRY_DSN from the environment. I've never played around with it before but it should just be possible to put it into the configuration through the docker-compose.yml file. I don't think this is a large concern for this PR though. For the record, looks like a DSN for the internal project can be shown by going to /settings/${org}/projects/internal/keys/

aldy505 · 2023-05-23T23:11:16Z

Snuba is very similar, retrieves SENTRY_DSN from the environment. I've never played around with it before but it should just be possible to put it into the configuration through the docker-compose.yml file. I don't think this is a large concern for this PR though. For the record, looks like a DSN for the internal project can be shown by going to /settings/${org}/projects/internal/keys/

I guess we can skip that one for now?

hubertdeng123 · 2023-05-23T23:13:23Z

I guess we can skip that one for now?

Yep

fixtures/envelope-with-profile.json

hubertdeng123 · 2023-05-26T15:40:49Z

_unit-test/create-docker-volumes-test.sh

 sentry-zookeeper"

 before=$(get_volumes)

 test "$before" == "" || test "$before" == "$expected_volumes"

-source install/create-docker-volumes.sh


got it, safe to remove then

fixtures/envelope-with-profile.json

…lines

BYK

🚀

_integration-test/run.sh

docker-compose.yml

chadwhitacre

Let's hold off on merging until we cut 23.5.2 (related to inc-389).

hubertdeng123 · 2023-05-31T20:52:43Z

23.5.2 is out now, this can be merged now

aldy505

Just a little bit more..

Note about the exposed port: if you don't specify it as exposed port, that port will still be opened to other containers. Having a new exposed ports might cause a vulnerability for people who didn't set up a proper OS firewall.

docker-compose.yml

…each the service Co-authored-by: Reinaldy Rafli <aldy505@proton.me>

aldy505

LGTM!

…are persisted

feat(profiling): Run profiling on self-hosted

17ac35b

phacops mentioned this pull request May 17, 2023

Add Profiling #1838

Closed

5 tasks

phacops requested a review from hubertdeng123 May 17, 2023 13:46

aldy505 reviewed May 17, 2023

View reviewed changes

docker-compose.yml Outdated Show resolved Hide resolved

phacops and others added 3 commits May 17, 2023 10:15

Run profiles and functions Snuba consumers

d919885

Fix typo in variable name

7f82276

Co-authored-by: Reinaldy Rafli <aldy505@proton.me>

Run profiles consumer

0cc8b66

Enable profiling in the UI

e01c431

Add an external volume to store profiles

d608f0c

aldy505 reviewed May 18, 2023

View reviewed changes

docker-compose.yml Outdated Show resolved Hide resolved

phacops added 2 commits May 18, 2023 11:14

Create sentry-vroom volume

a006a3a

Add missing Kafka broker environment variables

8aa571c

phacops requested review from aldy505 and a team and removed request for aldy505 May 18, 2023 15:15

aldy505 reviewed May 19, 2023

View reviewed changes

hubertdeng123 reviewed May 22, 2023

View reviewed changes

docker-compose.yml Outdated Show resolved Hide resolved

docker-compose.yml Outdated Show resolved Hide resolved

phacops added 3 commits May 23, 2023 12:57

Bump vroom image version on new release

c47f62f

Move ingest-profiles right below the rest of the consumers

4564d44

Hard code Snuba host variable

36daec3

enochtangg mentioned this pull request May 24, 2023

feat(profiling): Promote profiles and functions to complete getsentry/snuba#4238

Merged

phacops and others added 2 commits May 24, 2023 17:45

Fix storage name for functions

d079095

update create docker volumes unit test

90dffe1

BYK reviewed May 26, 2023

View reviewed changes

fixtures/envelope-with-profile.json Outdated Show resolved Hide resolved

hubertdeng123 reviewed May 26, 2023

View reviewed changes

phacops added 5 commits May 30, 2023 13:12

Remove local DSN and rename file since it's not valid JSON

6ffa0a5

Adjust curl command to send data from file as binary to preserve new …

f4e4c0a

…lines

Make the test profile thinner

9861b63

Get the profile ID from the test profile file

419cdc1

Make vroom volume not external

d5997b9

BYK approved these changes May 31, 2023

View reviewed changes

_integration-test/run.sh Outdated Show resolved Hide resolved

docker-compose.yml Show resolved Hide resolved

phacops added 4 commits May 31, 2023 10:02

Use a common path for curl and jq call and fix content type

58838d7

Move environment variable to a vroom-specific location

9e28cef

Add vroom as a default service

a540847

Set the proper environment variable for vroom service in web

40198e9

phacops requested review from hubertdeng123, aldy505 and chadwhitacre May 31, 2023 14:28

hubertdeng123 approved these changes May 31, 2023

View reviewed changes

chadwhitacre requested changes May 31, 2023

View reviewed changes

chadwhitacre self-requested a review May 31, 2023 20:56

chadwhitacre approved these changes May 31, 2023

View reviewed changes

aldy505 reviewed May 31, 2023

View reviewed changes

docker-compose.yml Outdated Show resolved Hide resolved

docker-compose.yml Show resolved Hide resolved

docker-compose.yml Show resolved Hide resolved

Remove exposing the port as it's not needed for other containers to r…

a49a5ed

…each the service Co-authored-by: Reinaldy Rafli <aldy505@proton.me>

aldy505 reviewed Jun 2, 2023

View reviewed changes

Add comments to make it explicit what those volumes are and how they …

a701258

…are persisted

phacops enabled auto-merge (squash) June 2, 2023 13:44

phacops merged commit e07be38 into master Jun 2, 2023
8 checks passed

phacops deleted the pierre/profiling-on-self-hosted branch June 2, 2023 13:58

aldy505 mentioned this pull request Jun 3, 2023

Create a cleanup job based on specified retention days getsentry/vroom#268

Closed

artemisart mentioned this pull request Jun 26, 2023

Support for profiling in Sentry 23.6.0 sentry-kubernetes/charts#917

Closed

github-actions bot locked and limited conversation to collaborators Jul 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(profiling): Run profiling on self-hosted #2154

feat(profiling): Run profiling on self-hosted #2154

phacops commented May 17, 2023 •

edited

Loading

aldy505 left a comment

phacops commented May 17, 2023

phacops commented May 17, 2023

aldy505 left a comment

aldy505 May 19, 2023

phacops May 31, 2023

hubertdeng123 left a comment

phacops commented May 23, 2023

aldy505 commented May 23, 2023

hubertdeng123 commented May 23, 2023

aldy505 commented May 23, 2023 •

edited

Loading

hubertdeng123 commented May 23, 2023

hubertdeng123 May 26, 2023

BYK left a comment

chadwhitacre left a comment

hubertdeng123 commented May 31, 2023

aldy505 left a comment

aldy505 left a comment

	snuba-consumer:
	<<: *depends_on-default
	snuba-outcomes-consumer:
	<<: *depends_on-default
	snuba-sessions-consumer:
	<<: *depends_on-default
	snuba-transactions-consumer:
	<<: *depends_on-default
	snuba-subscription-consumer-events:
	<<: *depends_on-default
	snuba-subscription-consumer-transactions:
	<<: *depends_on-default

feat(profiling): Run profiling on self-hosted #2154

feat(profiling): Run profiling on self-hosted #2154

Conversation

phacops commented May 17, 2023 • edited Loading

aldy505 left a comment

Choose a reason for hiding this comment

phacops commented May 17, 2023

phacops commented May 17, 2023

aldy505 left a comment

Choose a reason for hiding this comment

aldy505 May 19, 2023

Choose a reason for hiding this comment

phacops May 31, 2023

Choose a reason for hiding this comment

hubertdeng123 left a comment

Choose a reason for hiding this comment

phacops commented May 23, 2023

aldy505 commented May 23, 2023

hubertdeng123 commented May 23, 2023

aldy505 commented May 23, 2023 • edited Loading

hubertdeng123 commented May 23, 2023

hubertdeng123 May 26, 2023

Choose a reason for hiding this comment

BYK left a comment

Choose a reason for hiding this comment

chadwhitacre left a comment

Choose a reason for hiding this comment

hubertdeng123 commented May 31, 2023

aldy505 left a comment

Choose a reason for hiding this comment

aldy505 left a comment

Choose a reason for hiding this comment

phacops commented May 17, 2023 •

edited

Loading

aldy505 commented May 23, 2023 •

edited

Loading