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

Add Bugsnag to report errors #58

Merged
merged 16 commits into from
Sep 15, 2022
Merged

Add Bugsnag to report errors #58

merged 16 commits into from
Sep 15, 2022

Conversation

felipecruz91
Copy link
Collaborator

No description provided.

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

Docker image tag(s) pushed:

docker/volumes-backup-extension:pr-58

Labels added to images:

org.opencontainers.image.title=volumes-backup-extension
org.opencontainers.image.description=Back up, clone, restore, and share Docker volumes effortlessly.
org.opencontainers.image.url=https://github.com/docker/volumes-backup-extension
org.opencontainers.image.source=https://github.com/docker/volumes-backup-extension
org.opencontainers.image.version=pr-58
org.opencontainers.image.created=2022-09-15T08:16:14.122Z
org.opencontainers.image.revision=5043dda11720687423033f067415918fe2063d1a
org.opencontainers.image.licenses=Apache-2.0
org.opencontainers.image.title=Volumes Backup & Share
org.opencontainers.image.description=Back up, clone, restore, and share Docker volumes effortlessly.
org.opencontainers.image.vendor=Docker Inc.

@rumpl
Copy link
Member

rumpl commented Sep 7, 2022

QQ: can't all of the bugnsnag.Notify be done in one place? Like in a middleware or something?

@felipecruz91 felipecruz91 marked this pull request as ready for review September 7, 2022 14:53
Copy link
Member

@benja-M-1 benja-M-1 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I managed to trigger an error by removing the container that was used to clone a volume.

I left a small comment on the code.

I was expecting to have it configured in the frontend as well. I think it is valuable to have it even if the errors overlap the backend part:

  • they can provide more context when it comes to debug (for example, what button has been clicked)
  • also we might be surprised by some errors poping in, from my experience, it is better to have error monitoring with no errors rather than errors not monitored, especially when users can click on buttons and fill inputs 😅

Also, as an improvement, we could add more versions in the data to monitor if a bug is correctly fixed across versions

  • the Docker Desktop version
  • the Docker Extension backend version
  • the versions of the @docker packages (when we have bugsnag in the frontend)

Finally, should we identify users the same way we do it for events? It could be interesting when debugging to see that prior to one error the user had another one for example.

vm/main.go Outdated Show resolved Hide resolved
@felipecruz91
Copy link
Collaborator Author

felipecruz91 commented Sep 15, 2022

@benja-M-1

Also, as an improvement, we could add more versions in the data to monitor if a bug is correctly fixed across versions

  • the Docker Desktop version
  • the Docker Extension backend version
  • the versions of the https://github.com/docker packages (when we have bugsnag in the frontend)
  • Added the Docker Desktop version.

image

  • I can't include the Extension backend version as I'd need to ship the Extensions CLI and the repo is private.

Copy link
Member

@benja-M-1 benja-M-1 left a comment

Choose a reason for hiding this comment

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

One nit but other than it lgtm 👍 Can't wait to have this deployed

vm/internal/setup/bugsnag.go Outdated Show resolved Hide resolved
@benja-M-1 benja-M-1 merged commit 37a42db into main Sep 15, 2022
@felipecruz91 felipecruz91 deleted the feature/add-bugsnag branch September 16, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants