search

mattermost / mattermost-plugin-gitlab

GitLab plugin for Mattermost
Apache License 2.0
141 stars 85 forks source link

Added check to initialize telemetry based on the config value #446

Closed raghavaggarwal2308 closed 10 months ago

raghavaggarwal2308 commented 10 months ago

Summary

Ticket Link

Fixes https://community.mattermost.com/core/pl/meqh4gmgupnfbpuz1wrdge8xdw

codecov[bot] commented 10 months ago

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (98fdf4f) 33.40% compared to head (ad815a8) 33.35%. Report is 3 commits behind head on master.

Files Patch % Lines
server/telemetry.go 0.00% 10 Missing :warning:
server/plugin.go 0.00% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #446 +/- ## ========================================== - Coverage 33.40% 33.35% -0.06% ========================================== Files 22 22 Lines 3979 3985 +6 ========================================== Hits 1329 1329 - Misses 2519 2525 +6 Partials 131 131 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mickmister commented 10 months ago

For some background on this issue, this is a webapp-side telemetry issue, so the fix would need to be somewhere in the webapp (possibly in the core webapp as it turns out).

From the discussion here https://community.mattermost.com/core/pl/p44cwwtq7fdybe7b4ojpoux7xr, it seems the issue we were experiencing is related to importing the rudder js sdk library, implicitly imported by the mattermost-redux library. So it seems the solution involves modifying the mattermost-redux library to conditionally import the rudder sdk, only if telemetry is enabled. I will close this for now since I think the solution is not a change in the plugin repo itself. Also, I'm thinking our team won't be making the changes to resolve the issue