Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/telemetry/config: add gopls/*/latency histograms #63129

Open
findleyr opened this issue Sep 20, 2023 · 1 comment
Open

x/telemetry/config: add gopls/*/latency histograms #63129

findleyr opened this issue Sep 20, 2023 · 1 comment
Labels
gopls Issues related to the Go language server, gopls. telemetry x/telemetry issues Telemetry-Proposal
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Sep 20, 2023

Counter names

gopls/completion/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}
gopls/definition/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}
gopls/hover/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}
gopls/implementations/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}
gopls/references/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}
gopls/symbol/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}

Description

These counters measure server-side observed latency for various latency sensitive LSP operations. For pragmatic reasons, as well as to reduce variability, timing starts when gopls actually begins handling the request, and does not include jsonrpc2 queue time.

Bucketing is approximately exponential, though adjusted to capture meaningful boundaries. For example, the default completion budget is 100ms, and we typically think of anything faster than 200ms as OK. 500ms, 1s, and 5s are various landmarks of slowness.

More precisely, the <10ms bucket captures latency in the range [0, 10ms), and subsequent buckets save the last capture [P, V), where P is the previous bucket endpoint and V is the current bucket endpoint. The >=5 bucket captures everything else. Is there a different convention I should be following for bucket naming?.

The set of operations for this initial batch of instrumentation is chosen based on our experience, as the most critical latency-sensitive operations we support. (CC @adonovan for his opinion on this)

Rationale

The latency of these operations greatly affects the experience of using gopls. With the large variety of editing environments that gopls supports, it is impossible to adequately benchmark these operations in every scenario. Furthermore, users tend not to file issues for incremental performance regressions ("boiling the frog"), and we've seen cases where a significant regression went unnoticed by the team and largely unreported for months (for example #62665).

By capturing these latency distributions, we can form a picture of the typical user experience, identify the frequency of outliers / atypical experiences, catch performance regressions following releases, and better prioritize our performance work.

Do the counters carry sensitive user information?

No.

How?

We'll instrument timing around these operations in the relevant gopls request handlers.

Proposed Graph Config

counter: gopls/completion/latency:{<10ms,<50ms, <100ms, <200ms, <500ms, <1s, <5s, >=5s}
title: Latency of gopls completion requests
description: Server-side latency observed while handling the LSP textDocument/completion request.
type: histogram
program: golang.org/x/tools/gopls
version: v0.14.0  # the first binary version containing this counter.

New or Update

New

@gopherbot gopherbot added the telemetry x/telemetry issues label Sep 20, 2023
@gopherbot gopherbot added this to the Unreleased milestone Sep 20, 2023
@findleyr findleyr added the gopls Issues related to the Go language server, gopls. label Sep 20, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Sep 21, 2023
@gopherbot
Copy link

Change https://go.dev/cl/530215 mentions this issue: gopls: instrument telemetry for latency of important operations

gopherbot pushed a commit to golang/tools that referenced this issue Sep 22, 2023
Add instrumentation for the telemetry described in golang/go#63129,
along with a test. Also isolate the existing telemetry regtest.

Updates golang/go#63129
Updates golang/go#62665

Change-Id: If641b44d430eaaf96c469c804f42fd72cd821bed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. telemetry x/telemetry issues Telemetry-Proposal
Projects
Development

No branches or pull requests

2 participants