[FB-1484] Sentry: fix middleware and CI test for PLG

* create getter for monitor state

* refactor monitor

* fix http middleware

* change warn to error if attmpt to cluster on plg

* sentry: special considerations if execution is part of test

- skip test if they build a cluster as this will error by design
- skip sending messages to sentry if testing

.gitlab/.gitlab-ci.yml

@@ -140,7 +140,6 @@ run go tests future:
 
 run go tests future plg:
   stage: test
-  allow_failure: true
   image: golang:1.18
   rules:
     - if: '$CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web"'
@@ -148,7 +147,7 @@ run go tests future plg:
   script:
     - echo "Running featurebase plg-specific unit tests..."
     - PKG_LIST=$(go list ./... | grep -v internal/clustertests | paste -s -d, -)
-    - go test -tags=plg -timeout=30m -json -coverprofile=coverage-plg.out -covermode=atomic -coverpkg=${PKG_LIST} ./... | tee test-report-plg.out
+    - go test -tags=plg -timeout=30m -coverprofile=coverage-plg.out -covermode=atomic -coverpkg=${PKG_LIST} ./... | tee test-report-plg.out
   artifacts:
     paths:
       - coverage-plg.out

etcd/embed.go

@@ -283,14 +283,16 @@ func (e *Etcd) parseOptions() *embed.Config {
 		// Checks if FB is running the single-node free version or the multi-node
 		// enterprise version. Sentry.io is enabled on single-node.
 		if AllowCluster() == false {
+			// %% end sonarcloud ignore %%
 			monitor.InitErrorMonitor()
 			e.logger.Infof("Initializing Monitor: Capturing usage metrics")
 			//check for multiple nodes in the cluster and error if present
 			nodes := strings.Split(e.options.InitCluster, ",")
 			if len(nodes) > 1 {
-				e.logger.Warnf("Multiple cluster nodes detected - this version of FeatureBase only supports single node. %+v", e.options.InitCluster)
+				e.logger.Errorf("Multiple cluster nodes detected - this version of FeatureBase only supports single node. %+v", e.options.InitCluster)
 				os.Exit(1)
 			}
+			// %% end sonarcloud ignore %%
 		}
 		cfg.InitialCluster = e.options.InitCluster
 		cfg.ClusterState = embed.ClusterStateFlagNew

http_handler.go

@@ -421,10 +421,11 @@ func (h *Handler) collectStats(next http.Handler) http.Handler {
 
 func (h *Handler) monitorPerformance(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if !monitor.IsOn {
+		if !monitor.IsOn() {
 			next.ServeHTTP(w, r)
 			return
 		}
+		// %% begin sonarcloud ignore %%
 		prefixes := make(map[string]struct{})
 		prefixes["index"] = struct{}{}
 		prefixes["info"] = struct{}{}
@@ -438,11 +439,14 @@ func (h *Handler) monitorPerformance(next http.Handler) http.Handler {
 			if _, ok := prefixes[pathParts[1]]; ok {
 				path := scrubPath(pathParts)
 				txName := fmt.Sprintf("URL: %s, Method: %s", path, r.Method)
-				monitor.CapturePerformance(r.Context(), "http", txName, func() {
-					next.ServeHTTP(w, r)
-				})
+				span := monitor.StartSpan(r.Context(), "http", txName)
+				next.ServeHTTP(w, r)
+				span.Finish()
+				return
 			}
 		}
+		next.ServeHTTP(w, r)
+		// %% end sonarcloud ignore %%
 	})
 }
 

logger/logger.go

@@ -99,7 +99,7 @@ func (s *standardLogger) printf(level int, format string, v ...interface{}) {
 	if level > s.verbosity {
 		return
 	}
-	if monitor.IsOn {
+	if monitor.IsOn() {
 		// intercepts the log message and sends it to the monitor
 		monitor.CaptureException(level, format, v...)
 	}

monitor/monitor.go

@@ -3,6 +3,7 @@ package monitor
 
 import (
 	"context"
+	"flag"
 	"fmt"
 	"log"
 	"time"
@@ -18,13 +19,13 @@ const (
 	LevelDebug
 )
 
-var IsOn bool
+var isOn bool
 
 // Initialiazing Sentry with particular settings
 func InitErrorMonitor() {
-	IsOn = true
+	isOn = true
 	err := sentry.Init(sentry.ClientOptions{
-		Dsn:              "https://a6c854a5aa2a4c5cb5baaf01e6968a77@o1007484.ingest.sentry.io/6448164",
+		Dsn:              getDSN(),
 		AttachStacktrace: true,
 		Debug:            false,
 		TracesSampleRate: 1,
@@ -42,7 +43,7 @@ func InitErrorMonitor() {
 
 // CaptureMessage sends a message to Sentry.
 func CaptureMessage(message string) {
-	if !IsOn {
+	if !isOn || isTest() {
 		return
 	}
 	sentry.CaptureMessage(message)
@@ -51,7 +52,7 @@ func CaptureMessage(message string) {
 
 // CaptureException sends an error to Sentry.
 func CaptureException(level int, format string, v ...interface{}) {
-	if !IsOn {
+	if !isOn || isTest() {
 		return
 	}
 	if level > LevelWarn {
@@ -63,16 +64,6 @@ func CaptureException(level int, format string, v ...interface{}) {
 	defer sentry.Flush(2 * time.Second)
 }
 
-// CapturePerformance spans a function to capture performance metrics.
-func CapturePerformance(ctx context.Context, txType, txName string, fn func()) {
-	if !IsOn {
-		return
-	}
-	span := sentry.StartSpan(ctx, txType, sentry.TransactionName(txName))
-	fn()
-	span.Finish()
-}
-
 // monitorRun runs in a goroutine and sends a heartbeat to Sentry every 24 hours.
 func monitorRun() {
 	for i := 0; ; i++ {
@@ -80,3 +71,36 @@ func monitorRun() {
 		time.Sleep(24 * time.Hour)
 	}
 }
+
+// IsOn returns true if the monitor is enabled.
+func IsOn() bool {
+	return isOn
+}
+
+// isTest returns true if execution is part of test
+func isTest() bool {
+	return flag.Lookup("test.v") != nil
+}
+
+// DSN identifies which sentry project to report to
+func getDSN() string {
+	if isTest() {
+		return "https://13194f56bf7b4e049fab181d489dd297@o1007484.ingest.sentry.io/6493546"
+	}
+	return "https://a6c854a5aa2a4c5cb5baaf01e6968a77@o1007484.ingest.sentry.io/6448164"
+}
+
+// Wrappers around Sentry's span to minimize exposure of sentry elsewhere in the codebase and for single-responsibility
+func StartSpan(ctx context.Context, txType, txName string) *sentry.Span {
+	if !isOn || isTest() {
+		return &sentry.Span{}
+	}
+	return sentry.StartSpan(ctx, txType, sentry.TransactionName(txName))
+}
+
+func Finish(span *sentry.Span) {
+	if !isOn || isTest() {
+		return
+	}
+	span.Finish()
+}

test/cluster.go

@@ -13,6 +13,7 @@ import (
 	pilosa "github.com/molecula/featurebase/v3"
 	"github.com/molecula/featurebase/v3/api/client"
 	"github.com/molecula/featurebase/v3/disco"
+	"github.com/molecula/featurebase/v3/etcd"
 	"github.com/molecula/featurebase/v3/logger"
 	"github.com/molecula/featurebase/v3/proto"
 	"github.com/molecula/featurebase/v3/server"
@@ -515,6 +516,9 @@ func (c *Cluster) AwaitState(expectedState disco.ClusterState, timeout time.Dura
 // If it is empty, default options are used. Otherwise, it must contain size
 // slices of command options, which are used with corresponding nodes.
 func MustNewCluster(tb testing.TB, size int, opts ...[]server.CommandOption) *Cluster {
+	if size > 1 && !etcd.AllowCluster() {
+		tb.Skip("Testing PLG which does not allow clustering")
+	}
 	tb.Helper()
 
 	// We want tests to default to using the in-memory translate store, so we