Skip to content

Commit fc60870

Browse files
myronmarstonclaude
andcommitted
Add extension_data to QueryDetailsTracker for query registry logging
Add mechanism for GraphQL extensions to log additional data in ElasticGraphQueryExecutorQueryDuration message. Query registry uses this to log query_registration_status with values: - matched_registered_query: exact match of registered query - differing_registered_query: same operation name but query differs - unregistered_query: client registered, no query with that name - unregistered_client: client not registered at all Key changes: - Add extension_data field and []= method to QueryDetailsTracker - Merge extension_data into QueryExecutor log output - Update query validators to return registration_status as 3rd element - Update GraphQL extension to set query_registration_status in tracker - Optimize ForRegisteredClient to check operation name before canonicalization for better performance - Use selected_operation_name instead of operation_name to correctly identify single-operation queries Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a6bf012 commit fc60870

17 files changed

Lines changed: 228 additions & 36 deletions

File tree

elasticgraph-graphql/lib/elastic_graph/graphql/query_details_tracker.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class QueryDetailsTracker < Struct.new(
1717
:datastore_query_server_duration_ms,
1818
:datastore_query_client_duration_ms,
1919
:queried_shard_count,
20+
:extension_data,
2021
:mutex
2122
)
2223
def self.empty
@@ -27,6 +28,7 @@ def self.empty
2728
datastore_query_server_duration_ms: 0,
2829
datastore_query_client_duration_ms: 0,
2930
queried_shard_count: 0,
31+
extension_data: {},
3032
mutex: ::Thread::Mutex.new
3133
)
3234
end
@@ -52,6 +54,13 @@ def record_datastore_query_metrics(client_duration_ms:, server_duration_ms:, que
5254
def datastore_request_transport_duration_ms
5355
datastore_query_client_duration_ms - datastore_query_server_duration_ms
5456
end
57+
58+
# Allows extensions to set custom data that will be included in the query duration log.
59+
def []=(key, value)
60+
mutex.synchronize do
61+
extension_data[key] = value
62+
end
63+
end
5564
end
5665
end
5766
end

elasticgraph-graphql/lib/elastic_graph/graphql/query_executor.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def execute(
117117
"datastore_query_count" => query_tracker.query_counts_per_datastore_request.sum,
118118
"over_slow_threshold" => (duration > @slow_query_threshold_ms).to_s,
119119
"slo_result" => slo_result_for(query, duration)
120-
})
120+
}.merge(query_tracker.extension_data))
121121
end
122122

123123
result

elasticgraph-graphql/sig/elastic_graph/graphql/query_details_tracker.rbs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ module ElasticGraph
77
attr_accessor datastore_query_server_duration_ms: ::Integer
88
attr_accessor datastore_query_client_duration_ms: ::Integer
99
attr_accessor queried_shard_count: ::Integer
10+
attr_accessor extension_data: ::Hash[::String, untyped]
1011
attr_accessor mutex: ::Thread::Mutex
1112

1213
def initialize: (
@@ -16,6 +17,7 @@ module ElasticGraph
1617
datastore_query_server_duration_ms: ::Integer,
1718
datastore_query_client_duration_ms: ::Integer,
1819
queried_shard_count: ::Integer,
20+
extension_data: ::Hash[::String, untyped],
1921
mutex: ::Thread::Mutex
2022
) -> void
2123
end
@@ -30,6 +32,7 @@ module ElasticGraph
3032
) -> void
3133

3234
def datastore_request_transport_duration_ms: () -> ::Integer
35+
def []=: (::String, untyped) -> untyped
3336
end
3437
end
3538
end

elasticgraph-graphql/sig/graphql_gem.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ module GraphQL
113113
attr_reader query_string: ::String?
114114

115115
def selected_operation: () -> Language::Nodes::OperationDefinition?
116+
def selected_operation_name: () -> ::String?
116117
def static_errors: () -> ::Array[_ValidationError]
117118

118119
class Context
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# Copyright 2024 - 2026 Block, Inc.
2+
#
3+
# Use of this source code is governed by an MIT-style
4+
# license that can be found in the LICENSE file or at
5+
# https://opensource.org/licenses/MIT.
6+
#
7+
# frozen_string_literal: true
8+
9+
require "elastic_graph/graphql/query_details_tracker"
10+
11+
module ElasticGraph
12+
class GraphQL
13+
RSpec.describe QueryDetailsTracker do
14+
describe "#[]=" do
15+
let(:tracker) { QueryDetailsTracker.empty }
16+
17+
it "allows extensions to set custom data in extension_data" do
18+
tracker["custom_key"] = "custom_value"
19+
expect(tracker.extension_data).to eq("custom_key" => "custom_value")
20+
end
21+
22+
it "allows multiple values to be set" do
23+
tracker["key1"] = "value1"
24+
tracker["key2"] = "value2"
25+
26+
expect(tracker.extension_data).to eq(
27+
"key1" => "value1",
28+
"key2" => "value2"
29+
)
30+
end
31+
32+
it "allows overwriting existing extension data" do
33+
tracker["key"] = "original_value"
34+
tracker["key"] = "new_value"
35+
36+
expect(tracker.extension_data).to eq("key" => "new_value")
37+
end
38+
39+
it "allows non-string values (per RBS signature)" do
40+
tracker["int_key"] = 42
41+
tracker["array_key"] = [1, 2, 3]
42+
43+
expect(tracker.extension_data).to eq(
44+
"int_key" => 42,
45+
"array_key" => [1, 2, 3]
46+
)
47+
end
48+
end
49+
end
50+
end
51+
end

elasticgraph-graphql/spec/unit/elastic_graph/graphql/query_executor_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,31 @@ class GraphQL
369369
}.to log(a_string_including("resulted in errors"))
370370
end
371371

372+
context "when extensions provide additional log data" do
373+
it "includes extension data in the logged duration message" do
374+
# Simulate an extension setting data in the query tracker
375+
allow(::GraphQL::Execution::Interpreter).to receive(:run_all).and_wrap_original do |original, schema, queries, context:|
376+
query_tracker = context[:elastic_graph_query_tracker]
377+
query_tracker["custom_field"] = "custom_value"
378+
query_tracker["another_field"] = "another_value"
379+
original.call(schema, queries, context: context)
380+
end
381+
382+
execute_expecting_no_errors(<<-QUERY, client: Client.new(name: "client-name", source_description: "client-description"))
383+
query GetColors {
384+
colors(args: {red: 12}) {
385+
red
386+
}
387+
}
388+
QUERY
389+
390+
expect(logged_duration_message).to include(
391+
"custom_field" => "custom_value",
392+
"another_field" => "another_value"
393+
)
394+
end
395+
end
396+
372397
context "when the schema has been customized (as in an extension like elasticgraph-apollo)" do
373398
before(:context) do
374399
multiply_resolver = Class.new do

elasticgraph-query_registry/lib/elastic_graph/query_registry/client_data.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,18 @@ def with_updated_last_query(query_string, query)
5454
end
5555

5656
def unregistered_query_error_for(query, client)
57-
if operation_names.include?(query.operation_name.to_s)
58-
"Query #{fingerprint_for(query)} differs from the registered form of `#{query.operation_name}` " \
57+
# Note: we use `selected_operation_name` instead of `operation_name` because `operation_name` can return
58+
# `nil` for single-operation queries when no explicit operation_name parameter is passed if accessed before
59+
# the query AST is parsed, whereas `selected_operation_name` parses the query AST and returns the operation
60+
# name from the query document in that case.
61+
selected_op_name = query.selected_operation_name.to_s
62+
63+
if operation_names.include?(selected_op_name)
64+
"Query #{fingerprint_for(query)} differs from the registered form of `#{selected_op_name}` " \
5965
"for client #{client.description}."
6066
else
6167
"Query #{fingerprint_for(query)} is unregistered; client #{client.description} has no " \
62-
"registered query with a `#{query.operation_name}` operation."
68+
"registered query with a `#{selected_op_name}` operation."
6369
end
6470
end
6571

elasticgraph-query_registry/lib/elastic_graph/query_registry/graphql_extension.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,16 @@ def initialize(
6060
private
6161

6262
def build_and_execute_query(query_string:, variables:, operation_name:, context:, client:)
63-
query, errors = @registry.build_and_validate_query(
63+
query, errors, registration_status = @registry.build_and_validate_query(
6464
query_string,
6565
variables: variables,
6666
operation_name: operation_name,
6767
context: context,
6868
client: client
6969
)
7070

71+
context.fetch(:elastic_graph_query_tracker)["query_registration_status"] = registration_status
72+
7173
if errors.empty?
7274
[query, execute_query(query, client: client)]
7375
else

elasticgraph-query_registry/lib/elastic_graph/query_registry/query_validators/for_registered_client.rb

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
require "elastic_graph/graphql/client"
1010
require "elastic_graph/query_registry/client_data"
11+
require "elastic_graph/query_registry/registration_status"
1112
require "graphql"
1213

1314
module ElasticGraph
@@ -44,19 +45,34 @@ def build_and_validate_query(query_string, client:, variables: {}, operation_nam
4445

4546
if (cached_query = client_data.cached_query_for(query_string.to_s))
4647
prepared_query = prepare_query_for_execution(cached_query, variables: variables, operation_name: operation_name, context: context)
47-
return [prepared_query, []]
48+
return [prepared_query, [], RegistrationStatus::MATCHED_REGISTERED_QUERY]
4849
end
4950

5051
query = yield
5152

53+
# Check operation name first (fast O(1) set lookup) to avoid canonicalization when not needed.
54+
# Note: we use `selected_operation_name` instead of `operation_name` because `operation_name` returns
55+
# `nil` for single-operation queries when no explicit operation_name parameter is passed, whereas
56+
# `selected_operation_name` returns the operation name from the query document in that case.
57+
registration_status =
58+
if client_data.operation_names.include?(query.selected_operation_name.to_s)
59+
if client_data.canonical_query_strings.include?(ClientData.canonical_query_string_from(query, schema_element_names: schema.element_names))
60+
RegistrationStatus::MATCHED_REGISTERED_QUERY
61+
else
62+
RegistrationStatus::DIFFERING_REGISTERED_QUERY
63+
end
64+
else
65+
RegistrationStatus::UNREGISTERED_QUERY
66+
end
67+
5268
# This client allows any query, so we can just return the query with no errors here.
5369
# Note: we could put this at the top of the method, but if the query is registered and matches
5470
# the registered form, the `cached_query` above is more efficient as it avoids unnecessarily
5571
# parsing the query.
56-
return [query, []] if allow_any_query_for_clients.include?(client.name)
72+
return [query, [], registration_status] if allow_any_query_for_clients.include?(client.name)
5773

58-
if !client_data.canonical_query_strings.include?(ClientData.canonical_query_string_from(query, schema_element_names: schema.element_names))
59-
return [query, [client_data.unregistered_query_error_for(query, client)]]
74+
unless registration_status == RegistrationStatus::MATCHED_REGISTERED_QUERY
75+
return [query, [client_data.unregistered_query_error_for(query, client)], registration_status]
6076
end
6177

6278
# The query is slightly different from a registered query, but not in any material fashion
@@ -74,7 +90,7 @@ def build_and_validate_query(query_string, client:, variables: {}, operation_nam
7490
(_ = cached_client_data).with_updated_last_query(query_string, cachable_query)
7591
end
7692

77-
[query, []]
93+
[query, [], registration_status]
7894
end
7995

8096
private

elasticgraph-query_registry/lib/elastic_graph/query_registry/query_validators/for_unregistered_client.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#
77
# frozen_string_literal: true
88

9+
require "elastic_graph/query_registry/registration_status"
10+
911
module ElasticGraph
1012
module QueryRegistry
1113
module QueryValidators
@@ -15,15 +17,15 @@ module QueryValidators
1517
def build_and_validate_query(query_string, client:, variables: {}, operation_name: nil, context: {})
1618
query = yield
1719

18-
return [query, []] if allow_unregistered_clients
20+
return [query, [], RegistrationStatus::UNREGISTERED_CLIENT] if allow_unregistered_clients
1921

2022
client_name = client&.name
21-
return [query, []] if client_name && allow_any_query_for_clients.include?(client_name)
23+
return [query, [], RegistrationStatus::UNREGISTERED_CLIENT] if client_name && allow_any_query_for_clients.include?(client_name)
2224

2325
[query, [
2426
"Client #{client&.description || "(unknown)"} is not a registered client, it is not in " \
2527
"`allow_any_query_for_clients` and `allow_unregistered_clients` is false."
26-
]]
28+
], RegistrationStatus::UNREGISTERED_CLIENT]
2729
end
2830
end
2931
end

0 commit comments

Comments
 (0)