From aecc53bddee8f8bcd1a35b7478996ae2e96eab1a Mon Sep 17 00:00:00 2001 From: adityachopra29 Date: Sat, 18 Jan 2025 02:25:28 +0530 Subject: [PATCH 1/6] clean up Makefile, fix ci accordingly Signed-off-by: adityachopra29 --- .github/workflows/ci-lint-test.yml | 2 +- Makefile | 33 +++++++++++--- Makefile.Protobuf.mk | 71 ------------------------------ 3 files changed, 28 insertions(+), 78 deletions(-) diff --git a/.github/workflows/ci-lint-test.yml b/.github/workflows/ci-lint-test.yml index 8f3a607..6373653 100644 --- a/.github/workflows/ci-lint-test.yml +++ b/.github/workflows/ci-lint-test.yml @@ -31,4 +31,4 @@ jobs: submodules: recursive - name: Verify Protobuf types are up to date - run: make new-proto && git diff --name-status --exit-code \ No newline at end of file + run: make proto && git diff --name-status --exit-code \ No newline at end of file diff --git a/Makefile b/Makefile index e6e0b91..1dd34ba 100644 --- a/Makefile +++ b/Makefile @@ -71,7 +71,8 @@ PROTO_GOGO_MAPPINGS := $(shell echo \ Mmodel.proto=github.com/jaegertracing/jaeger-idl/model/v1 \ | sed 's/ //g') -PROTO_GEN_GO_DIR ?= proto-gen-go +PROTO_GEN_GO_DIR ?= proto-gen +PROTO_GEN_GO_DIR_OLD ?= proto-gen-go #temporary arrangement, will be removed once api_v3 is migrated to idl PROTO_GEN_PYTHON_DIR ?= proto-gen-python PROTO_GEN_JAVA_DIR ?= proto-gen-java PROTO_GEN_JS_DIR ?= proto-gen-js @@ -83,9 +84,9 @@ PROTO_GEN_CSHARP_DIR ?= proto-gen-csharp PROTOC_WITHOUT_GRPC_common := $(PROTOC) \ $(PROTO_INCLUDES) \ - --gogo_out=plugins=grpc,$(PROTO_GOGO_MAPPINGS):$(PWD)/${PROTO_GEN_GO_DIR} \ --python_out=${PROTO_GEN_PYTHON_DIR} \ - --js_out=${PROTO_GEN_JS_DIR} + --js_out=${PROTO_GEN_JS_DIR} \ + # --gogo_out=plugins=grpc,$(PROTO_GOGO_MAPPINGS):$(PWD)/${PROTO_GEN_GO_DIR} \ ifeq ($(shell uname -m),arm64) PROTOC_WITHOUT_GRPC := $(PROTOC_WITHOUT_GRPC_common) @@ -126,8 +127,22 @@ else SED=sed endif -# import other Makefiles after the variables are defined -include Makefile.Protobuf.mk +# Macro to compile Protobuf $(2) into directory $(1). $(3) can provide additional flags. +# DO NOT DELETE EMPTY LINE at the end of the macro, it's required to separate commands. +# Arguments: +# $(1) - output directory +# $(2) - path to the .proto file +# $(3) - additional flags to pass to protoc, e.g. extra -Ixxx +# $(4) - additional options to pass to gogo plugin +define proto_compile + $(call print_caption, "Processing $(2) --> $(1)") + + $(PROTOC) \ + $(PROTO_INCLUDES) \ + --gogo_out=plugins=grpc,$(strip $(4)),$(PROTO_GOGO_MAPPINGS):$(PWD)/$(strip $(1)) \ + $(3) $(2) + +endef .PHONY: test-ci test-ci: @@ -155,6 +170,12 @@ proto-api-v2: proto/api_v2/collector.proto \ proto/api_v2/sampling.proto + mkdir -p ${PROTO_GEN_GO_DIR}/${API_V2_PATH} + $(call proto_compile, model/v1, proto/api_v2/model.proto) + $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/query.proto) + $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/collector.proto) + $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/sampling.proto) + .PHONY: proto-api-v3 proto-api-v3: # API v3 @@ -163,7 +184,7 @@ proto-api-v3: # GRPC gateway $(PROTOC) \ $(PROTO_INCLUDES) \ - --grpc-gateway_out=logtostderr=true,grpc_api_configuration=proto/api_v3/query_service_http.yaml,$(PROTO_GOGO_MAPPINGS):${PROTO_GEN_GO_DIR} \ + --grpc-gateway_out=logtostderr=true,grpc_api_configuration=proto/api_v3/query_service_http.yaml,$(PROTO_GOGO_MAPPINGS):${PROTO_GEN_GO_DIR_OLD} \ proto/api_v3/query_service.proto # Swagger $(PROTOC) \ diff --git a/Makefile.Protobuf.mk b/Makefile.Protobuf.mk index bd7f62f..e69de29 100644 --- a/Makefile.Protobuf.mk +++ b/Makefile.Protobuf.mk @@ -1,71 +0,0 @@ -# Copyright (c) 2023 The Jaeger Authors. -# SPDX-License-Identifier: Apache-2.0 - -# Generate gogo, swagger, go-validators, gRPC-storage-plugin output. -# -# -I declares import folders, in order of importance. This is how proto resolves the protofile imports. -# It will check for the protofile relative to each of thesefolders and use the first one it finds. -# -# --gogo_out generates GoGo Protobuf output with gRPC plugin enabled. -# --govalidators_out generates Go validation files for our messages types, if specified. -# -# The lines starting with Mgoogle/... are proto import replacements, -# which cause the generated file to import the specified packages -# instead of the go_package's declared by the imported protof files. -# - -DOCKER_PROTOBUF_VERSION=0.5.0 -DOCKER_PROTOBUF=jaegertracing/protobuf:$(DOCKER_PROTOBUF_VERSION) -# PROTOC := ${DOCKER} run --rm -u ${shell id -u} -v${PWD}:${PWD} -w${PWD} ${DOCKER_PROTOBUF} --proto_path=${PWD} - -PATCHED_OTEL_PROTO_DIR = proto-gen/.patched-otel-proto - -# The source directory for OTLP Protobufs from the sub-sub-module. -OTEL_PROTO_SRC_DIR=opentelemetry-proto/opentelemetry/proto - -# Find all OTEL .proto files, remove leading path (only keep relevant namespace dirs). -OTEL_PROTO_FILES=$(subst $(OTEL_PROTO_SRC_DIR)/,,\ - $(shell ls $(OTEL_PROTO_SRC_DIR)/{common,resource,trace}/v1/*.proto)) - -# Macro to execute a command passed as argument. -# DO NOT DELETE EMPTY LINE at the end of the macro, it's required to separate commands. -define exec-command -$(1) - -endef - -# DO NOT DELETE EMPTY LINE at the end of the macro, it's required to separate commands. -define print_caption - @echo "🏗️ " - @echo "🏗️ " $1 - @echo "🏗️ " - -endef - -# Macro to compile Protobuf $(2) into directory $(1). $(3) can provide additional flags. -# DO NOT DELETE EMPTY LINE at the end of the macro, it's required to separate commands. -# Arguments: -# $(1) - output directory -# $(2) - path to the .proto file -# $(3) - additional flags to pass to protoc, e.g. extra -Ixxx -# $(4) - additional options to pass to gogo plugin -define proto_compile - $(call print_caption, "Processing $(2) --> $(1)") - - $(PROTOC) \ - $(PROTO_INCLUDES) \ - --gogo_out=plugins=grpc,$(strip $(4)),$(PROTO_GOGO_MAPPINGS):$(PWD)/$(strip $(1)) \ - $(3) $(2) - -endef - -.PHONY: new-proto -new-proto: new-proto-api-v2 - -.PHONY: new-proto-api-v2 -new-proto-api-v2: - mkdir -p proto-gen/api_v2 - $(call proto_compile, model/v1, proto/api_v2/model.proto) - $(call proto_compile, proto-gen/api_v2, proto/api_v2/query.proto) - $(call proto_compile, proto-gen/api_v2, proto/api_v2/collector.proto) - $(call proto_compile, proto-gen/api_v2, proto/api_v2/sampling.proto) From 9ac0876749b2f08c2a742aed85e120661bfe367f Mon Sep 17 00:00:00 2001 From: adityachopra29 Date: Mon, 20 Jan 2025 18:44:10 +0530 Subject: [PATCH 2/6] clean Makefile, make module Signed-off-by: adityachopra29 --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1dd34ba..7f78045 100644 --- a/Makefile +++ b/Makefile @@ -79,6 +79,8 @@ PROTO_GEN_JS_DIR ?= proto-gen-js PROTO_GEN_CPP_DIR ?= proto-gen-cpp PROTO_GEN_CSHARP_DIR ?= proto-gen-csharp +API_V2_PATH ?= api_v2 + # The jaegertracing/protobuf container image does not # include Java/C#/C++ plugins for Apple Silicon (arm64). @@ -86,7 +88,6 @@ PROTOC_WITHOUT_GRPC_common := $(PROTOC) \ $(PROTO_INCLUDES) \ --python_out=${PROTO_GEN_PYTHON_DIR} \ --js_out=${PROTO_GEN_JS_DIR} \ - # --gogo_out=plugins=grpc,$(PROTO_GOGO_MAPPINGS):$(PWD)/${PROTO_GEN_GO_DIR} \ ifeq ($(shell uname -m),arm64) PROTOC_WITHOUT_GRPC := $(PROTOC_WITHOUT_GRPC_common) @@ -154,6 +155,7 @@ proto: proto-prepare proto-api-v2 proto-api-v3 .PHONY: proto-prepare proto-prepare: mkdir -p ${PROTO_GEN_GO_DIR} \ + ${PROTO_GEN_GO_DIR_OLD} \ ${PROTO_GEN_JAVA_DIR} \ ${PROTO_GEN_PYTHON_DIR} \ ${PROTO_GEN_JS_DIR} \ From a42436d5d70e92ce759d033241c0fed9ec7549c7 Mon Sep 17 00:00:00 2001 From: adityachopra29 Date: Mon, 20 Jan 2025 23:14:10 +0530 Subject: [PATCH 3/6] create dot-directory for polyglot files, suggested changes Signed-off-by: adityachopra29 --- .gitignore | 1 + Makefile | 80 +++++++++++++++++++++++++++++++----------------------- 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index 67959fb..f0febcc 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .idea/ gen-*/ proto-gen-*/ +.proto-gen-polyglot/ diff --git a/Makefile b/Makefile index 7f78045..3d929d1 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,7 @@ swagger-validate: .PHONY: clean clean: rm -rf *gen-* || true + rm -rf .*gen-* || true .PHONY: thrift thrift: thrift-image clean $(THRIFT_FILES) @@ -72,12 +73,13 @@ PROTO_GOGO_MAPPINGS := $(shell echo \ | sed 's/ //g') PROTO_GEN_GO_DIR ?= proto-gen -PROTO_GEN_GO_DIR_OLD ?= proto-gen-go #temporary arrangement, will be removed once api_v3 is migrated to idl -PROTO_GEN_PYTHON_DIR ?= proto-gen-python -PROTO_GEN_JAVA_DIR ?= proto-gen-java -PROTO_GEN_JS_DIR ?= proto-gen-js -PROTO_GEN_CPP_DIR ?= proto-gen-cpp -PROTO_GEN_CSHARP_DIR ?= proto-gen-csharp +POLYGLOT_DIR_ROOT ?= .proto-gen-polyglot +PROTO_GEN_GO_DIR_POLYGLOT ?= $(POLYGLOT_DIR_ROOT)/proto-gen-go +PROTO_GEN_PYTHON_DIR_POLYGLOT ?= $(POLYGLOT_DIR_ROOT)/proto-gen-python +PROTO_GEN_JAVA_DIR_POLYGLOT ?= $(POLYGLOT_DIR_ROOT)/proto-gen-java +PROTO_GEN_JS_DIR_POLYGLOT ?= $(POLYGLOT_DIR_ROOT)/proto-gen-js +PROTO_GEN_CPP_DIR_POLYGLOT ?= $(POLYGLOT_DIR_ROOT)/proto-gen-cpp +PROTO_GEN_CSHARP_DIR_POLYGLOT ?= $(POLYGLOT_DIR_ROOT)/proto-gen-csharp API_V2_PATH ?= api_v2 @@ -86,35 +88,36 @@ API_V2_PATH ?= api_v2 PROTOC_WITHOUT_GRPC_common := $(PROTOC) \ $(PROTO_INCLUDES) \ - --python_out=${PROTO_GEN_PYTHON_DIR} \ - --js_out=${PROTO_GEN_JS_DIR} \ + --gogo_out=plugins=grpc,$(PROTO_GOGO_MAPPINGS):$(PWD)/${PROTO_GEN_GO_DIR_POLYGLOT} \ + --python_out=${PROTO_GEN_PYTHON_DIR_POLYGLOT} \ + --js_out=${PROTO_GEN_JS_DIR_POLYGLOT} ifeq ($(shell uname -m),arm64) PROTOC_WITHOUT_GRPC := $(PROTOC_WITHOUT_GRPC_common) else PROTOC_WITHOUT_GRPC := $(PROTOC_WITHOUT_GRPC_common) \ - --java_out=${PROTO_GEN_JAVA_DIR} \ - --cpp_out=${PROTO_GEN_CPP_DIR} \ - --csharp_out=base_namespace:${PROTO_GEN_CSHARP_DIR} + --java_out=${PROTO_GEN_JAVA_DIR_POLYGLOT} \ + --cpp_out=${PROTO_GEN_CPP_DIR_POLYGLOT} \ + --csharp_out=base_namespace:${PROTO_GEN_CSHARP_DIR_POLYGLOT} endif PROTOC_WITH_GRPC_common := $(PROTOC_WITHOUT_GRPC) \ - --grpc-python_out=${PROTO_GEN_PYTHON_DIR} \ - --grpc-js_out=${PROTO_GEN_JS_DIR} + --grpc-python_out=${PROTO_GEN_PYTHON_DIR_POLYGLOT} \ + --grpc-js_out=${PROTO_GEN_JS_DIR_POLYGLOT} ifeq ($(shell uname -m),arm64) PROTOC_WITH_GRPC := $(PROTOC_WITH_GRPC_common) else PROTOC_WITH_GRPC := $(PROTOC_WITH_GRPC_common) \ - --grpc-java_out=${PROTO_GEN_JAVA_DIR} \ - --grpc-cpp_out=${PROTO_GEN_CPP_DIR} \ - --grpc-csharp_out=${PROTO_GEN_CSHARP_DIR} + --grpc-java_out=${PROTO_GEN_JAVA_DIR_POLYGLOT} \ + --grpc-cpp_out=${PROTO_GEN_CPP_DIR_POLYGLOT} \ + --grpc-csharp_out=${PROTO_GEN_CSHARP_DIR_POLYGLOT} endif PROTOC_INTERNAL := $(PROTOC) \ $(PROTO_INCLUDES) \ - --csharp_out=internal_access,base_namespace:${PROTO_GEN_CSHARP_DIR} \ - --python_out=${PROTO_GEN_PYTHON_DIR} + --csharp_out=internal_access,base_namespace:${PROTO_GEN_CSHARP_DIR_POLYGLOT} \ + --python_out=${PROTO_GEN_PYTHON_DIR_POLYGLOT} GO=go GOOS ?= $(shell $(GO) env GOOS) @@ -150,20 +153,34 @@ test-ci: go test -v -coverprofile=coverage.txt ./... .PHONY: proto -proto: proto-prepare proto-api-v2 proto-api-v3 +proto: proto-prepare proto-api-v2 + +.PHONY: proto-all +proto-all: proto-prepare-all proto-api-v2-all proto-api-v3-all + +.PHONY: proto-prepare-all +proto-prepare-all: + mkdir -p ${PROTO_GEN_GO_DIR_POLYGLOT} \ + ${PROTO_GEN_JAVA_DIR_POLYGLOT} \ + ${PROTO_GEN_PYTHON_DIR_POLYGLOT} \ + ${PROTO_GEN_JS_DIR_POLYGLOT} \ + ${PROTO_GEN_CPP_DIR_POLYGLOT} \ + ${PROTO_GEN_CSHARP_DIR_POLYGLOT} .PHONY: proto-prepare proto-prepare: - mkdir -p ${PROTO_GEN_GO_DIR} \ - ${PROTO_GEN_GO_DIR_OLD} \ - ${PROTO_GEN_JAVA_DIR} \ - ${PROTO_GEN_PYTHON_DIR} \ - ${PROTO_GEN_JS_DIR} \ - ${PROTO_GEN_CPP_DIR} \ - ${PROTO_GEN_CSHARP_DIR} + mkdir -p ${PROTO_GEN_GO_DIR} .PHONY: proto-api-v2 proto-api-v2: + mkdir -p ${PROTO_GEN_GO_DIR}/${API_V2_PATH} + $(call proto_compile, model/v1, proto/api_v2/model.proto) + $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/query.proto) + $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/collector.proto) + $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/sampling.proto) + +.PHONY: proto-api-v2-all +proto-api-v2-all: $(PROTOC_WITHOUT_GRPC) \ proto/api_v2/model.proto @@ -172,21 +189,16 @@ proto-api-v2: proto/api_v2/collector.proto \ proto/api_v2/sampling.proto - mkdir -p ${PROTO_GEN_GO_DIR}/${API_V2_PATH} - $(call proto_compile, model/v1, proto/api_v2/model.proto) - $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/query.proto) - $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/collector.proto) - $(call proto_compile, ${PROTO_GEN_GO_DIR}/${API_V2_PATH}, proto/api_v2/sampling.proto) -.PHONY: proto-api-v3 -proto-api-v3: +.PHONY: proto-api-v3-all +proto-api-v3-all: # API v3 $(PROTOC_WITH_GRPC) \ proto/api_v3/query_service.proto # GRPC gateway $(PROTOC) \ $(PROTO_INCLUDES) \ - --grpc-gateway_out=logtostderr=true,grpc_api_configuration=proto/api_v3/query_service_http.yaml,$(PROTO_GOGO_MAPPINGS):${PROTO_GEN_GO_DIR_OLD} \ + --grpc-gateway_out=logtostderr=true,grpc_api_configuration=proto/api_v3/query_service_http.yaml,$(PROTO_GOGO_MAPPINGS):${PROTO_GEN_GO_DIR_POLYGLOT} \ proto/api_v3/query_service.proto # Swagger $(PROTOC) \ From 3d475d544ff3ac47c19a14ec32fbb8b4ad171886 Mon Sep 17 00:00:00 2001 From: adityachopra29 Date: Mon, 20 Jan 2025 23:20:06 +0530 Subject: [PATCH 4/6] delete Makefile.Protobuf.mk Signed-off-by: adityachopra29 --- Makefile.Protobuf.mk | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 Makefile.Protobuf.mk diff --git a/Makefile.Protobuf.mk b/Makefile.Protobuf.mk deleted file mode 100644 index e69de29..0000000 From c97bab9aade0edfe4dcd81a57955ccb6ad2a16eb Mon Sep 17 00:00:00 2001 From: adityachopra29 Date: Mon, 20 Jan 2025 23:51:09 +0530 Subject: [PATCH 5/6] fix Signed-off-by: adityachopra29 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3d929d1..007ba16 100644 --- a/Makefile +++ b/Makefile @@ -214,7 +214,7 @@ proto-api-v3-all: gogoproto/gogo.proto .PHONY: proto-zipkin -proto-zipkin: +proto-zipkin: proto-prepare-all $(PROTOC_WITHOUT_GRPC) \ proto/zipkin.proto From afece2f14d967406faf981ca8ed288fc95730212 Mon Sep 17 00:00:00 2001 From: adityachopra29 Date: Wed, 22 Jan 2025 04:34:51 +0530 Subject: [PATCH 6/6] suggested changes Signed-off-by: adityachopra29 --- .gitignore | 1 - Makefile | 12 ++++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index f0febcc..6eae0f3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ .idea/ gen-*/ -proto-gen-*/ .proto-gen-polyglot/ diff --git a/Makefile b/Makefile index 007ba16..3c7dc3c 100644 --- a/Makefile +++ b/Makefile @@ -131,6 +131,14 @@ else SED=sed endif +# DO NOT DELETE EMPTY LINE at the end of the macro, it's required to separate commands. +define print_caption + @echo "🏗️ " + @echo "🏗️ " $1 + @echo "🏗️ " + +endef + # Macro to compile Protobuf $(2) into directory $(1). $(3) can provide additional flags. # DO NOT DELETE EMPTY LINE at the end of the macro, it's required to separate commands. # Arguments: @@ -152,10 +160,10 @@ endef test-ci: go test -v -coverprofile=coverage.txt ./... -.PHONY: proto +# proto target is used to generate source code that is released as part of this library proto: proto-prepare proto-api-v2 -.PHONY: proto-all +# proto-all target is used to generate code for all languages as a validation step. proto-all: proto-prepare-all proto-api-v2-all proto-api-v3-all .PHONY: proto-prepare-all