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.

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

CLI: Add support for sentry #529

Open
rajdip-b opened this issue Nov 11, 2024 · 7 comments · May be fixed by #637
Open

CLI: Add support for sentry #529

rajdip-b opened this issue Nov 11, 2024 · 7 comments · May be fixed by #637
Assignees
Labels
difficulty: 3 help wanted Extra attention is needed pending-dependency Stuck due to a required dependency priority: high scope: cli Everything related to the CLI type: enhancement New feature or request

Comments

@rajdip-b
Copy link
Member

rajdip-b commented Nov 11, 2024

Description

We would like the CLI to send errors to Sentry if metrics_enabled (refer to #528) is enabled. Sentry should be notified in the following events:

  • An internal error occurs in any of the components of the CLI
  • A 500 is returned from the API

Solution

  • You would need to send the error to sentry with the environment set as CLI.
  • Add a Logger.report function that will be used to first call Logger.error and then send the data to sentry. We should not be printing too much using Logger.error in this case.

Requires #528

@rajdip-b rajdip-b added type: enhancement New feature or request help wanted Extra attention is needed pending-dependency Stuck due to a required dependency priority: high difficulty: 3 scope: cli Everything related to the CLI labels Nov 11, 2024
@adityaRajGit
Copy link
Contributor

Can I try?

@rajdip-b
Copy link
Member Author

Sure!, but it depends on the #528 so we need that before you an attempt this.

@rajdip-b
Copy link
Member Author

rajdip-b commented Jan 8, 2025

@muntaxir4 can you get started with this?

@muntaxir4
Copy link
Contributor

Yes picked this up today. Should the DSN be restricted to Keyshade only?

@rajdip-b
Copy link
Member Author

Yeah. Dont worry too much about that though. I'm looking into these changes. Perhaps don't raise a pr aswell. I'm also working on a lot of changes to the release system. It would be better if your put up a pr to that other branch. I'll specify you the name once I'm done with that.

@muntaxir4
Copy link
Contributor

Yeah. Dont worry too much about that though. I'm looking into these changes. Perhaps don't raise a pr aswell. I'm also working on a lot of changes to the release system. It would be better if your put up a pr to that other branch. I'll specify you the name once I'm done with that.

Can you specify which branch it is?

@rajdip-b
Copy link
Member Author

Couldn't yet put that up. You can continue your work on some other branch. And when I'm done, you can just raise a PR into that branch.

@muntaxir4 muntaxir4 linked a pull request Jan 18, 2025 that will close this issue
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: 3 help wanted Extra attention is needed pending-dependency Stuck due to a required dependency priority: high scope: cli Everything related to the CLI type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants