1
0
mirror of https://github.com/golang/go synced 2024-11-18 10:44:45 -07:00
Commit Graph

92 Commits

Author SHA1 Message Date
Rob Findley
df83f4e7c1 internal/lsp: fix builds and tests for go1.12+
Seems we've drifted a bit from go1.12 support, mostly due to error
wrapping.

Fix this, as well as some assorted other failures.

I haven't tested 1.12 interactively.

For golang/go#39146

Change-Id: Id347ead2a13e89b76d2ae0047750e6b6b49911eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/250941
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-27 14:30:46 +00:00
Heschi Kreinick
c9619e8fac internal/lsp: separate LSP files from FS files
FileHandle currently includes LSP-level information about Version and
Session. That's dangerous, because the cache operates in terms of
URIs and content only -- we explicitly want to share results across
sessions and versions if they happen to be the same.

Split the LSP information into separate types, VersionedFileHandle and
VersionedFileIdentity.

Change-Id: I158646b783375b58245468599301e2a29c657e71
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245058
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-03 22:16:06 +00:00
Rob Findley
f2c07d7d8e internal/lsp/lsprpc: improvements to daemon logging
The gopls daemon had different default logging behavior than the sidecar
gopls: by default, the daemon was started with -logfile=auto.
Additionally, because most logs are reflected back to the forwarder, the
actual daemon logs have very little (if any) information.

This means that if you simply start gopls with -remote=auto, you'll get
a single logfile named /tmp/gopls-<pid>.log, which is mostly empty. This
is not a delightful experience.

Fix this via several improvements:
 + Log lifecycle events in the Daemon, to give the log a purpose.
 + Give the daemon a new default log location:
   /tmp/gopls-daemon-<pid>.log.
 + Don't pass -logfile=auto to the daemon by default.

Fixes golang/go#40105

Change-Id: I5e91ea98b4968c512bce76a596bbae441f461a66
Reviewed-on: https://go-review.googlesource.com/c/tools/+/241440
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-16 19:33:22 +00:00
Heschi Kreinick
8b7669898d internal/lsp/debug: save per-package stats in memory debug files
The per-package stats have proven pretty useful, and I don't want to
have to teach users how to save them. Create zip files and add them in.

Since some users may be sensitive about revealing any information about
the code, generate two variants: one with package names, and one
without.

Change-Id: Icc5631b4cebbbabfdd2fcea4a4cdf4f205dbcab9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239037
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-19 18:00:37 +00:00
Heschi Kreinick
4651fa3054 internal/lsp/debug: show per-package memory usage
Calculate and display very crude memory usage statistics. This is
complicated by various levels of sharing and indirection, so the numbers
should be taken with *large* grains of salt and interpreted mostly by
experts.

Still, the results are interesting and helpful.

Change-Id: Ia9aff974c7d5fddd63df0cfd5cecc08ead33cf84
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236163
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-12 20:10:52 +00:00
Ian Cottrell
742c5eb664 internal/lsp: fix the cache debug page
The cache does not know the sessions associated with it, so the
debug template cannot display them.

Change-Id: I9b043592351ddce097f4e74f3e9aa7a6ad24e613
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237618
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-12 02:23:31 +00:00
Ian Cottrell
54cf04ef09 internal/lsp: decouple client and server debug
This uses log messages to convey information to the debug system, which
has the benefit of logging even if the debug pages are not active and
also not requiring systems to reach into the debug system or require
extra lifetime tracking Not all things are decoupled yet as there are a
couple of places (notably the handshaker) that read information out of
the debug system.

Change-Id: Iec1f81c34ab3b11b3e3d6e6eb39b98ee5ed0d849
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236337
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-11 13:49:48 +00:00
Ian Cottrell
c42cb6316f internal/lsp: minor cleanup of the client and server debug
This removes the interfaces and the debug structs in the lsprpc package
in favour of just having the debug structs in the debug package.

Change-Id: I67541688444f4ef367007740c5446dcd7be6575f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236198
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:50 +00:00
Ian Cottrell
ce53dc4445 internal/lsp: clean out the debug handling
This removes all the cache/session/view hooks from the lsp and instead
uses normal introspection from the client to find all the entities
rather than keeping shadow copies of the object graph in the debug page
handler.
This required the addition only of the ability to view the sessions open
on a cache and exposing the unique identifier for all the objects, both
of which are useful and reasonable things to have in the API anyway.

Change-Id: Ic19903e7b19832ca79290954ec373d4177789742
Reviewed-on: https://go-review.googlesource.com/c/tools/+/236197
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-06-06 01:49:14 +00:00
yuichi kishimoto
52effbd89c internal/lsp: fix typo in code comment
Change-Id: I1120517383d0d3d314984ae7ba49cafbee34820d
GitHub-Last-Rev: ed8df77aaa108ef079340718685727f413f2e9d5
GitHub-Pull-Request: golang/tools#230
Reviewed-on: https://go-review.googlesource.com/c/tools/+/235837
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-05-30 23:37:09 +00:00
Ian Cottrell
01e0872ccf internal/event: improve the logging of events
This extracts the printing code from the log writer so it can be re-used
and then changes the loggers in the lsp to use it.
This means that messages that used to look like

  date:
    message=text

now print as

  date: text

which makes the logs a lot easier to read.

Change-Id: I9dfbae47cdc9aeb7d3ca3279e445f39f2e590827
Reviewed-on: https://go-review.googlesource.com/c/tools/+/232989
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-05-11 17:49:55 +00:00
Ian Cottrell
535e1470ec internal/lsp: use %w in error wrappers
This fixes a bunch of fmt.Errorf calls to use %w rather than %v when wrapping
an error with additional context.

Change-Id: I03088376fbf89aa537555e825e5d02544d813ed2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/231477
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-04 14:52:14 +00:00
Ian Cottrell
a90b7300be internal/event: remove the event.eventType type
Instead of tagging events with their type, instead we infer the type from
the label pattern.
The standard event creators all have a matching test that returns true
if the the labels pattern matches the ones that would be built by the
creator.
Spans and logs already have a unique label pattern, other event types
required a special label marker.
This makes the system much more extensible, and also cleans up some
the API.

Change-Id: I1fbc9ec07aa84ead6c12bbd5ca65b13b605bfa4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229242
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-04-27 15:30:19 +00:00
Ian Cottrell
a466788a31 internal/event: change event.At to be a private field
This was the last piece of Event that was public, and it was only public to
allow mutation in tests.
Adding CloneEvent allows tests to create an updated copy rather than
update the event in place.

Change-Id: I2215d1eb0317063948ef0fac955fa768a209564d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229241
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 18:13:49 +00:00
Ian Cottrell
a82abb5396 internal/event: extract keys to their own package
Now key types can be implemented outside the package that holds labels or events, they should be.
This prevents the large list of types from poluting the public interface of the core packages.

Change-Id: I927f31cb5e4e1d0c29619681015962f890623e5c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229240
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 18:13:43 +00:00
Ian Cottrell
c81623a0cb internal/event: move event/core.Tag to event/label.Label
Also moves core.Key to label.Key, but leaves the implementations
behind for now.
After using for a while, the word Tag conveys slightly the wrong
concept, tagging implies the entire set of information, label maps
better to a single named piece of information.
A label is just a named key/value pair, it is not really tied to the
event package, separating it makes it much easier to understand the
public symbols of the event and core packages, and allows us to also
move the key implementations somewhere else, which otherwise dominate
the API.

Change-Id: I46275d531cec91e28af6ab1e74a2713505d52533
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229239
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-23 18:13:33 +00:00
Ian Cottrell
7b212d60a1 internal/event: renaming the main event API functions
event.Log removed
event.Print -> event.Log
event.Record -> event.Metric
event.StartSpan -> event.Start

In order to support this core now exposes the MakeEvent and Export functions.

Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:21:36 +00:00
Ian Cottrell
cf0cb92717 internal/telemetry: renaming to internal/event
internal/telemetry/event was renamed to internal/event/core
Some things were partly moved from internal/telemetry/event straight to
internal/event to minimize churn in the following restructuring.

Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23 17:20:48 +00:00
Ian Cottrell
6a75126720 internal/lsp: make tag iteration allocation-free
Change the tag iteration api to something less flexible
that allows for event iteration without allocation.

Change-Id: I212d45ebceea0183d1a61e6b611e0558649be60a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227301
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-04-08 13:20:38 +00:00
Rohan Challa
46bd65c853 internal/lsp/cache: add concurrency error check for go cmds
This change attempts to fix a concurrency error that would cause
textDocument/CodeLens, textDocument/Formatting, textDocument/DocumentLink,
and textDocument/Hover from failing on go.mod files.

The issue was that the go command would return a potential concurrency
error since the ModHandle and the ModTidyHandle are both using the
temporary go.mod file.

Updates golang/go#37824

Change-Id: I6cd63c1f75817c7308e033aec473966536a2a3bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224917
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-08 03:22:09 +00:00
Ian Cottrell
903869a827 internal/lsp: make event directly implement TagMap
Makes Event implement TagMap directly, instead of
having to build an entirely new object.

Change-Id: I0c1e8638de3dc3347f60fd93af3df6b7f8387751
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227300
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-04-06 17:24:01 +00:00
Ian Cottrell
7db14c95bf internal/lsp: rewrite the rpc debug page
Now it only uses the telemetry messages directly rather than the metric system.
This is much faster and more direct, and removes a blocker on improving the
metrics support.
Also fixes the fact that recieced and sent were the wrong way round before,
probably as an artifact of the old protocol logging code, and also removes
the bytes histogram which was a lie (it was a histogram of write sizes that
was presented as a histogram of message sizes)

fixes golang/go#38168

Change-Id: Ib1c3459c0ff1cf0c6087a828981e80c1ce1c5c1b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227139
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-06 14:44:18 +00:00
Rebecca Stambler
44a64ad78b internal/lsp, go/packages: don't log context cancellation errors
Instead of checking the context, check the error. This may expose some
errors that are not wrapped correctly. Replaced all uses of errors
with golang.org/x/xerrors.

Change-Id: Ia40160f8ea352e02618765f2a9415a4ece0dcd94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227036
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-03 19:08:13 +00:00
Ian Cottrell
80f63e2b9b internal/lsp: rewrite the stats using the newer telemetry features
This allows us to reduce the handler interface and delete the telemetry handler.
It is also safer and faster, and can be easily disabled along with the rest of
the telemetry system.

Change-Id: Ia4961d7f2e374f7dc22360d6a4020a065bfeae6f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225957
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-31 00:50:33 +00:00
Ian Cottrell
657a652153 internal/telemetry: add a synchronization to telemetry
Some things that used to be safe due to the global
lock now need ther own synchronization primitives.

Fixes golang/go#38102

Change-Id: I03c692682d57620d96fe84b7dc74efae0d3f8f09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226317
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-30 19:15:27 +00:00
Ian Cottrell
f61d083d3b internal/telemetry: use tags instead of special event fields.
Change-Id: I0e6a26c62bd1f6eaa07c38a06152cb7a0f2eedc2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225579
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-29 02:56:35 +00:00
Ian Cottrell
c9942794f0 internal/telemetry: render trace tags using typed keys
Type switch on the key and use it to get the value and decide how
to render it.
Use the same render for all debug tag printing.

Change-Id: Ia305fded7dcf05b57c5805f48bb5c22fa7def71f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225380
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:46:26 +00:00
Ian Cottrell
1249273038 internal/telemetry: make metrics take a strongly typed key
Now that keys are solidly typed, we can use them for the metrics.
This prevents accidentally using the wrong type of key, and
allows us to use the typed accesorrs rather than the raw value.

Change-Id: I553bd8e12128d3f00a3e926dbd3bfd420cd3f135
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225378
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-26 17:45:58 +00:00
Ian Cottrell
224c947ce5 internal/telemetry: replace TagSet with TagMap and TagPointer
This separates the concerns of tag collections that have to be iterated
and tag collections that need lookup by key.
Also make it so that events just carry a plain slice of tags.
We pass a TagMap down through the exporters and allow it to be
extended on the way.
We no longer need the event.Query method (or the event type)
We now exclusivley use Key as the identity, and no longer have a
common core implementation but just implement it directly in each
type.
This removes some confusion that was causing the same key through
different paths to end up with a different identity.

Change-Id: I61e47adcb397f4ca83dd90342b021dd8e9571ed3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224278
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-25 01:00:44 +00:00
Ian Cottrell
62abcc1da2 internal/telemetry: change Exporter to be a function type.
Change-Id: Id410da20310baf4da6875de08e4449c7a6fb0250
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224277
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-25 00:42:15 +00:00
Rebecca Stambler
6fc5d0bc36 internal/lsp: print view-specific environment
For debugging purposes, we print the output of `go env` on start. Now,
we print a view-specific `go env`, which will helps us when debugging
multi-project workspaces. Additional information included: the folder of
the view, if the view has a valid build configuration, and the build
flags for the view.

Updates golang/go#37978

Change-Id: Iadffd46f8ac5410971558a304b8bbcb2f9bdbcc3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/225137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-24 20:15:47 +00:00
Ian Cottrell
8dcfad9e01 internal/telemetry: switch metrics to use the event system
Change-Id: If036530ffe47e7df925bcf6592f664d6020a3d65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223997
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:44:30 +00:00
Ian Cottrell
b378960d5b internal/telemetry: replace event.TagList with event.TagSet
This allows us to hide the implementation details of how tags are stored on a
context from the normal interface, to allow us to explore more efficient
mechanisms.
The current storage is not intended as the most efficient choice, this cl is
about isolating the API so we can experiment with benchmarks in the future.

Change-Id: Ib101416bccd8ecdee269cee636b1564d51e1da8a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222854
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-23 14:43:48 +00:00
Rob Findley
9a0fabac01 internal/lsp: fix errors found by staticcheck
While experimenting with different static analysis on x/tools, I noticed
that there are many actionable diagnostics found by staticcheck. Fix the
ones that were not false positives.

Change-Id: I0b68cf1f636b57b557db879fad84fff9b7237a89
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222248
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-19 19:20:54 +00:00
Ian Cottrell
540150da73 internal/telemetry: add type safe tag keys
This changes the way keys work, there is still a single internal key
implementation for performance reasons, but the public interface is a set of key
implementations that have type safe Of and Get methods.
This also hides the implemenation of Tag so that we can modify the storage form
and find a more efficient storage if needed.

Change-Id: I6a39cc75c2824c6a92e52d59f16e82e876f16e9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223137
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:29:43 +00:00
Ian Cottrell
d7fc2cf50e internal/telemetry: delete the event.TagOf method
It encourages poor performing log lines, and also reduces the readability of
those lines.
Also delete the Key.With method which was unused, and should never be used
instead of the event.Label function anyway.

Change-Id: I9b55102864ee49a7d03e60af022a2002170c0fb1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222851
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:23:15 +00:00
Ian Cottrell
04208b9e8a internal/lsp: move the telemetry package
Move the lsp specific telemetry package that just declares the labels under the
debug package and call it tag, to make all the usages much more readable.

Change-Id: Ic89b3408dd3b8b3d914cc69d81f41b8919aaf424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222850
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:49 +00:00
Ian Cottrell
cb106d260e internal/telemetry: allow ProcessEvent to modify the event
This allows early exporters to adjust the event for later ones.
This is used to lookup key values from the context if needed.
Also add a Query type event which is intended to perform all event
modifications but nothing else, and is used to lookup values from
the context. This cleans up a weirdness where the current lookup
presumes there will be an exporter with a matching mechanism.

Change-Id: I835d1e0b2511553c30f94b7becfe7b7b5462c111
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223657
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-18 13:22:01 +00:00
Ian Cottrell
eff45d17df internal/telemetry: convert key from string to struct
This makes keys a pointer, which reduces the allocations during logging, and has
a significant improvement in memory and cpu cost when there is no exporter.
Packing a string into an interface requires a 16 byte allocation, packing a pointer
does not.

name                old time/op    new time/op    delta
/LogNoExporter-8      4.39µs ± 2%    3.83µs ± 2%  -12.74%  (p=0.000 n=18+20)
/Log-8                23.5µs ± 2%    22.6µs ± 1%   -4.20%  (p=0.000 n=19+18)

name                old alloc/op   new alloc/op   delta
/LogNoExporter-8      1.70kB ± 0%    1.38kB ± 0%  -18.78%  (p=0.000 n=20+20)
/Log-8                4.91kB ± 0%    4.91kB ± 0%     ~     (p=1.000 n=20+20)

name                old allocs/op  new allocs/op  delta
/LogNoExporter-8        83.0 ± 0%      63.0 ± 0%  -24.10%  (p=0.000 n=20+20)
/Log-8                   163 ± 0%       163 ± 0%     ~     (all equal)

Change-Id: Iec127f1bff8d5c8f4bd0a6d9f6d8fc4b8bc740b2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222599
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 04:17:52 +00:00
Ian Cottrell
206ec5b82a internal/lsp: migrate telemetry to using the event package
Change-Id: Idc662385ed08bd62593ccd1d54afd3fa8c1a7d29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-12 03:59:16 +00:00
Peter Weinberger
20ab64c0d9 internal/lsp: bring generated protocol code up to date; stop corrupting logs
1. logging at debug/rpc.go:105 was corrupting logs.
2. DocumentSymbol (a Server method) now returns []interface{}.
3. The latest version of the LSP changed CompletionItem.TextEdit to
possibly be of the new type InsertReplaceEdit, so the generated Go
code now has TextEdit an interface{} rather than a *TextEdit. This
required a change to tests/util.go.
4. The latest version also introduced several other new types,
and new members in some structs.

Tests pass and the I've use the new gopls a little.

Change-Id: Ic44d46f0c923882c9076c2754c1c85e09fbcaa2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222668
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-09 20:21:50 +00:00
Rob Findley
8287d625a0 internal/lsp/debug: guard rpc stats with a Mutex
This type was previously guarded implicitly by the global exporter
mutex. With that gone, we've started seeing panics due to races in
(*rpcs).Metric.

Guard it with a mutex.

Change-Id: I2a8c670ecfbaee9422e1f1d282b1fedb86b6a5e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222300
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
2020-03-06 16:15:13 +00:00
Ian Cottrell
32f14692fc internal/lsp: use standardised events for tagging
This means that tags also become cheap if there is no exporter and cleans up the
mess with how spans, tags and logs were related.
Also fixes the currently broken metrics that relied on the span tags.

Change-Id: I8e56b6218a60fd31a1f6c8d329dbb2cab1b9254d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222065
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-05 14:00:10 +00:00
Ian Cottrell
c4206d458c internal/telemetry: change tracing to be event based
We no longer use the span as the core type of tracing, instead that is an
artifact of the exporter, and start and end tracing is just event based.
This both makes the interface normalized, and also means the null exporter case
is considerably cheaper in memory and cpu.
See below for benchstat changes

name                 old time/op    new time/op    delta
TracingNoExporter-8    4.19µs ±12%    2.71µs ±11%  -35.33%  (p=0.000 n=20+20)
Tracing-8              24.1µs ± 3%     5.1µs ±17%  -78.66%  (p=0.000 n=16+20)

name                 old alloc/op   new alloc/op   delta
TracingNoExporter-8    2.32kB ± 0%    0.40kB ± 0%  -82.76%  (p=0.000 n=20+20)
Tracing-8              6.32kB ± 0%    2.32kB ± 0%  -63.30%  (p=0.000 n=20+20)

name                 old allocs/op  new allocs/op  delta
TracingNoExporter-8      35.0 ± 0%      15.0 ± 0%  -57.14%  (p=0.000 n=20+20)
Tracing-8                 215 ± 0%        35 ± 0%  -83.72%  (p=0.000 n=20+20)

Change-Id: I3cf25871fa49584819504b5c19aa580e5dd03395
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221740
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-04 02:41:40 +00:00
Ian Cottrell
046aa1cdaf internal/telemetry: moving towards a unified event based exporter
This adds a type to events and makes the logging calls use it.

Change-Id: Iaa50fe2e332caae611b1e00424c878a3bc479feb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221741
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:56:52 +00:00
Ian Cottrell
ae19ec7143 internal/telemetry: use atomics to get the exporter
We change the main exporter to be stored and fetched using atomics rather
than aquiring a mutex for a mild (but significant in the disabled case) speedup.
Also has the benefit of not holding a global lock over all telemetry operations.

benchstat of logging benchmatks before and after:

name                 old time/op    new time/op    delta
Baseline-8              329ns ± 2%     327ns ± 2%     ~     (p=0.181 n=19+17)
LoggingNoExporter-8    3.08µs ± 3%    2.42µs ± 2%  -21.42%  (p=0.000 n=20+19)
Logging-8              13.7µs ± 2%    13.2µs ± 1%   -3.49%  (p=0.000 n=19+19)
LoggingStdlib-8        5.39µs ± 3%    5.41µs ± 2%     ~     (p=0.177 n=19+20)

This is a replacement for https://go-review.googlesource.com/c/tools/+/212244
but built on the single exporter principle rather than the exporter list.

Change-Id: Icc99319c4357e0bcb63386c64372a733e8a76796
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221218
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-03-03 22:56:03 +00:00
Ian Cottrell
4183ba16a9 internal/lsp: move the debug.Instance onto the Context
This allows us to register a telemetry exporter that works with mulitple active
debug instances.
It also means we don't have to store the debug information in our other objects.

Change-Id: I9a9d5b0407c3352b6eaff80fb2c434ca33f4e397
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221558
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-03-03 22:54:53 +00:00
Pontus Leitzler
7ae42e3a8b internal/lsp/debug: add nil checks in debug page
There are missing nil checks in a few places when using the gopls
debug page. It will result in panics when trying to access a debug
page that no longer exist (for example a closed session).

This change adds nil checks and updates the template to not render
non-existing debug address.

Change-Id: Ic9163f3727fd8c51122cbd4ab4374fc4d55477c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221699
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-02 15:51:55 +00:00
Ian Cottrell
71482053b8 internal/telemetry: remove Flush method from exporter
It is never invoked anyway, and it provices no useful benefit while complicating
the implementation, as it is the only non context aware method.

Change-Id: Id5a99439fedafdf4d71285e36103b4854cf3635a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221540
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-02-28 22:46:39 +00:00
Ian Cottrell
2e887e3d13 internal/telemetry: removing the concept of exporter lists
Instead we only have a single exporter, and it must delegate behaviour
to any other exporters it wants to include.
This removes a whole collection of suprises caused by init functions adding
new exporters to a list, as well as generally making things faster, at the small
expense of needing to implement a custom exporter if you want to combine the
features of a few other exporters.

This is essentially the opposite of https://go-review.googlesource.com/c/tools/+/212243
which will now be abandoned in favor of this approach.

Change-Id: Icacb4c1f0f40f99ddd1d82c73d4f25a3486e56ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220857
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-26 21:51:01 +00:00