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.

Sign up for GitHub

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

Remove server telemetry #650

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Remove server telemetry #650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/ruby_lsp/executor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ def execute(request)
response = T.let(nil, T.untyped)
error = T.let(nil, T.nilable(Exception))

request_time = Benchmark.realtime do
begin
response = run(request)
rescue StandardError, LoadError => e
error = e
end

Result.new(response: response, error: error, request_time: request_time)
Result.new(response: response, error: error)
end

private
Expand Down
1 change: 0 additions & 1 deletion lib/ruby_lsp/internal.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
require "syntax_tree"
require "yarp"
require "language_server-protocol"
require "benchmark"
require "bundler"
require "uri"
require "cgi"
Expand Down
48 changes: 5 additions & 43 deletions lib/ruby_lsp/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,55 +153,17 @@ def finalize_request(result, request)
error: {
code: Constant::ErrorCodes::INTERNAL_ERROR,
message: error.inspect,
data: request.to_json,
data: {
errorClass: error.class.name,
errorMessage: error.message,
backtrace: error.backtrace&.map { |bt| bt.sub(/^#{Dir.home}/, "~") }&.join("\n"),
},
},
)
elsif response != VOID
@writer.write(id: request[:id], result: response)
end

request_time = result.request_time
if request_time
@writer.write(method: "telemetry/event", params: telemetry_params(request, request_time, error))
end
end
end

sig do
params(
request: T::Hash[Symbol, T.untyped],
request_time: Float,
error: T.nilable(Exception),
).returns(T::Hash[Symbol, T.any(String, Float)])
end
def telemetry_params(request, request_time, error)
uri = request.dig(:params, :textDocument, :uri)
params = {
request: request[:method],
lspVersion: RubyLsp::VERSION,
requestTime: request_time,
}

if error
params[:errorClass] = error.class.name
params[:errorMessage] = error.message

log_params = request[:params]
params[:params] = log_params.reject { |k, _| k == :textDocument }.to_json if log_params

backtrace = error.backtrace
params[:backtrace] = backtrace.map { |bt| bt.sub(/^#{Dir.home}/, "~") }.join("\n") if backtrace
end

if uri
home = URI::Generic.from_path(path: Dir.home)

parsed_uri = URI(uri)
path = parsed_uri.path
params[:uri] = path ? path.sub(T.must(home.path), "~") : parsed_uri.opaque
end

params
end
end
end
14 changes: 2 additions & 12 deletions lib/ruby_lsp/utils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,10 @@ class Result
sig { returns(T.nilable(Exception)) }
attr_reader :error

sig { returns(T.nilable(Float)) }
attr_reader :request_time

sig do
params(
response: T.untyped,
error: T.nilable(Exception),
request_time: T.nilable(Float),
).void
end
def initialize(response:, error: nil, request_time: nil)
sig { params(response: T.untyped, error: T.nilable(Exception)).void }
def initialize(response:, error: nil)
@response = response
@error = error
@request_time = request_time
end
end

Expand Down
60 changes: 5 additions & 55 deletions test/integration_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ def test_document_symbol
initialize_lsp(["documentSymbols"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/documentSymbol", { textDocument: { uri: @uri } })
symbol = response[:result].first
assert_equal("Foo", symbol[:name])
Expand All @@ -75,8 +73,6 @@ def test_document_highlight
initialize_lsp(["documentHighlights"])
open_file_with("$foo = 1")

assert_telemetry("textDocument/didOpen")

response = make_request(
"textDocument/documentHighlight",
{ textDocument: { uri: @uri }, position: { line: 0, character: 1 } },
Expand All @@ -90,8 +86,6 @@ def test_hover
initialize_lsp(["hover"])
open_file_with("$foo = 1")

assert_telemetry("textDocument/didOpen")

response = make_request(
"textDocument/hover",
{ textDocument: { uri: @uri }, position: { line: 0, character: 1 } },
Expand All @@ -105,13 +99,8 @@ def test_definition
initialize_lsp(["definition"])
open_file_with("require 'ruby_lsp/utils'")

read_response("textDocument/didOpen")

# Populate the index
send_request("initialized")
read_response("initialized")
# Read the response for the progress end notification
read_response("$/progress")
make_request("initialized")

response = make_request(
"textDocument/definition",
Expand All @@ -131,16 +120,14 @@ def test_document_highlight_with_syntax_error
{ textDocument: { uri: @uri }, position: { line: 0, character: 1 } },
)

assert_nil(response[:result])
assert_empty(response[:result])
assert_nil(response[:error])
end

def test_semantic_highlighting
initialize_lsp(["semanticHighlighting"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/semanticTokens/full", { textDocument: { uri: @uri } })
assert_equal([0, 6, 3, 2, 1], response[:result][:data])
end
Expand All @@ -153,8 +140,6 @@ def foo
end
DOC

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/documentLink", { textDocument: { uri: @uri } })
assert_match(/syntax_tree/, response.dig(:result, 0, :target))
end
Expand All @@ -163,8 +148,6 @@ def test_formatting
initialize_lsp(["formatting"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/formatting", { textDocument: { uri: @uri } })
assert_equal(<<~FORMATTED, response[:result].first[:newText])
# typed: true
Expand All @@ -179,8 +162,6 @@ def test_on_type_formatting
initialize_lsp(["onTypeFormatting"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request(
"textDocument/onTypeFormatting",
{ textDocument: { uri: @uri, position: { line: 0, character: 0 }, character: "\n" } },
Expand All @@ -192,8 +173,6 @@ def test_code_actions
initialize_lsp(["codeActions"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request(
"textDocument/codeAction",
{
Expand Down Expand Up @@ -248,8 +227,6 @@ def test_code_action_resolve
initialize_lsp(["codeActions"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request(
"codeAction/resolve",
{
Expand All @@ -267,8 +244,6 @@ def test_document_did_close
initialize_lsp([])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

assert(send_request("textDocument/didClose", { textDocument: { uri: @uri } }))
end

Expand All @@ -292,8 +267,6 @@ def test_folding_ranges
initialize_lsp(["foldingRanges"])
open_file_with("class Foo\n\nend")

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/foldingRange", { textDocument: { uri: @uri } })
assert_equal({ startLine: 0, endLine: 1, kind: "region" }, response[:result].first)
end
Expand All @@ -302,8 +275,6 @@ def test_code_lens
initialize_lsp(["codeLens"])
open_file_with("class Foo\n\nend")

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/codeLens", { textDocument: { uri: @uri } })
assert_empty(response[:result])
end
Expand All @@ -314,19 +285,14 @@ def test_request_with_telemetry

send_request("textDocument/foldingRange", { textDocument: { uri: @uri } })

assert_telemetry("textDocument/didOpen")

response = read_response("textDocument/foldingRange")
assert_equal({ startLine: 0, endLine: 1, kind: "region" }, response[:result].first)
assert_telemetry("textDocument/foldingRange")
end

def test_selection_ranges
initialize_lsp(["selectionRanges"])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request(
"textDocument/selectionRange",
{
Expand All @@ -353,16 +319,14 @@ def test_selection_ranges_with_syntax_error
},
)

assert_nil(response[:result])
refute_nil(response[:result].first)
assert_nil(response[:error])
end

def test_diagnostics
initialize_lsp([])
open_file_with("class Foo\nend")

assert_telemetry("textDocument/didOpen")

response = make_request("textDocument/diagnostic", { textDocument: { uri: @uri } })

assert_equal("full", response.dig(:result, :kind))
Expand All @@ -372,30 +336,16 @@ def test_diagnostics
def test_workspace_symbol
initialize_lsp(["workspaceSymbol"])
open_file_with("class Foo\nend")
# Read the response for the progress indicator notifications
read_response("textDocument/didOpen")

# Populate the index
send_request("initialized")
read_response("initialized")
# Read the response for the progress end notification
read_response("$/progress")
make_request("initialized")

response = make_request("workspace/symbol", {})
refute_empty(response[:result])
end

private

def assert_telemetry(request, expected_uri = @uri.path.sub(Dir.home, "~"))
telemetry_response = read_response("telemetry/event")

assert_equal(expected_uri, telemetry_response.dig(:params, :uri))
assert_equal(RubyLsp::VERSION, telemetry_response.dig(:params, :lspVersion))
assert_equal(request, telemetry_response.dig(:params, :request))
assert_instance_of(Float, telemetry_response.dig(:params, :requestTime))
end

def make_request(request, params = nil)
send_request(request, params)
read_response(request)
Expand Down Expand Up @@ -456,6 +406,6 @@ def initialize_lsp(enabled_features, experimental_features_enabled: false)
end

def open_file_with(content)
make_request("textDocument/didOpen", { textDocument: { uri: @uri, text: content } })
send_request("textDocument/didOpen", { textDocument: { uri: @uri, text: content } })
end
end