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.

Sign up for GitHub

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

Jump to bottom

feat(relay): Add Relay to onpremise installation #421

Merged
merged 32 commits into from Apr 24, 2020
Merged

feat(relay): Add Relay to onpremise installation #421

merged 32 commits into from Apr 24, 2020

Conversation

RaduW
Copy link
Contributor

@RaduW RaduW commented Mar 30, 2020

This PR adds the necessary services necessary to have Sentry working with Relay message ingestion.

@RaduW RaduW requested review from BYK and removed request for BYK March 30, 2020 10:23
BYK
BYK suggested changes Mar 30, 2020
View reviewed changes
Copy link
Collaborator

@BYK BYK 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 pretty great! I have some comments which are not major things but they are still important to get before we can merge so requesting changes.

I also thing we should add the Snuba outcomes consumer here for TSDB. @jan-auer @untitaker can you comment on that?

docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
relay_config/config.yml Outdated Show resolved Hide resolved
relay_config/config.yml Outdated Show resolved Hide resolved
reverse_proxy_config/nginx.conf Outdated Show resolved Hide resolved
reverse_proxy_config/nginx.conf Outdated Show resolved Hide resolved
reverse_proxy_config/nginx.conf Outdated Show resolved Hide resolved
reverse_proxy_config/nginx.conf Outdated Show resolved Hide resolved
reverse_proxy_config/nginx.conf Outdated Show resolved Hide resolved
@jan-auer jan-auer changed the title feat(add-relay) Add Relay to onpremise installation feat(relay): Add Relay to onpremise installation Apr 2, 2020
RaduW and others added 21 commits April 3, 2020 13:19
@RaduW
Changes from code review, still needs work on authentication informat…
8cc2621 
…ion generation
@RaduW
generate Relay credentials at install time
7c630fa
@RaduW
generate Relay credentials at install time
2c7052f
@RaduW
increase travis timeout until the web service responds
5a1248f
@RaduW
update web check to the login url
c239ece
@RaduW
set user when generating credentials (to fix linux installations)
a8dd78d
@RaduW
fix sed command line to work both on Mac & Linux
e1b9047
@RaduW
WIP - back to login url
a80c0be
@RaduW
WIP
f607bf6
@RaduW
WIP - dump container logs in travis
391fd18
@RaduW
WIP
4182534
@RaduW
WIP
51ba14a
@RaduW
WIP
06296ae
@RaduW
WIP
61d9479
@RaduW
WIP
671bdb4
@untitaker
pairing with radu: move back to binds, fix perms problems
e232d18
@RaduW
make the credential file readable by everybody
f011054 
 so that user 1001 (the user Relay normally runs under) can access it.
@RaduW
fix small problems, add more messages
ac0578d
@RaduW
more messages to verify everything is ok
dd947ca
@RaduW
and more messages to verify everything is ok
a566d5b
@RaduW
minor cleanup
c897937
@RaduW RaduW requested a review from BYK April 8, 2020 16:08
@jan-auer
Copy link
Member

Please see changes in getsentry/sentry#18390:

  • There's a new envelope endpoint (introduced in feat: Implement an explicit envelope endpoint relay#552) that needs to be forwarded.
  • We need to introduce a size limit of at least 32m for the chunk-upload endpoint to work.
  • We should probably reconsider size limits of other endpoints, or just generally increase it to some higher value to get those out of the way.

If you'd like me to, I can make the changes to this PR.

@jan-auer
Copy link
Member

Another option would be to make a wildcard forward like in getsentry/sentry#18433. By definition, all of these endpoints need to be handled by Relay, so we can as well forward them. The benefit of this is that Relay responds with 404 on non-existing endpoints, while Sentry incorrectly responds with 403.

BYK
BYK reviewed Apr 23, 2020
View reviewed changes
nginx/nginx.conf Outdated Show resolved Hide resolved
RaduW and others added 2 commits April 24, 2020 11:44
@RaduW @BYK
Update install.sh
7049363 
output errors to stderr instead of stdout

Co-Authored-By: Burak Yigit Kaya <byk@sentry.io>
@RaduW @BYK
Update nginx/nginx.conf
b2be9fa 
Set max upload size limit to support envelope chunks

Co-Authored-By: Burak Yigit Kaya <byk@sentry.io>
BYK
BYK suggested changes Apr 24, 2020
View reviewed changes
Copy link
Collaborator

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Will accept when all unrelated debug changes are removed.

BYK
BYK approved these changes Apr 24, 2020
View reviewed changes
Copy link
Collaborator

@BYK BYK left a comment

Choose a reason for hiding this comment

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

🎉

@BYK BYK merged commit e97da7c into master Apr 24, 2020
@BYK BYK deleted the feat/add-relay branch April 24, 2020 12:32
@jan-auer
Copy link
Member

❤️ 🎉 Thanks both!

mlaradji pushed a commit to mlaradji/onpremise that referenced this pull request Apr 25, 2020
@RaduW @BYK @mlaradji
feat(relay): Add Relay to onpremise installation (getsentry#421)
d1baea1 
Co-Authored-By: Burak Yigit Kaya <byk@sentry.io>
@giggsey giggsey mentioned this pull request Apr 25, 2020
Closed
giggsey added a commit to giggsey/onpremise that referenced this pull request Apr 25, 2020
@giggsey
Use restart-policy for nginx & relay
e896a8d 
All the other services have a restart-policy, apart from these new ones added in getsentry#421
@giggsey giggsey mentioned this pull request Apr 25, 2020
Merged
@kagansari
Copy link

You may want to review this. I got permisson denied error on /etc/relay during installation

@jan-auer
Copy link
Member

@BYK Seems like we missed this since there is no cloudbuild check configured. Is there a way to have a GH check on both getsentry/sentry and getsentry/onprem?

@BYK
Copy link
Collaborator

BYK commented Apr 27, 2020

@kagansari yeah, sorry for the disruption we are aware there are issues under certain circumstances. We relied on our local environment and then Travis CI for this which both passed. Should be resolved today.

BYK added a commit that referenced this pull request Apr 27, 2020
@BYK
fix(relay): Fix failed to write credentials
7e9cd08 
Fixes the issue reported on #421 (comment) and https://forum.sentry.io/t/could-not-write-etc-relay-credentials-json/9558?u=byk
@BYK BYK mentioned this pull request Apr 27, 2020
Merged
@guillkara guillkara mentioned this pull request May 15, 2020
Closed
MaicolBen pushed a commit to hinthealth/onpremise that referenced this pull request Jul 20, 2020
@RaduW @BYK @MaicolBen
feat(relay): Add Relay to onpremise installation (getsentry#421)
bdd0c8a 
Co-Authored-By: Burak Yigit Kaya <byk@sentry.io>
@yoan-adfinis yoan-adfinis mentioned this pull request Aug 7, 2020
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jan-auer jan-auer jan-auer left review comments

@BYK BYK BYK approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@RaduW @jan-auer @kagansari @BYK @untitaker