fix(profiling): Ingest profile file path#4060
Conversation
`ingest-profiles` is now using vroomrs to ingest profiles instead of writing through vroom. For self-hosted, we need to make sure filestore for profiles is properly configured so vroom can find the ingested profiles.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4060 +/- ##
=======================================
Coverage 99.49% 99.49%
=======================================
Files 3 3
Lines 197 197
=======================================
Hits 196 196
Misses 1 1 ☔ View full report in Codecov by Sentry. |
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
Looks okay but without any tests, hard to be sure. Also added some comments I'd like to see addressed.
install/bootstrap-s3-profiles.sh
Outdated
| # 3. Point `filestore-profiles` and vroom to the SeaweedFS "profiles" bucket. | ||
|
|
||
| start_service_and_wait_ready seaweedfs | ||
| $dc exec -e "HTTP_PROXY=${HTTP_PROXY:-}" -e "HTTPS_PROXY=${HTTPS_PROXY:-}" -e "NO_PROXY=${NO_PROXY:-}" -e "http_proxy=${http_proxy:-}" -e "https_proxy=${https_proxy:-}" -e "no_proxy=${no_proxy:-}" seaweedfs apk add --no-cache s3cmd |
There was a problem hiding this comment.
@aldy505 we should have a shortcut for this monstrosity:
$dc exec -e "HTTP_PROXY=${HTTP_PROXY:-}" -e "HTTPS_PROXY=${HTTPS_PROXY:-}" -e "NO_PROXY=${NO_PROXY:-}" -e "http_proxy=${http_proxy:-}" -e "https_proxy=${https_proxy:-}" -e "no_proxy=${no_proxy:-}"
There was a problem hiding this comment.
Fun fact, there's a little problem. Surprisingly seaweedfs only reads the lowercased http_proxy :)
curses at myself
install/bootstrap-s3-profiles.sh
Outdated
| # 3. Point `filestore-profiles` and vroom to the SeaweedFS "profiles" bucket. | ||
|
|
||
| start_service_and_wait_ready seaweedfs | ||
| $dc exec -e "HTTP_PROXY=${HTTP_PROXY:-}" -e "HTTPS_PROXY=${HTTPS_PROXY:-}" -e "NO_PROXY=${NO_PROXY:-}" -e "http_proxy=${http_proxy:-}" -e "https_proxy=${https_proxy:-}" -e "no_proxy=${no_proxy:-}" seaweedfs apk add --no-cache s3cmd |
There was a problem hiding this comment.
Fun fact, there's a little problem. Surprisingly seaweedfs only reads the lowercased http_proxy :)
curses at myself
Co-authored-by: Burak Yigit Kaya <byk@sentry.io>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
aldy505
force-pushed
the
txiao/fix/ingest-profile-file-path
branch
from
3cc8e38 to
e61c180
Compare
| EOF | ||
| ) | ||
|
|
||
| $dc exec seaweedfs sh -c "printf '%s' '$lifecycle_policy' > /tmp/profiles-lifecycle-policy.xml" |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@BYK Can you review this before UK lunch time today? It should be around 20:00 my time. I will merge by then if you have no comments. |
| profiles_config=$(sed -n '/filestore.profiles-backend/,/s3v4"/{p}' sentry/config.example.yml) | ||
| echo "$profiles_config" >>$SENTRY_CONFIG_YML | ||
| echo | ||
| echo "To avoid this prompt in the future, use one of these flags:" |
There was a problem hiding this comment.
This config stuff should use common helpers to avoid repeating this pattern everywhere
|
|
||
| bucket_list=$($s3cmd --access_key=sentry --secret_key=sentry --no-ssl --region=us-east-1 --host=localhost:8333 --host-bucket='localhost:8333/%(bucket)' ls) | ||
| start_service_and_wait_ready seaweedfs | ||
| $dc exec -e "HTTP_PROXY=${HTTP_PROXY:-}" -e "HTTPS_PROXY=${HTTPS_PROXY:-}" -e "NO_PROXY=${NO_PROXY:-}" -e "http_proxy=${http_proxy:-}" -e "https_proxy=${https_proxy:-}" -e "no_proxy=${no_proxy:-}" seaweedfs apk add --no-cache s3cmd |
There was a problem hiding this comment.
This should use dc_proxy_args for podman compatibility?
There was a problem hiding this comment.
I didn't know that exists!
| # Use a temporary container to copy files from the volume to SeaweedFS | ||
|
|
||
| echo "Migration completed." | ||
| $dc exec -e "HTTP_PROXY=${HTTP_PROXY:-}" -e "HTTPS_PROXY=${HTTPS_PROXY:-}" -e "NO_PROXY=${NO_PROXY:-}" -e "http_proxy=${http_proxy:-}" -e "https_proxy=${https_proxy:-}" -e "no_proxy=${no_proxy:-}" -u root vroom sh -c 'mkdir -p /var/lib/apt/lists/partial && apt-get update && apt-get install -y --no-install-recommends s3cmd' |
There was a problem hiding this comment.
Let's ask Copilot to do it after this? :D
There was a problem hiding this comment.
Works for me. Or you can wait a bit until that patch is finished
| SENTRY_KAFKA_BROKERS_PROFILING: "kafka:9092" | ||
| SENTRY_KAFKA_BROKERS_OCCURRENCES: "kafka:9092" | ||
| SENTRY_BUCKET_PROFILES: "s3://profiles?region=us-east-1&endpoint=seaweedfs:8333&s3ForcePathStyle=true&disableSSL=true" | ||
| AWS_ACCESS_KEY: "sentry" |
There was a problem hiding this comment.
Should these be global en vars to avoid drift?
There was a problem hiding this comment.
No, I don't think so. From the gist that I understand, we're retiring vroom right?
There was a problem hiding this comment.
Why do I know more stuff then 😂
There was a problem hiding this comment.
vroom is no longer used for ingestion for is still needed to power the profiling feature in the UI
|
Please check logs when re-run script install.sh |
|
@nikita-devops This PR does not touch |
|
yes, from commit |
5 participants
ingest-profilesis now using vroomrs to ingest profiles instead of writing through vroom. For self-hosted, we need to make sure filestore for profiles is properly configured so vroom can find the ingested profiles.Fixes the issue in #4012 (comment)