internal/lsp/cmd: add a flag to disable telemetry
govim integration tests (and probably some real user sessions) are
broken because telemetry metrics are not threadsafe, resulting in an
index out of range panic.
Fix this by adding a flag (labeled temporary) to disable telemetry
export.
Also temporarily update govim to master to pick up some fixes, and run
only the -short tests to avoid timeouts.
Updates golang/go#38042
Change-Id: I584e5d200c2f732bd4024002ee6253d09623b29f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226057
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/gopls/integration/govim/Dockerfile b/gopls/integration/govim/Dockerfile
index f729e2f..e4af733 100644
--- a/gopls/integration/govim/Dockerfile
+++ b/gopls/integration/govim/Dockerfile
@@ -17,4 +17,4 @@
# a redundant copy of govim to the build cache using `go mod download`.
RUN GOVIM_VERSION=$(go mod download -json github.com/govim/govim@latest | jq -r .Version) && \
git clone https://github.com/govim/govim /src/govim && cd /src/govim && \
- git checkout $GOVIM_VERSION
+ git checkout master
diff --git a/gopls/integration/govim/run_tests_for_cloudbuild.sh b/gopls/integration/govim/run_tests_for_cloudbuild.sh
index 0f59938..3433364 100755
--- a/gopls/integration/govim/run_tests_for_cloudbuild.sh
+++ b/gopls/integration/govim/run_tests_for_cloudbuild.sh
@@ -9,8 +9,12 @@
# build step. We do this so that we can capture govim test artifacts regardless
# of the test results.
+# See golang.org/issues/38042. Temporarily disable telemetry until event
+# exporters are threadsafe.
+export GOVIM_GOPLS_FLAGS="-telemetry.disable"
+
# Substitute the locally built gopls binary for use in govim integration tests.
-go test ./cmd/govim -gopls /workspace/gopls/gopls
+go test -short ./cmd/govim -gopls /workspace/gopls/gopls
# Stash the error, for use in a later build step.
echo "exit $?" > /workspace/govim_test_result.sh
diff --git a/internal/lsp/cmd/serve.go b/internal/lsp/cmd/serve.go
index 90a34f2..861de10 100644
--- a/internal/lsp/cmd/serve.go
+++ b/internal/lsp/cmd/serve.go
@@ -17,6 +17,7 @@
"golang.org/x/tools/internal/lsp/debug"
"golang.org/x/tools/internal/lsp/lsprpc"
"golang.org/x/tools/internal/lsp/protocol"
+ "golang.org/x/tools/internal/telemetry/event"
"golang.org/x/tools/internal/tool"
)
@@ -34,6 +35,7 @@
RemoteListenTimeout time.Duration `flag:"remote.listen.timeout" help:"when used with -remote=auto, the listen.timeout used when auto-starting the remote"`
RemoteDebug string `flag:"remote.debug" help:"when used with -remote=auto, the debug address used when auto-starting the remote"`
RemoteLogfile string `flag:"remote.logfile" help:"when used with -remote=auto, the filename for the remote daemon to log to"`
+ DisableExport bool `flag:"telemetry.disable" help:"TEMPORARY WORKAROUND: disable telemetry processing entirely. This flag will be removed in the future, once telemetry issues are resolved."`
app *Application
}
@@ -60,6 +62,12 @@
return tool.CommandLineErrorf("server does not take arguments, got %v", args)
}
+ // Temporary workaround for golang.org/issues/38042: allow disabling
+ // telemetry export.
+ if s.DisableExport {
+ event.SetExporter(nil)
+ }
+
di := debug.GetInstance(ctx)
if di != nil {
closeLog, err := di.SetLogFile(s.Logfile)
