change to use field option

Signed-off-by: xil <fridalu66@gmail.com>

Author: lucy66hw

CHANGELOG.md

@@ -7,7 +7,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Add compatibility matrix ([#373](https://github.com/opensearch-project/opensearch-protobufs/pull/373))
 - Enhance spec-filter to support x-operation-group filtering and merge parameters across operations in same x-operation-group instead of selecting by max parameters ([#374](https://github.com/opensearch-project/opensearch-protobufs/pull/374))
 - Add BoostingQuery and SimpleQueryString protos ([#376](https://github.com/opensearch-project/opensearch-protobufs/pull/376))
-- Add @grpc_only annotation to preserve manually-maintained protobuf fields ([#377](https://github.com/opensearch-project/opensearch-protobufs/pull/377))
+- Add tooling_skip field option to preserve manually-maintained protobuf fields ([#378](https://github.com/opensearch-project/opensearch-protobufs/pull/378))
 
 ### Changed
 

protos/schemas/options.proto

@@ -0,0 +1,14 @@
+syntax = "proto3";
+package org.opensearch.protobufs;
+
+import "google/protobuf/descriptor.proto";
+
+option go_package = "github.com/opensearch-project/opensearch-protobufs/go/opensearchpb";
+option java_multiple_files = true;
+option java_outer_classname = "OptionsProto";
+option java_package = "org.opensearch.protobufs";
+
+extend google.protobuf.FieldOptions {
+  // Mark a field to be skipped by tooling. These fields are manually maintained and won't be marked as deprecated.
+  optional bool tooling_skip = 50001;
+}

tools/proto-convert/src/config/protobuf-schema-template/partial_header.mustache

@@ -4,6 +4,8 @@
 syntax = "proto3";
 package org.opensearch.protobufs;
 
+import "protos/schemas/options.proto";
+
 option go_package = "github.com/opensearch-project/opensearch-protobufs/go/opensearchpb";
 option java_multiple_files = true;
 option java_outer_classname = "CommonProto";

tools/proto-convert/src/postprocessing/CompatibilityMerger.ts

@@ -13,7 +13,7 @@ import {
 import { CompatibilityReporter, formatField } from './CompatibilityReporter';
 
 const DEPRECATED: Annotation = { name: 'deprecated', value: 'true' };
-const GRPC_ONLY_TAG = '@grpc_only';
+const TOOLING_SKIP: Annotation = { name: '(tooling_skip)', value: 'true' };
 
 /**
  * Extract base name
@@ -43,15 +43,14 @@ function isDeprecated(item: HasAnnotations): boolean {
     ) ?? false;
 }
 
-/** Type with comment field */
-type HasComment = { comment?: string };
-
 /**
- * Check if an item has @grpc_only tag in its comment.
- * Fields with this tag are manually maintained for gRPC and won't be deprecated.
+ * Check if an item has tooling_skip option set to true.
+ * Fields with this option are manually maintained and won't be deprecated.
  */
-function isGrpcOnly(item: HasComment): boolean {
-    return item.comment?.includes(GRPC_ONLY_TAG) ?? false;
+function hasToolingSkip(item: HasAnnotations): boolean {
+    return item.annotations?.some(a =>
+        a.name === TOOLING_SKIP.name && a.value === TOOLING_SKIP.value
+    ) ?? false;
 }
 
 /**
@@ -126,8 +125,7 @@ function mergeField(
             }
         }
     } else {
-        if (isGrpcOnly(sourceField)) {
-            // Keep gRPC-only field as-is, don't deprecate it
+        if (hasToolingSkip(sourceField)) {
             return sourceField;
         }
         reporter?.addFieldChange({

tools/proto-convert/test/postprocessing/CompatibilityMerger.test.ts

@@ -276,14 +276,14 @@ describe('mergeMessage', () => {
             expect(deprecatedOptions).toHaveLength(1);
         });
 
-        it('should keep fields with @grpc_only comment without deprecating', () => {
+        it('should keep fields with tooling_skip option without deprecating', () => {
             const source: ProtoMessage = {
                 name: 'TestMessage',
                 fields: [{
-                    name: 'grpc_field',
+                    name: 'skip_field',
                     type: 'string',
                     number: 1,
-                    comment: '@grpc_only'
+                    annotations: [{ name: '(tooling_skip)', value: 'true' }]
                 }, {
                     name: 'normal_field',
                     type: 'string',
@@ -301,21 +301,22 @@ describe('mergeMessage', () => {
 
             const result = mergeMessage(source, upcoming);
 
-            // grpc_field should remain without deprecated annotation
+            // skip_field should remain without deprecated annotation added
             expect(result.fields).toHaveLength(2);
-            const grpcField = result.fields.find(f => f.name === 'grpc_field');
-            expect(grpcField).toBeDefined();
-            expect(grpcField!.annotations).toBeUndefined();
+            const skipField = result.fields.find(f => f.name === 'skip_field');
+            expect(skipField).toBeDefined();
+            // Should only have tooling_skip, not deprecated
+            expect(skipField!.annotations).not.toContainEqual({ name: 'deprecated', value: 'true' });
         });
 
-        it('should not report @grpc_only fields as deprecated', () => {
+        it('should not report tooling_skip fields as deprecated', () => {
             const source: ProtoMessage = {
                 name: 'TestMessage',
                 fields: [{
-                    name: 'grpc_field',
+                    name: 'skip_field',
                     type: 'string',
                     number: 1,
-                    comment: '@grpc_only'
+                    annotations: [{ name: '(tooling_skip)', value: 'true' }]
                 }]
             };
             const upcoming: ProtoMessage = {
@@ -327,18 +328,18 @@ describe('mergeMessage', () => {
             mergeMessage(source, upcoming, reporter);
 
             const markdown = reporter.toMarkdown();
-            // Should not report deprecation for @grpc_only fields
+            // Should not report deprecation for tooling_skip fields
             expect(markdown).toContain('No changes detected');
         });
 
-        it('should keep @grpc_only field if it exists in upcoming', () => {
+        it('should keep tooling_skip field if it exists in upcoming', () => {
             const source: ProtoMessage = {
                 name: 'TestMessage',
                 fields: [{
                     name: 'field_name',
                     type: 'string',
                     number: 1,
-                    comment: '@grpc_only'
+                    annotations: [{ name: '(tooling_skip)', value: 'true' }]
                 }]
             };
             const upcoming: ProtoMessage = {
@@ -356,18 +357,18 @@ describe('mergeMessage', () => {
             expect(result.fields[0].name).toBe('field_name');
         });
 
-        it('should assign new field number after max including @grpc_only field', () => {
+        it('should assign new field number after max including tooling_skip field', () => {
             const source: ProtoMessage = {
                 name: 'TestMessage',
                 fields: [{
                     name: 'existing_field',
                     type: 'string',
                     number: 1
                 }, {
-                    name: 'grpc_field',
+                    name: 'skip_field',
                     type: 'string',
                     number: 5,  // This is the max field number
-                    comment: '@grpc_only'
+                    annotations: [{ name: '(tooling_skip)', value: 'true' }]
                 }]
             };
             const upcoming: ProtoMessage = {
@@ -385,9 +386,9 @@ describe('mergeMessage', () => {
 
             const result = mergeMessage(source, upcoming);
 
-            // grpc_field is kept, new_field is added
+            // skip_field is kept, new_field is added
             expect(result.fields).toHaveLength(3);
-            expect(result.fields.find(f => f.name === 'grpc_field')).toBeDefined();
+            expect(result.fields.find(f => f.name === 'skip_field')).toBeDefined();
 
             const newField = result.fields.find(f => f.name === 'new_field');
             expect(newField).toBeDefined();