Skip to content

Feat(gateway): sentry #192

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

Merged
merged 18 commits into from
May 15, 2025
Merged

Feat(gateway): sentry #192

merged 18 commits into from
May 15, 2025

Conversation

vertex451
Copy link
Member

@vertex451 vertex451 commented Apr 21, 2025

Resolves #184
Related to openmfp/helm-charts#758

Changes

Replaced envconfig with viper in both Listener and Gateway

  • Used fields from the default config when possible
  • Set default values in root init func.
    P.S. I am not sure about the config struct, currently I am using a single config for both Gateway and Listener, but we can split it into two different configs if it makes more sense. It will affect the config initialization thought.

Added sentry.CaptureError on the most crucial parts.

Note: This PR doesn't cover Sentry integration in the Listener.
Also, I ignored errors coming from user input, instead I focused on the service level errors that we would like to monitor, namely:

  • Failed to start watching files
  • Failed to handle file event because it is not known.
  • Failed to process file change(failed to read from file or to create a GraphQL schema)
  • Failed to Serve HTTP because handler is not found
  • Subscription errors:
    • start a subscription
    • cast event to unstructured
    • determine file changes

TBD

  • Sentry integration - I added needed code, but we still need to connect the deployment on dev, int, prod to the Sentry Dashboard.

On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
@vertex451 vertex451 marked this pull request as ready for review April 21, 2025 13:53
@vertex451 vertex451 requested a review from a team as a code owner April 21, 2025 13:53
Copy link
Member

@aaronschweig aaronschweig left a comment

Choose a reason for hiding this comment

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

Please have a look at https://github.com/openmfp/account-operator/blob/main/cmd/root.go#L31-L53 to see how to properly implement the new configuration with viper

vertex451 and others added 6 commits May 12, 2025 10:25
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
cmd/gateway.go Outdated

// Perform graceful shutdown of HTTP server
log.Info().Msg("Shutting down HTTP server...")
if err := server.Shutdown(ctx); err != nil {
Copy link

Choose a reason for hiding this comment

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

You can not use the already closed ctx here. You need to create a new context (with a timeout maybe).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, I created a new context with timeout from the default config.

vertex451 added 4 commits May 15, 2025 08:16
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
On-behalf-of: @SAP [email protected]
Signed-off-by: Artem Shcherbatiuk <[email protected]>
@vertex451 vertex451 merged commit d7f57e1 into main May 15, 2025
12 checks passed
@vertex451 vertex451 deleted the feat/sentry branch May 15, 2025 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sentry integration for the gateway
4 participants