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

DM-48476: Basic Sentry integration #90

Merged
merged 2 commits into from
Jan 22, 2025
Merged

DM-48476: Basic Sentry integration #90

merged 2 commits into from
Jan 22, 2025

Conversation

fajpunk
Copy link
Member

@fajpunk fajpunk commented Jan 16, 2025 •
edited
Loading

Let's integrate with Sentry in the most basic possible way and see what happens 😄

With this, and the Phalanx PR, we will start getting Slack notifications for uncaught exceptions, and traces for fastapi requests, database queries, and arq jobs.

  • We're not tracing the SSE endpoint because Sentry accumulates spans in memory for all events for as long as they're being streamed to a client. The uninstrumented endpoint also seems to hold on to memory, but Sentry definitely makes it worse.
  • The project has already been created manually. Project creation can be done via API, mabye via a Phalanx command in the future.
  • A catch-all alert has been created manually. We'll definitely want some phalanx functionality to create alerts via the Sentry API, because we'll need to make them for every environment.

fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 16, 2025
@fajpunk
DM-48476: times-square Sentry
6e2f6b3 
Support for lsst-sqre/times-square#90
@fajpunk fajpunk mentioned this pull request Jan 16, 2025
Merged
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 16, 2025
@fajpunk
DM-48476: times-square Sentry
e2c2d3f 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 16, 2025
@fajpunk
DM-48476: times-square Sentry
4bb33e4 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 16, 2025
@fajpunk
DM-48476: times-square Sentry
927cd22 
Support for lsst-sqre/times-square#90
@fajpunk fajpunk force-pushed the tickets/DM-48476/yolo-sentry branch 3 times, most recently from 6c6177a to c98caa6 Compare January 17, 2025 15:07
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
dd47c76 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
b66478e 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
8a0f48d 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
5b2689c 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
36906e0 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
7248891 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 17, 2025
@fajpunk
DM-48476: times-square Sentry
8e352f1 
Support for lsst-sqre/times-square#90
@fajpunk fajpunk force-pushed the tickets/DM-48476/yolo-sentry branch from d5aed35 to 20a1843 Compare January 21, 2025 15:40
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 21, 2025
@fajpunk
DM-48476: times-square Sentry
2fb54cf 
Support for lsst-sqre/times-square#90
@fajpunk fajpunk force-pushed the tickets/DM-48476/yolo-sentry branch 4 times, most recently from 1d7a5eb to 77dc627 Compare January 21, 2025 18:18
@fajpunk fajpunk requested review from jonathansick and removed request for jonathansick January 21, 2025 18:21
jonathansick
jonathansick approved these changes Jan 21, 2025
View reviewed changes
Copy link
Member

@jonathansick jonathansick left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, let's get this into production. Add a scriv fragment before closing and perhaps consider releasing as a version 0.16.0

tests/handlers/internal_test.py Outdated Show resolved Hide resolved
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 21, 2025
@fajpunk
DM-48476: times-square Sentry
542f1c5 
Support for lsst-sqre/times-square#90
@fajpunk fajpunk force-pushed the tickets/DM-48476/yolo-sentry branch 2 times, most recently from 6c54d09 to 062140d Compare January 22, 2025 17:28
@fajpunk
DM-48476: Sentry - the most basic integration possible
1f8833e 
Let's use Sentry in the most basic possible way and see what
happens.Don't trace the SSE endpoint because Sentry holds spans in
memory aslong as the connection is open. It seems like the
uninstrumented SSEendopoint also holds on to memory for some reason that
I (Dan) don'tunderstand, but tracing it definitely makes it worse.
@fajpunk fajpunk force-pushed the tickets/DM-48476/yolo-sentry branch from 062140d to 1f8833e Compare January 22, 2025 17:36
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 22, 2025
@fajpunk
DM-48476: times-square Sentry
5014d95 
Support for lsst-sqre/times-square#90
@fajpunk fajpunk merged commit d12eb2f into main Jan 22, 2025
4 checks passed
@fajpunk fajpunk deleted the tickets/DM-48476/yolo-sentry branch January 22, 2025 17:48
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 22, 2025
@fajpunk
DM-48476: times-square Sentry
594726b 
Support for lsst-sqre/times-square#90
fajpunk added a commit to lsst-sqre/phalanx that referenced this pull request Jan 22, 2025
@fajpunk
DM-48476: times-square Sentry
76f50bc 
Support for lsst-sqre/times-square#90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathansick jonathansick jonathansick 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.
2 participants
@fajpunk @jonathansick