Repository: mattermost/mattermost-plugin-github
Pull request #653 (2023-03-09 09:34:33)

[MM-51269] Log errors from telemetry events
#### Summary
Previously errors returned from the telemetry tracker didn't get logged. This PR adds proper log messages for all errors the telemetry tracker returns.

#### Ticket Link
https://mattermost.atlassian.net/browse/MM-51269m

codecov-commenter: # [Codecov](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/653?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) Report
Patch coverage has no change and project coverage change: **`-0.02`** :warning:
> Comparison is base [(`39a2a70`)](https://codecov.io/gh/mattermost/mattermost-plugin-github/commit/39a2a7021f8579b1004080d426113be14d146bb6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) 15.62% compared to head [(`b988cd7`)](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/653?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) 15.60%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #653      +/-   ##
==========================================
- Coverage   15.62%   15.60%   -0.02%     
==========================================
  Files          15       15              
  Lines        5447     5452       +5     
==========================================
  Hits          851      851              
- Misses       4553     4558       +5     
  Partials       43       43              
```


| [Impacted Files](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/653?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) | Coverage Δ | |
|---|---|---|
| [server/plugin/telemetry.go](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/653?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3BsdWdpbi90ZWxlbWV0cnkuZ28=) | `0.00% <0.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/mattermost/mattermost-plugin-github/pull/653?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost).


Patch:
diff --git a/server/plugin/flows.go b/server/plugin/flows.go
index d5c7454..c480401 100644
--- a/server/plugin/flows.go
+++ b/server/plugin/flows.go
@@ -12,7 +12,6 @@ import (
 	pluginapi "github.com/mattermost/mattermost-plugin-api"
 	"github.com/mattermost/mattermost-plugin-api/experimental/flow"
 
-	"github.com/mattermost/mattermost-plugin-api/experimental/telemetry"
 	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost-server/v6/model"
@@ -23,6 +22,11 @@ type PingBroker interface {
 	SubscribePings() <-chan *github.PingEvent
 }
 
+type Tracker interface {
+	TrackEvent(event string, properties map[string]interface{})
+	TrackUserEvent(event, userID string, properties map[string]interface{})
+}
+
 type FlowManager struct {
 	client           *pluginapi.Client
 	pluginURL        string
@@ -32,7 +36,7 @@ type FlowManager struct {
 	getGitHubClient  func(ctx context.Context, userID string) (*github.Client, error)
 
 	pingBroker PingBroker
-	tracker    telemetry.Tracker
+	tracker    Tracker
 
 	setupFlow        *flow.Flow
 	oauthFlow        *flow.Flow
@@ -50,7 +54,7 @@ func (p *Plugin) NewFlowManager() *FlowManager {
 		getGitHubClient:  p.GetGitHubClient,
 
 		pingBroker: p.webhookBroker,
-		tracker:    p.tracker,
+		tracker:    p,
 	}
 
 	fm.setupFlow = fm.newFlow("setup").WithSteps(
@@ -223,14 +227,14 @@ func (fm *FlowManager) StartSetupWizard(userID string, delegatedFrom string) err
 }
 
 func (fm *FlowManager) trackStartSetupWizard(userID string, fromInvite bool) {
-	_ = fm.tracker.TrackUserEvent("setup_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("setup_wizard_start", userID, map[string]interface{}{
 		"from_invite": fromInvite,
 		"time":        model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompleteSetupWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("setup_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("setup_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
@@ -249,13 +253,13 @@ func (fm *FlowManager) StartOauthWizard(userID string) error {
 }
 
 func (fm *FlowManager) trackStartOauthWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("oauth_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("oauth_wizard_start", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompleteOauthWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("oauth_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("oauth_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
@@ -591,13 +595,13 @@ func (fm *FlowManager) StartWebhookWizard(userID string) error {
 }
 
 func (fm *FlowManager) trackStartWebhookWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("webhook_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("webhook_wizard_start", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompleteWebhookWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("webhook_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("webhook_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
@@ -755,13 +759,13 @@ func (fm *FlowManager) StartAnnouncementWizard(userID string) error {
 }
 
 func (fm *FlowManager) trackStartAnnouncementWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("announcement_wizard_start", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("announcement_wizard_start", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
 
 func (fm *FlowManager) trackCompletAnnouncementWizard(userID string) {
-	_ = fm.tracker.TrackUserEvent("announcement_wizard_complete", userID, map[string]interface{}{
+	fm.tracker.TrackUserEvent("announcement_wizard_complete", userID, map[string]interface{}{
 		"time": model.GetMillis(),
 	})
 }
diff --git a/server/plugin/permalinks.go b/server/plugin/permalinks.go
index 84d97cc..e0cc3a0 100644
--- a/server/plugin/permalinks.go
+++ b/server/plugin/permalinks.go
@@ -3,6 +3,7 @@ package plugin
 import (
 	"context"
 	"encoding/hex"
+	"path"
 	"strings"
 	"time"
 
@@ -75,7 +76,7 @@ func (p *Plugin) getReplacements(msg string) []replacement {
 			case "commit":
 				r.permalinkInfo.commit = m[j]
 			case "path":
-				r.permalinkInfo.path = m[j]
+				r.permalinkInfo.path = strings.TrimPrefix(path.Join("/", m[j]), "/")
 			case "line":
 				r.permalinkInfo.line = m[j]
 			}
diff --git a/server/plugin/telemetry.go b/server/plugin/telemetry.go
index 813951e..d3e814d 100644
--- a/server/plugin/telemetry.go
+++ b/server/plugin/telemetry.go
@@ -11,6 +11,20 @@ const (
 	keysPerPage = 1000
 )
 
+func (p *Plugin) TrackEvent(event string, properties map[string]interface{}) {
+	err := p.tracker.TrackEvent(event, properties)
+	if err != nil {
+		p.API.LogDebug("Error sending telemetry event", "event", event, "error", err.Error())
+	}
+}
+
+func (p *Plugin) TrackUserEvent(event, userID string, properties map[string]interface{}) {
+	err := p.tracker.TrackUserEvent(event, userID, properties)
+	if err != nil {
+		p.API.LogDebug("Error sending user telemetry event", "event", event, "error", err.Error())
+	}
+}
+
 func (p *Plugin) SendDailyTelemetry() {
 	config := p.getConfiguration()
 
@@ -19,7 +33,7 @@ func (p *Plugin) SendDailyTelemetry() {
 		p.API.LogWarn("Failed to get the number of connected users for telemetry", "error", err)
 	}
 
-	_ = p.tracker.TrackEvent("stats", map[string]interface{}{
+	p.TrackEvent("stats", map[string]interface{}{
 		"connected_user_count":          connectedUserCount,
 		"is_oauth_configured":           config.IsOAuthConfigured(),
 		"is_sass":                       config.IsSASS(),


Test patch:
diff --git a/server/plugin/permalinks_test.go b/server/plugin/permalinks_test.go
index 0927652..98b8135 100644
--- a/server/plugin/permalinks_test.go
+++ b/server/plugin/permalinks_test.go
@@ -48,7 +48,34 @@ func TestGetReplacements(t *testing.T) {
 					},
 				},
 			},
-		}, {
+		},
+		{
+			name:            "basic link with relative path",
+			input:           "start https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/../../../authentication.go#L15-L22 lorem ipsum",
+			numReplacements: 1,
+			replacements: []replacement{
+				{
+					index: 6,
+					word:  "https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/../../../authentication.go#L15-L22",
+					permalinkInfo: struct {
+						haswww string
+						commit string
+						user   string
+						repo   string
+						path   string
+						line   string
+					}{
+						haswww: "",
+						commit: "cbb25838a61872b624ac512556d7bc932486a64c",
+						line:   "L15-L22",
+						path:   "authentication.go",
+						user:   "mattermost",
+						repo:   "mattermost-server",
+					},
+				},
+			},
+		},
+		{
 			name:            "duplicate expansions",
 			input:           "start https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/app/authentication.go#L15-L22 lorem ipsum https://github.com/mattermost/mattermost-server/blob/cbb25838a61872b624ac512556d7bc932486a64c/app/authentication.go#L15-L22 lorem ipsum",
 			numReplacements: 2,