chore : Document all the types and fields of model.proto#187
chore : Document all the types and fields of model.proto#187chethanm99 wants to merge 10 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds documentation comments to proto/api_v2/model.proto to describe the purpose of key message/enum types and several fields in Jaeger’s API v2 protobuf model.
Changes:
- Added doc comments for core model enums/messages (e.g.,
ValueType,KeyValue,Span,Trace,Batch,DependencyLink). - Added per-field documentation for several identifiers and timestamps/durations (e.g.,
trace_id,span_id,start_time,duration). - Updated comments around batching/relationships to better explain span and service dependencies.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proto/api_v2/model.proto
Outdated
| //ValueType describes the type of value stored in a key-value pair. | ||
| enum ValueType { |
There was a problem hiding this comment.
New doc comments are missing a space after // (e.g., //ValueType). The rest of this repo’s .proto comments use // with a space; please update the newly added comments for consistent formatting.
proto/api_v2/model.proto
Outdated
| } | ||
|
|
||
| //Log is a time-stamped event which captures the incidents such as errors, | ||
| //lifecycle events that occured during the execution of the span in key-value format. |
There was a problem hiding this comment.
Spelling: "occured" should be "occurred".
| //lifecycle events that occured during the execution of the span in key-value format. | |
| //lifecycle events that occurred during the execution of the span in key-value format. |
proto/api_v2/model.proto
Outdated
| SpanRefType ref_type = 3; | ||
| } | ||
|
|
||
| //Process represents the service that resuted in the creation of span. |
There was a problem hiding this comment.
Spelling: "resuted" should be "resulted".
| //Process represents the service that resuted in the creation of span. | |
| //Process represents the service that resulted in the creation of span. |
proto/api_v2/model.proto
Outdated
| (gogoproto.customtype) = "TraceID", | ||
| (gogoproto.customname) = "TraceID" | ||
| ]; | ||
| //span_id indentifies the ID of the span. |
There was a problem hiding this comment.
Spelling: "indentifies" should be "identifies".
| //span_id indentifies the ID of the span. | |
| //span_id identifies the ID of the span. |
proto/api_v2/model.proto
Outdated
| repeated Span spans = 1; | ||
| Process process = 2 [ | ||
| (gogoproto.nullable) = true | ||
| ]; |
There was a problem hiding this comment.
The indentation inside message Batch looks off (the spans and process fields are indented more than other messages in this file). Please align indentation to match the surrounding proto style to avoid noisy diffs/formatting issues.
| repeated Span spans = 1; | |
| Process process = 2 [ | |
| (gogoproto.nullable) = true | |
| ]; | |
| repeated Span spans = 1; | |
| Process process = 2 [ | |
| (gogoproto.nullable) = true | |
| ]; |
proto/api_v2/model.proto
Outdated
| BINARY = 4; | ||
| }; | ||
|
|
||
| //KeyValue is a message type used to store dynamically-typed attributes under a single structure. |
There was a problem hiding this comment.
The file-level // TODO: document all types and fields comment is still present near the top of this file. Since this PR’s stated goal is to complete that documentation, consider removing or updating the TODO to avoid implying the work is still incomplete.
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proto/api_v2/model.proto
Outdated
| //Process represents the service that resulted in the creation of span. | ||
| message Process { | ||
| //service_name represents the name of the service which resulted in creation of span. | ||
| string service_name = 1; | ||
| //tags describe the metadata about the service such as Host, IP, Environment etc. |
There was a problem hiding this comment.
The Process message comments are missing spaces after // and have some grammar/capitalization issues (e.g., “creation of span” → “creation of a span”, “Host, IP, Environment etc.”). Please adjust wording and punctuation so the docs read cleanly.
| //Process represents the service that resulted in the creation of span. | |
| message Process { | |
| //service_name represents the name of the service which resulted in creation of span. | |
| string service_name = 1; | |
| //tags describe the metadata about the service such as Host, IP, Environment etc. | |
| // Process represents the service that resulted in the creation of a span. | |
| message Process { | |
| // service_name represents the name of the service that resulted in the creation of a span. | |
| string service_name = 1; | |
| // tags describe metadata about the service, such as host, IP address, environment, etc. |
proto/api_v2/model.proto
Outdated
| (gogoproto.customtype) = "SpanID", | ||
| (gogoproto.customname) = "SpanID" | ||
| ]; | ||
| //It represents the name of the operation. |
There was a problem hiding this comment.
operation_name comment (“It represents…”) is vague and doesn’t mention the field name, and it’s missing a space after //. Suggest phrasing like // operation_name is ... (matching style in sampling.proto) and avoid ambiguous pronouns.
| //It represents the name of the operation. | |
| // operation_name is the name of the operation. |
proto/api_v2/model.proto
Outdated
| @@ -142,6 +164,7 @@ | |||
| (gogoproto.nullable) = false | |||
| ]; | |||
| } | |||
| //spans represent the spans within the trace. | |||
| repeated Span spans = 1; | |||
There was a problem hiding this comment.
Trace/field comments here are missing spaces after // (e.g., //Trace, //spans) and have minor grammar issues (“collection of spans with same trace ID”). Please adjust to consistent doc-comment style and wording.
proto/api_v2/model.proto
Outdated
| //Batch represents the group of spans sent to Jaeger over a wire. | ||
| message Batch { | ||
| repeated Span spans = 1; | ||
| Process process = 2 [ | ||
| (gogoproto.nullable) = true | ||
| ]; | ||
| //It represents the spans within the batch. | ||
| repeated Span spans = 1; | ||
| Process process = 2 [ | ||
| (gogoproto.nullable) = true | ||
| ]; |
There was a problem hiding this comment.
Batch message/field comments are missing spaces after //, and //It represents the spans... is vague. Also, if the intent is to document all fields, process (field 2) still has no comment.
proto/api_v2/model.proto
Outdated
| //Log is a time-stamped event which captures the incidents such as errors, | ||
| //lifecycle events that occurred during the execution of the span in key-value format. | ||
| message Log { |
There was a problem hiding this comment.
The Log message comment is missing spaces after // and reads a bit ungrammatically (e.g., “captures the incidents such as errors”). Consider rephrasing to a complete sentence and keep comment formatting consistent with the other proto files (space after //, wrapped lines).
| @@ -142,6 +164,7 @@ | |||
| (gogoproto.nullable) = false | |||
| ]; | |||
| } | |||
There was a problem hiding this comment.
The nested ProcessMapping fields are indented inconsistently compared to the rest of the file (extra leading spaces on string process_id / Process process). Since this hunk is already being edited, please normalize indentation to match surrounding messages (2-space indent).
proto/api_v2/model.proto
Outdated
| //DependencyLink represents the relationship between the two services. | ||
| message DependencyLink { | ||
| //parent represents the calling service. | ||
| string parent = 1; | ||
| //child represents the called service. | ||
| string child = 2; | ||
| //call_count represents the number of calls between the two services. | ||
| uint64 call_count = 3; |
There was a problem hiding this comment.
DependencyLink comments are missing spaces after // (e.g., //DependencyLink, //parent, etc.). Please update to // DependencyLink ... / // parent ... for consistency with the rest of proto/api_v2.
| string key = 1; | ||
| ValueType v_type = 2; | ||
| string v_str = 3; | ||
| bool v_bool = 4; | ||
| int64 v_int64 = 5; | ||
| double v_float64 = 6; | ||
| bytes v_binary = 7; |
There was a problem hiding this comment.
This change removes the file-level TODO to document all types/fields, but many fields below still have no comments (e.g., KeyValue.key, v_type, v_str, etc.). Either complete field-level documentation throughout this file or keep the TODO so it’s clear the documentation work is still incomplete.
proto/api_v2/model.proto
Outdated
| //SpanRef describes the reference from one span to another, it | ||
| //establishes relationship between the spans. |
There was a problem hiding this comment.
SpanRef comment has two issues: it’s missing a space after // and it’s a run-on sentence (“..., it establishes ...”). Please split into separate sentences and keep the comment format consistent.
| //SpanRef describes the reference from one span to another, it | |
| //establishes relationship between the spans. | |
| // SpanRef describes the reference from one span to another. | |
| // It establishes the relationship between the spans. |
proto/api_v2/model.proto
Outdated
| //trace_id represents the trace containing the referenced span. | ||
| bytes trace_id = 1 [ | ||
| (gogoproto.nullable) = false, | ||
| (gogoproto.customtype) = "TraceID", | ||
| (gogoproto.customname) = "TraceID" | ||
| ]; | ||
| //span_id represents the referenced span within the trace. | ||
| bytes span_id = 2 [ | ||
| (gogoproto.nullable) = false, | ||
| (gogoproto.customtype) = "SpanID", | ||
| (gogoproto.customname) = "SpanID" | ||
| ]; | ||
| //ref_type describes the type of relationship between the spans. | ||
| SpanRefType ref_type = 3; |
There was a problem hiding this comment.
Field comments here are missing a space after // (e.g., //trace_id) and don’t follow the style used elsewhere in proto/api_v2 (// trace_id ...). Please update these (and similar new field comments throughout the file) for consistent formatting.
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| string process_id = 1 [ | ||
| (gogoproto.customname) = "ProcessID" | ||
| ]; | ||
| Process process = 2 [ | ||
| ]; | ||
| Process process = 2 [ | ||
| (gogoproto.nullable) = false | ||
| ]; | ||
| ]; |
There was a problem hiding this comment.
In Trace.ProcessMapping, indentation is inconsistent with the rest of this file (e.g., SpanRef.trace_id options are indented 2 spaces, with option lines aligned under the [), and there’s an extra leading space before Process process. This makes the proto harder to read/maintain—please normalize indentation to match surrounding messages.
proto/api_v2/model.proto
Outdated
| string parent = 1; | ||
| // child represents the called service. | ||
| string child = 2; | ||
| //call_count represents the number of calls between the two services. |
There was a problem hiding this comment.
Missing space after // in the call_count field comment (//call_count ...). This looks like a typo/style issue and breaks consistency with the other comments in the file.
| //call_count represents the number of calls between the two services. | |
| // call_count represents the number of calls between the two services. |
proto/api_v2/model.proto
Outdated
| (gogoproto.stdtime) = true, | ||
| (gogoproto.nullable) = false | ||
| ]; | ||
| // duration represents how long did the span run. |
There was a problem hiding this comment.
Grammar in the new duration field comment is off (“how long did the span run”). Consider rephrasing to something like “duration represents how long the span ran” or “duration is the elapsed time of the span” for clarity.
| // duration represents how long did the span run. | |
| // duration represents how long the span ran. |
| @@ -112,36 +134,44 @@ message Span { | |||
| (gogoproto.nullable) = false, | |||
| (gogoproto.customtype) = "Flags" | |||
| ]; | |||
| // start_time represents the time when the span started. | |||
| google.protobuf.Timestamp start_time = 6 [ | |||
| (gogoproto.stdtime) = true, | |||
| (gogoproto.nullable) = false | |||
| ]; | |||
| // duration represents how long did the span run. | |||
| google.protobuf.Duration duration = 7 [ | |||
| (gogoproto.stdduration) = true, | |||
| (gogoproto.nullable) = false | |||
| ]; | |||
| // tags represents the metadata of the operation. | |||
| repeated KeyValue tags = 8 [ | |||
| (gogoproto.nullable) = false | |||
| ]; | |||
| // logs represents the timestamped events within the span. | |||
| repeated Log logs = 9 [ | |||
| (gogoproto.nullable) = false | |||
| ]; | |||
| // process represents the process which emitted the span. | |||
| Process process = 10; | |||
| string process_id = 11 [ | |||
| (gogoproto.customname) = "ProcessID" | |||
| ]; | |||
| repeated string warnings = 12; | |||
| } | |||
There was a problem hiding this comment.
The PR description/title says all types and fields are documented, but several fields in this file still have no comments (e.g., Log.fields, Span.references, Span.flags, Span.process_id, Span.warnings, Trace.process_map, Trace.warnings, and the fields inside ProcessMapping). Either add comments for the remaining fields or adjust the PR scope/description so it’s accurate.
proto/api_v2/model.proto
Outdated
| repeated string warnings = 12; | ||
| } | ||
|
|
||
| // Trace represents the collection of spans with same trace ID. |
There was a problem hiding this comment.
Minor grammar in the new Trace doc comment: “collection of spans with same trace ID” is missing an article. Consider “collection of spans with the same trace ID” for readability.
| // Trace represents the collection of spans with same trace ID. | |
| // Trace represents the collection of spans with the same trace ID. |
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proto/api_v2/model.proto
Outdated
| Process process = 2 [ | ||
| ]; | ||
| // process represents process definition along with service name and tags. | ||
| Process process = 2 [ |
There was a problem hiding this comment.
The ProcessMapping block has inconsistent indentation, and there is an extra leading space before Process process = 2 that hurts readability. Please align indentation with the surrounding proto style.
| Process process = 2 [ | |
| Process process = 2 [ |
proto/api_v2/model.proto
Outdated
| } | ||
| // The list of spans that belong to this trace. | ||
| repeated Span spans = 1; | ||
| // process_map represents the mapping of process identifiers to thier process definitons. |
There was a problem hiding this comment.
Typo(s) in the field comment: "thier" and "definitons" should be corrected (e.g., "their process definitions").
| // process_map represents the mapping of process identifiers to thier process definitons. | |
| // process_map represents the mapping of process identifiers to their process definitions. |
proto/api_v2/model.proto
Outdated
| // ProcessMapping represents the name and tags of the service. | ||
| message ProcessMapping { | ||
| string process_id = 1 [ | ||
| // process_id is a unique identifier to each process in the trace. |
There was a problem hiding this comment.
This comment is grammatically incorrect: "a unique identifier to each process" should be reworded (e.g., "a unique identifier for each process").
| // process_id is a unique identifier to each process in the trace. | |
| // process_id is a unique identifier for each process in the trace. |
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proto/api_v2/model.proto
Outdated
|
|
||
| // Trace represents the collection of spans with the same trace ID. | ||
| message Trace { | ||
| // ProcessMapping represents the name and tags of the service. |
There was a problem hiding this comment.
The comment for ProcessMapping appears inaccurate: the nested message is a mapping from process_id to a Process, not directly "the name and tags of the service". Consider updating the comment to describe it as an ID-to-Process mapping.
| // ProcessMapping represents the name and tags of the service. | |
| // ProcessMapping represents a single mapping entry from a process_id | |
| // to its Process definition (including service name and tags). |
proto/api_v2/model.proto
Outdated
| ]; | ||
| } | ||
|
|
||
| // Span represents the single operation performed within the trace. |
There was a problem hiding this comment.
"Span represents the single operation..." is slightly ungrammatical; consider "Span represents a single operation..." for clearer documentation.
| // Span represents the single operation performed within the trace. | |
| // Span represents a single operation performed within the trace. |
proto/api_v2/model.proto
Outdated
| // Log is a time-stamped event which captures the incidents such as errors, | ||
| // lifecycle events that occurred during the execution of the span in key-value format. |
There was a problem hiding this comment.
The Log message comment is grammatically unclear ("captures the incidents such as"). Consider rephrasing to something like "captures incidents such as errors or lifecycle events" to improve readability.
| // Log is a time-stamped event which captures the incidents such as errors, | |
| // lifecycle events that occurred during the execution of the span in key-value format. | |
| // Log is a time-stamped event which captures incidents such as errors or lifecycle | |
| // events that occur during the execution of the span in key-value format. |
proto/api_v2/model.proto
Outdated
|
|
||
| // Span represents the single operation performed within the trace. | ||
| message Span { | ||
| // trace_id identifies ID of the trace, all the spans within it share the same trace_id. |
There was a problem hiding this comment.
The comment for trace_id is awkwardly phrased ("identifies ID of the trace"). Consider rewording to "identifies the trace" / "is the trace ID" to make the sentence grammatical.
| // trace_id identifies ID of the trace, all the spans within it share the same trace_id. | |
| // trace_id is the ID of the trace; all the spans within it share the same trace_id. |
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proto/api_v2/model.proto
Outdated
| bytes v_binary = 7; | ||
| } | ||
|
|
||
| // Log is a time-stamped event which captures incidents such as errors or lifecycles |
There was a problem hiding this comment.
In the Log message comment, "lifecycles events" is grammatically incorrect; it should be "lifecycle events".
| // Log is a time-stamped event which captures incidents such as errors or lifecycles | |
| // Log is a time-stamped event which captures incidents such as errors or lifecycle |
proto/api_v2/model.proto
Outdated
| string process_id = 11 [ | ||
| (gogoproto.customname) = "ProcessID" | ||
| ]; | ||
| // Warnings generation during span creation. |
There was a problem hiding this comment.
The comment on the Span.warnings field reads "Warnings generation"; this should be "Warnings generated" (or similar) to match the intended meaning.
| // Warnings generation during span creation. | |
| // Warnings generated during span creation. |
| string process_id = 1 [ | ||
| (gogoproto.customname) = "ProcessID" | ||
| ]; | ||
| Process process = 2 [ | ||
| ]; | ||
| // process represents process definition along with service name and tags. | ||
| Process process = 2 [ | ||
| (gogoproto.nullable) = false | ||
| ]; | ||
| ]; |
There was a problem hiding this comment.
Indentation inside the ProcessMapping field option blocks is inconsistent with the rest of this file (compare to Span.trace_id options). Align the option lines and closing bracket indentation to the same level used elsewhere to keep formatting consistent and avoid noisy diffs from auto-formatters.
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
proto/api_v2/model.proto:75
Lognow has a message-level doc comment, but its fields (timestampandfields) are still undocumented. Since the stated goal is to document all types and fields, please add field-level comments for these two fields (see style in proto/api_v2/sampling.proto where each field is documented).
// Log is a time-stamped event which captures incidents such as errors or lifecycle
// events that occur during the execution of the span in key-value format.
message Log {
google.protobuf.Timestamp timestamp = 1 [
(gogoproto.stdtime) = true,
(gogoproto.nullable) = false
];
repeated KeyValue fields = 2 [
(gogoproto.nullable) = false
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proto/api_v2/model.proto
Outdated
| // Process represents the service that resulted in the creation of a span. | ||
| message Process { | ||
| // service_name represents the name of the service that resulted in the creation of a span. | ||
| string service_name = 1; | ||
| // tags describe the metadata about the service, such as host, IP address, environment etc. |
There was a problem hiding this comment.
The new Process doc comment says it "represents the service" but the message includes both service_name and tags and is referenced elsewhere as the process that emitted the span. Consider updating the message and service_name comments to describe the emitting process/service (service name + process-level tags) to avoid under-documenting/misleading API consumers.
| // Process represents the service that resulted in the creation of a span. | |
| message Process { | |
| // service_name represents the name of the service that resulted in the creation of a span. | |
| string service_name = 1; | |
| // tags describe the metadata about the service, such as host, IP address, environment etc. | |
| // Process represents the process (and associated service) that emitted a span, including | |
| // its logical service name and process-level tags. | |
| message Process { | |
| // service_name represents the logical name of the service for the process that emitted the span. | |
| string service_name = 1; | |
| // tags describe process-level metadata for the emitting process/service, such as host, IP address, | |
| // environment, and other attributes that apply to the process as a whole. |
proto/api_v2/model.proto
Outdated
| ]; | ||
| // operation_name is the name of the operation. | ||
| string operation_name = 3; | ||
| // References to other spans. |
There was a problem hiding this comment.
Field doc style is inconsistent with other protos in this repo: many fields use comments that start with the field name (e.g. samplingRate is ... in proto/api_v2/sampling.proto), which improves generated docs. For references, the comment could start with references rather than a generic phrase so it's clearer in generated output.
| // References to other spans. | |
| // references are links to other spans. |
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
Signed-off-by: chethanm99 <chethanm1399@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -67,85 +76,116 @@ message Log { | |||
| ]; | |||
| } | |||
|
|
|||
| // SpanRefType describes the relationship between the two spans. | |||
| enum SpanRefType { | |||
| CHILD_OF = 0; | |||
| FOLLOWS_FROM = 1; | |||
| }; | |||
There was a problem hiding this comment.
PR description says all types/fields are documented, but some newly touched parts are still missing field/enum-value documentation (e.g., Log.timestamp, Log.fields, and the ValueType/SpanRefType enum values). Consider adding brief comments for these to fully satisfy the TODO/PR goal.
| // process represents the process which emitted the span. | ||
| Process process = 10; | ||
| // Identifier of the process that emits the span. | ||
| string process_id = 11 [ | ||
| (gogoproto.customname) = "ProcessID" | ||
| ]; | ||
| // Warnings generated during span creation. | ||
| repeated string warnings = 12; |
There was a problem hiding this comment.
The new comments on Span.process and Span.process_id don’t explain how they relate to Batch.process and Trace.process_map (there’s an existing note above Batch that span.Process takes priority). To avoid misleading API docs, clarify whether Span.process is optional and when consumers should use process_id to resolve a process definition from Trace.process_map.
| // process_id is a unique identifier for each process in the trace. | ||
| string process_id = 1 [ | ||
| (gogoproto.customname) = "ProcessID" | ||
| ]; | ||
| Process process = 2 [ | ||
| ]; | ||
| // process represents process definition along with service name and tags. | ||
| Process process = 2 [ | ||
| (gogoproto.nullable) = false | ||
| ]; | ||
| ]; |
There was a problem hiding this comment.
Indentation inside Trace.ProcessMapping is inconsistent with the rest of this proto (option lines under other fields are aligned with two extra spaces, but here they’re indented further). Please align the option indentation/closing bracket formatting to match surrounding fields for readability and to minimize noisy diffs in future formatting runs.
| // their own instances of Process, with span.Process taking priority | ||
| // over batch.Process. | ||
|
|
||
| // Batch represents the group of spans sent to Jaeger over a wire. |
There was a problem hiding this comment.
Minor grammar: “sent to Jaeger over a wire” reads awkwardly; “over the wire” (or “on the wire”) is the standard phrasing in the rest of the repo/industry docs.
| // Batch represents the group of spans sent to Jaeger over a wire. | |
| // Batch represents the group of spans sent to Jaeger over the wire. |
Description of the changes
TODO in this file says to document all the fields and types. So I have added comments describing each field and types.
Checklist
make lint testAI Usage in this PR (choose one)
See AI Usage Policy.