Skip to content

Commit 7be58c3

Browse files
jwilsclaude
andauthored
Map DateTime to google.protobuf.Timestamp and document temporal string formats (#1306)
## Why Mapping `DateTime` to `string` loses the type safety proto offers — a string can be malformed in ways a [`google.protobuf.Timestamp`](https://protobuf.dev/reference/protobuf/google.protobuf/#timestamp) cannot, and proto consumers get language-native timestamp types. For the remaining `string`-typed temporal scalars, the proto type is much wider than the ElasticGraph type, so the expected format deserves documentation at the point of use (per [review discussion](#1080 (comment))). ## What - Map `DateTime` to `google.protobuf.Timestamp`, importing `google/protobuf/timestamp.proto` automatically. A `Timestamp` is a UTC instant, so a publisher's original UTC offset is not preserved; `t.protobuf type: "string"` remains available as an override. - `t.protobuf` gains two options usable by any scalar: - `import:` maps a scalar to an externally defined proto type, emitting the needed `import` statement - `field_comment:` documents the expected format above each generated field - The built-in `string`-typed temporal scalars (`Date`, `LocalTime`, `TimeZone`) use `field_comment:` to document their formats (e.g. `// Must be formatted as an ISO 8601 date, e.g. "2024-11-25".`). Values are still validated at ingestion time, exactly as with JSON ingestion. - Each `protobuf` call replaces the full protobuf configuration, so an override that omits `import:` or `field_comment:` clears the value a prior call set. ## Risk Assessment Low — only affects the unreleased `elasticgraph-proto_ingestion` extension from #1080. ## References - Stacked on #1304 (→ #1080) - Addresses #1080 (comment) and #1080 (comment) ## Update — 2026-07-10 Nested-list wrapper comment propagation was removed after #1080 changed to reject lists of lists. Format comments continue to apply to supported scalar and single-list fields. ## Update — 2026-08-18 Addressed review feedback: - `comment:` is now `field_comment:`, and renders as `//` lines **above** the field rather than trailing the field line. Proto compilers attach leading comments to the code they generate for a field; a trailing comment is usually discarded. This also removed the reason the comment had to be a single line, so a multi-line `field_comment:` is now allowed. - Built-in temporal comments were reworded to read well above the field (e.g. `Must be formatted as an ISO 8601 date, e.g. "2024-11-25".`). - The "each `protobuf` call replaces the full configuration" behavior is now documented and covered in both directions (override that drops the import, and override that drops the field comment). - Enum and object types now answer `protobuf_import` with `nil`, so import rendering no longer greps for scalar types and a non-scalar type can start requiring an import without a change there. - Spec reorganization (moving scalar-related examples out of `schema_spec.rb`) is handled in a follow-up PR at the end of this stack. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 08b5661 commit 7be58c3

13 files changed

Lines changed: 500 additions & 52 deletions

File tree

elasticgraph-proto_ingestion/README.md

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,27 +94,100 @@ ElasticGraph.define_schema do |schema|
9494
end
9595
```
9696

97+
A custom scalar can also map to an externally defined proto type. Pass `import:` with the path of
98+
the proto file that defines the type:
99+
100+
```ruby
101+
# in config/schema/duration.rb
102+
103+
ElasticGraph.define_schema do |schema|
104+
schema.scalar_type "Duration" do |t|
105+
t.mapping type: "keyword"
106+
t.json_schema type: "string"
107+
t.protobuf type: "google.protobuf.Duration", import: "google/protobuf/duration.proto"
108+
end
109+
end
110+
```
111+
112+
The generated `schema.proto` then contains `import "google/protobuf/duration.proto";`. ElasticGraph
113+
emits the import only when a generated message uses the scalar. The `import:` value must be the
114+
path of a `.proto` file.
115+
116+
`field_comment:` documents the expected format on each generated field. This is useful when the
117+
proto type is wider than the ElasticGraph type:
118+
119+
```ruby
120+
# in config/schema/phone_number.rb
121+
122+
ElasticGraph.define_schema do |schema|
123+
schema.scalar_type "PhoneNumber" do |t|
124+
t.mapping type: "keyword"
125+
t.json_schema type: "string"
126+
t.protobuf type: "string", field_comment: "Must be an E.164 phone number."
127+
end
128+
end
129+
```
130+
131+
A `PhoneNumber` field then renders as:
132+
133+
```protobuf
134+
// Must be an E.164 phone number.
135+
string phone_number = 1;
136+
```
137+
138+
The comment goes above the field, below the field's own doc comment, because proto compilers
139+
attach these leading comments to the code they generate for the field. A `field_comment:` can
140+
span multiple lines. The option is named `field_comment:` rather than `comment:` because a scalar
141+
type has no proto representation of its own to comment on; the comment applies to each field of
142+
that type.
143+
144+
### Overriding a Built-in Scalar
145+
146+
Use `on_built_in_types` to change the protobuf type of a built-in scalar. For example, map
147+
`DateTime` to `string` to keep the original UTC offset of each event:
148+
149+
```ruby
150+
# in config/schema/protobuf.rb
151+
152+
ElasticGraph.define_schema do |schema|
153+
schema.on_built_in_types do |type|
154+
type.protobuf type: "string", field_comment: "Must be formatted as an ISO 8601 timestamp." if type.name == "DateTime"
155+
end
156+
end
157+
```
158+
159+
Each call to `protobuf` replaces the full protobuf configuration. The override above omits
160+
`import:`, so `schema.proto` no longer imports `google/protobuf/timestamp.proto`. An override that
161+
omits `field_comment:` likewise drops the built-in comment.
162+
97163
## Type Mappings
98164

99165
The generated `schema.proto` uses these built-in scalar mappings:
100166

101-
| ElasticGraph Type | Protobuf Type |
102-
|-------------------|------------|
103-
| `Boolean` | `bool` |
104-
| `Cursor` | `string` |
105-
| `Date` | `string` |
106-
| `DateTime` | `string` |
107-
| `Float` | `double` |
108-
| `ID` | `string` |
109-
| `Int` | `int32` |
110-
| `JsonSafeLong` | `int64` |
111-
| `LocalTime` | `string` |
112-
| `LongString` | `int64` |
113-
| `String` | `string` |
114-
| `TimeZone` | `string` |
115-
| `Untyped` | `string` |
167+
| ElasticGraph Type | Protobuf Type |
168+
|-------------------|-----------------------------|
169+
| `Boolean` | `bool` |
170+
| `Cursor` | `string` |
171+
| `Date` | `string` |
172+
| `DateTime` | `google.protobuf.Timestamp` |
173+
| `Float` | `double` |
174+
| `ID` | `string` |
175+
| `Int` | `int32` |
176+
| `JsonSafeLong` | `int64` |
177+
| `LocalTime` | `string` |
178+
| `LongString` | `int64` |
179+
| `String` | `string` |
180+
| `TimeZone` | `string` |
181+
| `Untyped` | `string` |
116182

117183
Additionally:
184+
- `DateTime` uses the [well-known `Timestamp` type](https://protobuf.dev/reference/protobuf/google.protobuf/#timestamp);
185+
`schema.proto` imports `google/protobuf/timestamp.proto` automatically. Note that a `Timestamp`
186+
is a UTC instant, so a publisher's original UTC offset is not preserved.
187+
- `string`-typed temporal scalars (`Date`, `LocalTime`, `TimeZone`) are wider than the
188+
ElasticGraph types they carry, so generated fields of these types document the expected format
189+
in a comment above the field (e.g. `// Must be formatted as an ISO 8601 date, e.g. "2024-11-25".`).
190+
Values are validated when events are ingested, just as with JSON ingestion.
118191
- List types become `repeated` fields.
119192
- Lists of lists (e.g. `[[Float!]!]!`) are not supported because Protocol Buffers cannot represent
120193
them directly. Schema artifact generation raises an error identifying the unsupported field.

elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ def to_proto
4848
sections = [
4949
%(syntax = "proto3";),
5050
"package #{@package_name};",
51+
*render_imports(types),
5152
render_definitions(types)
5253
]
5354

@@ -107,6 +108,15 @@ def render_definitions(types)
107108
.join("\n\n")
108109
end
109110

111+
# Every type reports the proto file it needs imported, or `nil` when it needs none. Today only
112+
# scalar types map to an externally defined proto type, but enum and object types can start
113+
# requiring an import without any change here.
114+
def render_imports(types)
115+
imports = types.filter_map(&:protobuf_import).uniq.sort
116+
117+
imports.empty? ? [] : [imports.map { |import| %(import "#{import}";) }.join("\n")]
118+
end
119+
110120
def validate_unique_enum_value_prefixes(types)
111121
enum_type_by_prefix = {} # : ::Hash[::String, untyped]
112122

elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,21 @@ def proto_type_reference(package_name)
6767
".#{package_name}.#{proto_name}"
6868
end
6969

70+
# Enum types render their own protobuf definition, so they never require an import.
71+
#
72+
# @return [nil]
73+
def protobuf_import
74+
nil
75+
end
76+
77+
# Enum values are self-describing, so fields of this type get no format comment.
78+
# Only scalar types document a format.
79+
#
80+
# @return [nil]
81+
def protobuf_field_comment
82+
nil
83+
end
84+
7085
# Returns the package-level prefix applied to this enum's protobuf values.
7186
#
7287
# @return [String]

elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rb

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,21 @@ def proto_type_reference(package_name)
6666
".#{package_name}.#{proto_name}"
6767
end
6868

69+
# Messages render their own protobuf definition, so they never require an import.
70+
#
71+
# @return [nil]
72+
def protobuf_import
73+
nil
74+
end
75+
76+
# Messages carry their documentation on the message definition itself, so fields of this
77+
# type get no format comment. Only scalar types document a format.
78+
#
79+
# @return [nil]
80+
def protobuf_field_comment
81+
nil
82+
end
83+
6984
private
7085

7186
def render_proto_message(schema, message_name, package_name)
@@ -75,7 +90,7 @@ def render_proto_message(schema, message_name, package_name)
7590
active_field_names = fields.map { |schema_field, _| schema_field.name }
7691
documentation = ProtoDocumentation.comment_lines_for(doc_comment).map { |line| "#{line}\n" }.join
7792
field_definitions = fields.map do |schema_field, field|
78-
repeated, field_type = proto_field_type_for(
93+
repeated, field_type, field_comment = proto_field_type_for(
7994
field.type,
8095
package_name: package_name,
8196
context_field_name: field.name
@@ -87,12 +102,9 @@ def render_proto_message(schema, message_name, package_name)
87102
)
88103
label = "repeated " if repeated
89104
line = " #{label}#{field_type} #{schema_field.name} = #{field_number};"
90-
field_documentation = ProtoDocumentation
91-
.comment_lines_for(schema_field.doc_comment, indent: " ")
92-
.map { |comment_line| "#{comment_line}\n" }
93-
.join
105+
comment_lines = field_comment_lines_for(schema_field.doc_comment, field_comment)
94106

95-
"#{field_documentation}#{line}"
107+
[*comment_lines, line].join("\n")
96108
end
97109
schema.reserved_field_numbers_for(message_name, active_field_names).each do |field_name, field_number|
98110
field_definitions << " reserved #{field_number}; // Previously used by #{field_name}."
@@ -150,6 +162,19 @@ def proto_fields
150162
end
151163
end
152164

165+
# Renders a field's documentation and its type's format comment as the `//` lines that go
166+
# above the field. Proto compilers attach these leading comments to the code they generate
167+
# for the field, whereas a trailing comment on the field line is usually discarded.
168+
def field_comment_lines_for(doc_comment, field_comment)
169+
doc_lines = ProtoDocumentation.comment_lines_for(doc_comment, indent: " ")
170+
return doc_lines unless field_comment
171+
172+
format_lines = ProtoDocumentation.comment_lines_for(field_comment, indent: " ")
173+
return format_lines if doc_lines.empty?
174+
175+
doc_lines + [" //"] + format_lines
176+
end
177+
153178
def proto_field_type_for(type_ref, package_name:, context_field_name:)
154179
list_depth, base_type_ref = ObjectInterfaceAndUnionExtension.list_depth_and_base_type(type_ref)
155180

@@ -160,7 +185,7 @@ def proto_field_type_for(type_ref, package_name:, context_field_name:)
160185
end
161186

162187
proto_type = _ = base_type_ref.resolved
163-
[list_depth == 1, proto_type.proto_type_reference(package_name)]
188+
[list_depth == 1, proto_type.proto_type_reference(package_name), proto_type.protobuf_field_comment]
164189
end
165190
end
166191
end

elasticgraph-proto_ingestion/lib/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rb

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,33 +14,57 @@ module SchemaDefinition
1414
module SchemaElements
1515
# Extends ScalarType with proto field type conversion.
1616
module ScalarTypeExtension
17-
# Default protobuf types applied to ElasticGraph's built-in scalar types as they are constructed.
18-
BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME = {
19-
"Boolean" => "bool",
20-
"Cursor" => "string",
21-
"Date" => "string",
22-
"DateTime" => "string",
23-
"Float" => "double",
24-
"ID" => "string",
25-
"Int" => "int32",
26-
"JsonSafeLong" => "int64",
27-
"LocalTime" => "string",
28-
"LongString" => "int64",
29-
"String" => "string",
30-
"TimeZone" => "string",
31-
"Untyped" => "string"
32-
}.freeze
17+
# Default protobuf options applied to ElasticGraph's built-in scalar types as they are constructed.
18+
BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME = {
19+
"Boolean" => {type: "bool"},
20+
"Cursor" => {type: "string"},
21+
"Date" => {type: "string", field_comment: %(Must be formatted as an ISO 8601 date, e.g. "2024-11-25".)},
22+
"DateTime" => {type: "google.protobuf.Timestamp", import: "google/protobuf/timestamp.proto"},
23+
"Float" => {type: "double"},
24+
"ID" => {type: "string"},
25+
"Int" => {type: "int32"},
26+
"JsonSafeLong" => {type: "int64"},
27+
"LocalTime" => {type: "string", field_comment: %(Must be formatted as an ISO 8601 local time, e.g. "14:23:12".)},
28+
"LongString" => {type: "int64"},
29+
"String" => {type: "string"},
30+
"TimeZone" => {type: "string", field_comment: %(Must be an IANA time zone identifier, e.g. "America/Los_Angeles".)},
31+
"Untyped" => {type: "string"}
32+
}.freeze # : ::Hash[::String, {type: ::String, ?import: ::String, ?field_comment: ::String}]
33+
34+
# An `import` is rendered as `import "PATH";`, so a quote or newline in the path would
35+
# produce invalid proto. `protoc` also requires the path to name a `.proto` file.
36+
VALID_PROTOBUF_IMPORT_PATH = %r{\A[\w./-]+\.proto\z}
3337

3438
# Configured protobuf type (e.g. string, int64, bool).
3539
# @dynamic protobuf_type
3640
attr_reader :protobuf_type
3741

38-
# Configures the protobuf type for this scalar type.
42+
# Proto file to import for the configured protobuf type, if it is externally defined.
43+
# @dynamic protobuf_import
44+
attr_reader :protobuf_import
45+
46+
# Comment rendered above each generated proto field of this scalar type.
47+
# @dynamic protobuf_field_comment
48+
attr_reader :protobuf_field_comment
49+
50+
# Configures the protobuf type for this scalar type. Each call replaces the full protobuf
51+
# configuration, so an override that omits `import:` or `field_comment:` clears the value
52+
# configured by a prior call.
3953
#
40-
# @param type [String] protobuf scalar type name
54+
# @param type [String] protobuf type name
55+
# @param import [String, nil] proto file to import for an externally defined type
56+
# @param field_comment [String, nil] comment rendered above each generated field of this type
4157
# @return [void]
42-
def protobuf(type:)
58+
# @raise [Errors::SchemaError] when `import` is not a `.proto` file path
59+
def protobuf(type:, import: nil, field_comment: nil)
60+
if import && !VALID_PROTOBUF_IMPORT_PATH.match?(import)
61+
raise Errors::SchemaError, "`protobuf` import for `#{name}` must be the path of a `.proto` file, " \
62+
"but got: #{import.inspect}."
63+
end
64+
4365
@protobuf_type = type
66+
@protobuf_import = import
67+
@protobuf_field_comment = field_comment
4468
end
4569

4670
# Applies any built-in protobuf type, yields for further configuration, and validates the result.
@@ -50,8 +74,8 @@ def protobuf(type:)
5074
# @raise [Errors::SchemaError] when a protobuf type is missing
5175
def initialize_proto_extension
5276
original_name = type_ref.with_reverted_override.name
53-
if (proto_type = BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME[original_name])
54-
protobuf type: proto_type
77+
if (proto_options = BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME[original_name])
78+
protobuf(**proto_options)
5579
end
5680

5781
yield

elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ module ElasticGraph
3434

3535
def proto_types: () -> ::Array[untyped]
3636
def render_definitions: (::Array[untyped] types) -> ::String
37+
def render_imports: (::Array[untyped] types) -> ::Array[::String]
3738
def validate_unique_enum_value_prefixes: (::Array[untyped] types) -> void
3839
def previous_field_names_for: (::String, ::String) -> ::Array[::String]
3940
def previous_field_names_by_type_name_and_field_name: () -> ::Hash[::String, ::Hash[::String, ::Array[::String]]]

elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/enum_type_extension.rbs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ module ElasticGraph
1111
def value: (::String) ?{ (::ElasticGraph::SchemaDefinition::SchemaElements::EnumValue & EnumValueExtension) -> void } -> void
1212
def proto_name: () -> ::String
1313
def proto_type_reference: (::String package_name) -> ::String
14+
def protobuf_import: () -> nil
15+
def protobuf_field_comment: () -> nil
1416
def proto_enum_value_prefix: () -> ::String
1517
def to_proto: (Schema schema, ::String package_name) -> ::String
1618
def referenced_proto_types: () -> ::Array[::ElasticGraph::SchemaDefinition::SchemaElements::graphQLType]

elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/object_interface_and_union_extension.rbs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ module ElasticGraph
1010

1111
def proto_name: () -> ::String
1212
def proto_type_reference: (::String package_name) -> ::String
13+
def protobuf_import: () -> nil
14+
def protobuf_field_comment: () -> nil
1315
def to_proto: (Schema schema, ::String package_name) -> ::String
1416
def referenced_proto_types: () -> ::Array[::ElasticGraph::SchemaDefinition::SchemaElements::graphQLType]
1517
def self.list_depth_and_base_type: (
@@ -24,11 +26,12 @@ module ElasticGraph
2426
::ElasticGraph::SchemaDefinition::SchemaElements::Field,
2527
::ElasticGraph::SchemaDefinition::Indexing::Field
2628
]]
29+
def field_comment_lines_for: (::String? doc_comment, ::String? field_comment) -> ::Array[::String]
2730
def proto_field_type_for: (
2831
::ElasticGraph::SchemaDefinition::SchemaElements::TypeReference,
2932
package_name: ::String,
3033
context_field_name: ::String
31-
) -> [bool, ::String]
34+
) -> [bool, ::String, ::String?]
3235
end
3336
end
3437
end

elasticgraph-proto_ingestion/sig/elastic_graph/proto_ingestion/schema_definition/schema_elements/scalar_type_extension.rbs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@ module ElasticGraph
33
module SchemaDefinition
44
module SchemaElements
55
module ScalarTypeExtension: ::ElasticGraph::SchemaDefinition::SchemaElements::ScalarType
6-
BUILT_IN_SCALAR_PROTO_TYPES_BY_NAME: ::Hash[::String, ::String]
6+
BUILT_IN_SCALAR_PROTO_OPTIONS_BY_NAME: ::Hash[::String, {type: ::String, ?import: ::String, ?field_comment: ::String}]
7+
VALID_PROTOBUF_IMPORT_PATH: ::Regexp
78

89
attr_reader protobuf_type: ::String?
10+
attr_reader protobuf_import: ::String?
11+
attr_reader protobuf_field_comment: ::String?
912

10-
def protobuf: (type: ::String) -> void
13+
def protobuf: (type: ::String, ?import: ::String?, ?field_comment: ::String?) -> void
1114
def initialize_proto_extension: () { () -> void } -> void
1215
def proto_name: () -> ::String
1316
def proto_type_reference: (::String package_name) -> ::String

0 commit comments

Comments
 (0)