Sentry as option #6759

diocas · 2022-04-07T21:05:00Z

Would you take this? This is quite useful for us to get user's issues without having to wait for a ticket.
I've made it optional and configurable.
But.. Our centrally managed Sentry instance doesn't run the latest version, so the config/package version in here is also not the latest.

update-docs · 2022-04-07T21:05:03Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

ownclouders · 2022-04-07T21:33:33Z

Results for oCISSharingPerm3 https://drone.owncloud.com/owncloud/web/24585/66/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingFolderAdvancedPermissionMultipleUsers-sharedFolderWithMultipleUsersAdvancedPermissions_feature-L50.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

dschmidt

Personally (not speaking for team as a whole) I'm a fan of Sentry and of providing this in general . 🎉

But I'm not happy with the PR as it is right now:

I'm not a fan of the * imports
Especially not where you changed the existing import.. Please use named imports (and maybe rename setUser to setSentryUser or sentrySetUser or something for more clarity)
We should not introduce outdated dependencies
Why can't you use an up to date version? Did they really have breaking changes?

diocas · 2022-04-08T06:58:34Z

Thanks for the comments.
Yes, the latest versions are not compatible with the older. ~~The biggest problem is that they introduced a new message that I cannot disable, and this would result in tons of errors.~~
I can try to see if we can update our central version.

Edit: it looks like it can be disabled after all!

CLAassistant · 2022-04-29T15:09:09Z

All committers have signed the CLA.

javfg · 2022-04-29T15:11:55Z

Sorry for the commit message, I was meaning to upload that to my fork. :)

diocas · 2022-05-03T12:57:38Z

@dschmidt now that @javfg did some improvements, can you do another review?

pascalwengerter

I'm fine with these changes (even though it adds 300kb to the vendor bundle), thanks for updating the dependency in the meantime! @dschmidt what about you?

pascalwengerter · 2022-05-12T14:47:43Z

@diocas I suggest another rebase once @dschmidt has added another review to get CI green, it has gotten way more reliable recently :)

dschmidt

I'm approving to unblock, but please revert the import change before merging

dschmidt · 2022-05-12T15:53:54Z

packages/web-runtime/src/container/bootstrap.ts

@@ -3,7 +3,7 @@ import { RuntimeConfiguration } from './types'
 import { buildApplication, NextApplication } from './application'
 import { Store } from 'vuex'
 import VueRouter from 'vue-router'
-import { VueConstructor } from 'vue'
+import Vue from 'vue'


No, please don't

As noted, please rebase and revert the change back to importing VueConstructor instead of Vue (and usage below, ofc)

pascalwengerter · 2022-05-12T15:57:13Z

packages/web-runtime/src/container/bootstrap.ts

@@ -3,7 +3,7 @@ import { RuntimeConfiguration } from './types'
 import { buildApplication, NextApplication } from './application'
 import { Store } from 'vuex'
 import VueRouter from 'vue-router'
-import { VueConstructor } from 'vue'
+import Vue from 'vue'


As noted, please rebase and revert the change back to importing VueConstructor instead of Vue (and usage below, ofc)

pascalwengerter · 2022-05-13T14:24:56Z

packages/web-runtime/src/container/bootstrap.ts

+ * @param runtimeConfiguration
+ */
+export const startSentry = (runtimeConfiguration: RuntimeConfiguration): void => {
+  if (runtimeConfiguration.sentry?.dsn) {


Also please add this to the docs with a quick note on how to enable

javfg · 2022-05-16T10:18:32Z

Hey @pascalwengerter @dschmidt, I've applied the suggestions you requested. Can you take a look?

Thanks!

pascalwengerter · 2022-05-16T10:20:00Z

Hey @pascalwengerter @dschmidt, I've applied the suggestions you requested. Can you take a look?

Thanks!

Hey, yes it does look good but I reckon you'll need to rebase on the most recent web master one more time due to merge conflicts 🥴

javfg · 2022-05-16T10:34:52Z

Hey @pascalwengerter @dschmidt, I've applied the suggestions you requested. Can you take a look?
Thanks!

Hey, yes it does look good but I reckon you'll need to rebase on the most recent web master one more time due to merge conflicts woozy_face

Just did, but the CI is still breaking. Did I miss something?

pascalwengerter · 2022-05-16T10:36:14Z

packages/web-runtime/package.json

@@ -6,7 +6,13 @@
  "dependencies": {
    "@fortawesome/fontawesome-free": "^6.1.1",
    "@popperjs/core": "^2.4.0",
+<<<<<<< HEAD


@javfg 👀

Sorry about that, should be fixed now.

sonarcloud · 2022-05-16T11:29:44Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

30.0% Coverage
0.0% Duplication

Author: Diogo Castro <diogo.castro@cern.ch> Date: Mon May 16 13:56:21 2022 +0200 Sentry as option (#6759) * Sentry support * Extensible Sentry config * Apply suggestions, add changelog Co-authored-by: Javier Ferrer <javier.ferrer@cern.ch>

diocas requested review from pascalwengerter and fschade as code owners April 7, 2022 21:05

diocas added the Early-Adopter:CERN label Apr 7, 2022

dschmidt requested changes Apr 7, 2022

View reviewed changes

javfg mentioned this pull request Apr 29, 2022

Update sentry libraries, remove wildcard imports cernbox/web#125

Merged

javfg force-pushed the up_sentry branch 2 times, most recently from 26ddcf0 to 762e53f Compare April 29, 2022 15:53

javfg mentioned this pull request Apr 29, 2022

Update sentry libraries, remove wildcard imports cernbox/web#126

Merged

diocas force-pushed the up_sentry branch 2 times, most recently from 510b056 to abdb13e Compare April 29, 2022 16:29

diocas requested a review from dschmidt May 5, 2022 09:36

pascalwengerter approved these changes May 12, 2022

View reviewed changes

dschmidt approved these changes May 12, 2022

View reviewed changes

pascalwengerter requested changes May 12, 2022

View reviewed changes

pascalwengerter reviewed May 13, 2022

View reviewed changes

javfg mentioned this pull request May 16, 2022

Apply some suggestions and add changelog cernbox/web#129

Merged

javfg added 2 commits May 16, 2022 12:26

Sentry support

f8fa992

Extensible Sentry config

8ff11f1

javfg force-pushed the up_sentry branch from 06368be to be34651 Compare May 16, 2022 10:32

pascalwengerter reviewed May 16, 2022

View reviewed changes

javfg force-pushed the up_sentry branch 3 times, most recently from b136e1e to 5debb34 Compare May 16, 2022 11:09

Apply suggestions, add changelog

5cabd2f

javfg force-pushed the up_sentry branch from 5debb34 to 5cabd2f Compare May 16, 2022 11:20

pascalwengerter approved these changes May 16, 2022

View reviewed changes

pascalwengerter merged commit d2f8544 into owncloud:master May 16, 2022

diocas deleted the up_sentry branch May 17, 2022 07:03

kulmann mentioned this pull request May 25, 2022

Release 2.0.0-beta2 owncloud/ocis#3857

Closed

25 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sentry as option #6759

Sentry as option #6759

diocas commented Apr 7, 2022

update-docs bot commented Apr 7, 2022

ownclouders commented Apr 7, 2022

webUISharingFolderAdvancedPermissionMultipleUsers-sharedFolderWithMultipleUsersAdvancedPermissions_feature-L50.png

dschmidt left a comment •

edited

Loading

diocas commented Apr 8, 2022 •

edited

Loading

CLAassistant commented Apr 29, 2022 •

edited

Loading

javfg commented Apr 29, 2022

diocas commented May 3, 2022

pascalwengerter left a comment

pascalwengerter commented May 12, 2022

dschmidt left a comment

dschmidt May 12, 2022

pascalwengerter May 12, 2022

pascalwengerter May 12, 2022

pascalwengerter May 13, 2022

javfg commented May 16, 2022 •

edited

Loading

pascalwengerter commented May 16, 2022

javfg commented May 16, 2022

pascalwengerter May 16, 2022

javfg May 16, 2022

sonarcloud bot commented May 16, 2022

Sentry as option #6759

Sentry as option #6759

Conversation

diocas commented Apr 7, 2022

update-docs bot commented Apr 7, 2022

ownclouders commented Apr 7, 2022

webUISharingFolderAdvancedPermissionMultipleUsers-sharedFolderWithMultipleUsersAdvancedPermissions_feature-L50.png

dschmidt left a comment • edited Loading

Choose a reason for hiding this comment

diocas commented Apr 8, 2022 • edited Loading

CLAassistant commented Apr 29, 2022 • edited Loading

javfg commented Apr 29, 2022

diocas commented May 3, 2022

pascalwengerter left a comment

Choose a reason for hiding this comment

pascalwengerter commented May 12, 2022

dschmidt left a comment

Choose a reason for hiding this comment

dschmidt May 12, 2022

Choose a reason for hiding this comment

pascalwengerter May 12, 2022

Choose a reason for hiding this comment

pascalwengerter May 12, 2022

Choose a reason for hiding this comment

pascalwengerter May 13, 2022

Choose a reason for hiding this comment

javfg commented May 16, 2022 • edited Loading

pascalwengerter commented May 16, 2022

javfg commented May 16, 2022

pascalwengerter May 16, 2022

Choose a reason for hiding this comment

javfg May 16, 2022

Choose a reason for hiding this comment

sonarcloud bot commented May 16, 2022

dschmidt left a comment •

edited

Loading

diocas commented Apr 8, 2022 •

edited

Loading

CLAassistant commented Apr 29, 2022 •

edited

Loading

javfg commented May 16, 2022 •

edited

Loading