Skip to content

Commit 0021ab0

Browse files
authored
Rework proto generation (#1371)
* Rework proto generation The changes here are: - Fix the default goal (using "default" target is not doing it). - Bail out with a useful message if proto submodule is not checked out. - Replace the use of docker image with downloading the protoc binary and building the gogofast plugin ourselves. This gives us a greater control over the invocation of protoc. - Use rsync to copy the generated code, instead of pax. Pax did not work for me (it was complaining about the unknown -0 flag). The control over the protoc invocation will be useful later, when we will want to generate a code with data structures in one place and the collector code elsewhere. The collector code may or may not depend on gRPC, but data structures have no need for it. This split will happen when we move the gRPC code out of the OTLP exporter module into a submodule. Getting rid of docker has the upside that the generated files do not belong to root, so there is no hassle of changing the ownership of the files, and it is not requires to use sudo for the `clean` target. And not using docker is faster. The downside of this work is that it depends on more tools: rsync, unzip and wget. I can only hope that macOS users have those tools too, and that those tools are invoked the same. * Update protogen workflow
1 parent 787e3f4 commit 0021ab0

File tree

5 files changed

+104
-41
lines changed

5 files changed

+104
-41
lines changed

.github/workflows/protogen.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ jobs:
1414
- uses: actions/setup-go@v2
1515
with:
1616
go-version: '^1.14.0'
17-
- run: sudo apt-get -y install pax
18-
- run: make -f Makefile.proto protobuf
17+
- run: sudo apt-get -y install rsync wget unzip
18+
- run: make -f Makefile.proto protobuf clean
1919
- uses: stefanzweifel/git-auto-commit-action@v4
2020
id: commit-changes
2121
with:

Makefile.proto

+96-39
Original file line numberDiff line numberDiff line change
@@ -13,60 +13,117 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515
#
16-
# This Makefile.proto has rules to generate *.pb.go files in
17-
# `exporters/otlp/internal/opentelemetry-proto-gen` from the .proto files in
18-
# `exporters/otlp/internal/opentelemetry-proto` using protoc with a go plugin.
16+
# This Makefile.proto has rules to generate go code for otlp
17+
# exporter. It does it by copying the proto files from
18+
# `exporters/otlp/internal/opentelemetry-proto` (which is a
19+
# submodule that needs to be checked out) into `gen/proto`, changing
20+
# the go_package option to a valid string, generating the go files and
21+
# finally copying the files into the module. The files are not
22+
# generated in place, because protoc generates a too-deep directory
23+
# structure.
1924
#
20-
# The protoc binary and other tools are sourced from a docker image
21-
# `PROTOC_IMAGE`.
25+
# Currently, all the generated code is in
26+
# `exporters/otlp/internal/opentelemetry-proto-gen`.
2227
#
23-
# Prereqs: The archiving utility `pax` is installed.
28+
# Prereqs: wget (for downloading the zip file with protoc binary),
29+
# unzip (for unpacking the archive), rsync (for copying back the
30+
# generated files).
2431

25-
PROTOC_IMAGE := namely/protoc-all:1.29_2
26-
PROTOBUF_VERSION := v1
27-
OTEL_PROTO_SUBMODULE := exporters/otlp/internal/opentelemetry-proto
28-
PROTOBUF_GEN_DIR := exporters/otlp/internal/opentelemetry-proto-gen
29-
PROTOBUF_TEMP_DIR := gen/pb-go
30-
PROTO_SOURCE_DIR := gen/proto
31-
SUBMODULE_PROTO_FILES := $(wildcard $(OTEL_PROTO_SUBMODULE)/opentelemetry/proto/*/$(PROTOBUF_VERSION)/*.proto \
32-
$(OTEL_PROTO_SUBMODULE)/opentelemetry/proto/collector/*/$(PROTOBUF_VERSION)/*.proto)
33-
SOURCE_PROTO_FILES := $(subst $(OTEL_PROTO_SUBMODULE),$(PROTO_SOURCE_DIR),$(SUBMODULE_PROTO_FILES))
32+
PROTOC_VERSION := 3.14.0
3433

35-
default: protobuf
34+
TOOLS_DIR := $(abspath ./.tools)
35+
TOOLS_MOD_DIR := ./internal/tools
36+
PROTOBUF_VERSION := v1
37+
OTEL_PROTO_SUBMODULE := exporters/otlp/internal/opentelemetry-proto
38+
GEN_TEMP_DIR := gen
39+
SUBMODULE_PROTO_FILES := $(wildcard $(OTEL_PROTO_SUBMODULE)/opentelemetry/proto/*/$(PROTOBUF_VERSION)/*.proto) $(wildcard $(OTEL_PROTO_SUBMODULE)/opentelemetry/proto/collector/*/$(PROTOBUF_VERSION)/*.proto)
3640

37-
.PHONY: protobuf protobuf-source gen-protobuf copy-protobufs
38-
protobuf: protobuf-source gen-protobuf copy-protobufs
41+
ifeq ($(strip $(SUBMODULE_PROTO_FILES)),)
42+
$(error Submodule at $(OTEL_PROTO_SUBMODULE) is not checked out, use "git submodule update --init")
43+
endif
3944

40-
protobuf-source: $(SOURCE_PROTO_FILES) | $(PROTO_SOURCE_DIR)/
45+
PROTOBUF_GEN_DIR := exporters/otlp/internal/opentelemetry-proto-gen
46+
PROTOBUF_TEMP_DIR := $(GEN_TEMP_DIR)/pb-go
47+
PROTO_SOURCE_DIR := $(GEN_TEMP_DIR)/proto
48+
SOURCE_PROTO_FILES := $(subst $(OTEL_PROTO_SUBMODULE),$(PROTO_SOURCE_DIR),$(SUBMODULE_PROTO_FILES))
4149

42-
# Changes go_package in .proto file to point to repo-local location
43-
define exec-replace-pkgname
44-
sed 's,go_package = "github.com/open-telemetry/opentelemetry-proto/gen/go,go_package = "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen,' < $(1) > $(2)
50+
.DEFAULT_GOAL := protobuf
4551

46-
endef
52+
UNAME_S := $(shell uname -s)
53+
UNAME_M := $(shell uname -m)
54+
55+
ifeq ($(UNAME_S),Linux)
56+
57+
PROTOC_OS := linux
58+
PROTOC_ARCH := $(UNAME_M)
59+
60+
else ifeq ($(UNAME_S),Darwin)
61+
62+
PROTOC_OS := osx
63+
PROTOC_ARCH := x86_64
64+
65+
endif
4766

48-
# replace opentelemetry-proto package name by go.opentelemetry.io/otel specific version
49-
$(SOURCE_PROTO_FILES): $(PROTO_SOURCE_DIR)/%.proto: $(OTEL_PROTO_SUBMODULE)/%.proto
50-
@mkdir -p $(@D)
51-
$(call exec-replace-pkgname,$<,$@)
67+
PROTOC_ZIP_URL := https://github.com/protocolbuffers/protobuf/releases/download/v$(PROTOC_VERSION)/protoc-$(PROTOC_VERSION)-$(PROTOC_OS)-$(PROTOC_ARCH).zip
5268

53-
# Command to run protoc using docker image
54-
define exec-protoc-all
55-
docker run -v `pwd`:/defs $(PROTOC_IMAGE) $(1)
69+
$(TOOLS_DIR)/PROTOC_$(PROTOC_VERSION):
70+
@rm -f "$(TOOLS_DIR)"/PROTOC_* && \
71+
touch "$@"
5672

73+
# Depend on a versioned file (like PROTOC_3.14.0), so when version
74+
# gets bumped, we will depend on a nonexistent file and thus download
75+
# a newer version.
76+
$(TOOLS_DIR)/protoc/bin/protoc: $(TOOLS_DIR)/PROTOC_$(PROTOC_VERSION)
77+
echo "Fetching protoc $(PROTOC_VERSION)" && \
78+
rm -rf $(TOOLS_DIR)/protoc && \
79+
wget -O $(TOOLS_DIR)/protoc.zip $(PROTOC_ZIP_URL) && \
80+
unzip $(TOOLS_DIR)/protoc.zip -d $(TOOLS_DIR)/protoc-tmp && \
81+
rm $(TOOLS_DIR)/protoc.zip && \
82+
touch $(TOOLS_DIR)/protoc-tmp/bin/protoc && \
83+
mv $(TOOLS_DIR)/protoc-tmp $(TOOLS_DIR)/protoc
84+
85+
$(TOOLS_DIR)/protoc-gen-gogofast: $(TOOLS_MOD_DIR)/go.mod $(TOOLS_MOD_DIR)/go.sum $(TOOLS_MOD_DIR)/tools.go
86+
cd $(TOOLS_MOD_DIR) && \
87+
go build -o $(TOOLS_DIR)/protoc-gen-gogofast github.com/gogo/protobuf/protoc-gen-gogofast && \
88+
go mod tidy
89+
90+
# Return a sed expression for replacing the go_package option in proto
91+
# file with a one that's valid for us.
92+
#
93+
# Example: $(call get-sed-expr,$(PROTOBUF_GEN_DIR))
94+
define get-sed-expr
95+
's,go_package = "github.com/open-telemetry/opentelemetry-proto/gen/go,go_package = "go.opentelemetry.io/otel/$(1),'
5796
endef
5897

59-
gen-protobuf: $(SOURCE_PROTO_FILES) | $(PROTOBUF_GEN_DIR)/
60-
$(foreach file,$(subst ${PROTO_SOURCE_DIR}/,,$(SOURCE_PROTO_FILES)),$(call exec-protoc-all, -i $(PROTO_SOURCE_DIR) -f ${file} -l gogo -o ${PROTOBUF_TEMP_DIR}))
98+
.PHONY: protobuf
99+
protobuf: protobuf-source gen-protobuf copy-protobufs
100+
101+
.PHONY: protobuf-source
102+
protobuf-source: $(SOURCE_PROTO_FILES)
103+
104+
# This copies proto files from submodule into $(PROTO_SOURCE_DIR),
105+
# thus satisfying the $(SOURCE_PROTO_FILES) prerequisite. The copies
106+
# have their package name replaced by go.opentelemetry.io/otel.
107+
$(PROTO_SOURCE_DIR)/%.proto: $(OTEL_PROTO_SUBMODULE)/%.proto
108+
@ \
109+
mkdir -p $(@D); \
110+
sed -e $(call get-sed-expr,$(PROTOBUF_GEN_DIR)) "$<" >"$@.tmp"; \
111+
mv "$@.tmp" "$@"
61112

62-
# requires `pax` to be installed, as it has consistent options for both BSD (Darwin) and Linux
63-
copy-protobufs: | $(PROTOBUF_GEN_DIR)/
64-
find ./$(PROTOBUF_TEMP_DIR)/go.opentelemetry.io/otel/$(PROTOBUF_GEN_DIR) -type f -print0 | \
65-
pax -0 -s ',^./$(PROTOBUF_TEMP_DIR)/go.opentelemetry.io/otel/$(PROTOBUF_GEN_DIR),,' -rw ./$(PROTOBUF_GEN_DIR)
113+
.PHONY: gen-protobuf
114+
gen-protobuf: $(SOURCE_PROTO_FILES) $(TOOLS_DIR)/protoc-gen-gogofast $(TOOLS_DIR)/protoc/bin/protoc
115+
@ \
116+
mkdir -p "$(PROTOBUF_TEMP_DIR)"; \
117+
set -e; for f in $^; do \
118+
if [[ "$${f}" == $(TOOLS_DIR)/* ]]; then continue; fi; \
119+
echo "protoc $${f#"$(PROTO_SOURCE_DIR)/"}"; \
120+
PATH="$(TOOLS_DIR):$${PATH}" $(TOOLS_DIR)/protoc/bin/protoc --proto_path="$(PROTO_SOURCE_DIR)" --gogofast_out="plugins=grpc:$(PROTOBUF_TEMP_DIR)" "$${f}"; \
121+
done
66122

67-
$(PROTO_SOURCE_DIR)/ $(PROTOBUF_GEN_DIR)/:
68-
mkdir -p $@
123+
.PHONY: copy-protobufs
124+
copy-protobufs:
125+
@rsync -a $(PROTOBUF_TEMP_DIR)/go.opentelemetry.io/otel/exporters .
69126

70127
.PHONY: clean
71128
clean:
72-
rm -rf ./gen
129+
rm -rf $(GEN_TEMP_DIR)

internal/tools/go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.14
44

55
require (
66
github.com/client9/misspell v0.3.4
7+
github.com/gogo/protobuf v1.3.1
78
github.com/golangci/golangci-lint v1.33.0
89
github.com/itchyny/gojq v0.11.2
910
golang.org/x/tools v0.0.0-20201013201025-64a9e34f3752

internal/tools/go.sum

+4
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ github.com/gofrs/flock v0.8.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14j
103103
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
104104
github.com/gogo/protobuf v1.2.1 h1:/s5zKNz0uPFCZ5hddgPdo2TK2TVrUNMn0OOX8/aZMTE=
105105
github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4=
106+
github.com/gogo/protobuf v1.3.1 h1:DqDEcV5aeaTmdFBePNpYsp3FlcVH/2ISVVM9Qf8PSls=
107+
github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
106108
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
107109
github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
108110
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
@@ -229,6 +231,7 @@ github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7
229231
github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
230232
github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
231233
github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
234+
github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
232235
github.com/kisielk/gotool v1.0.0 h1:AV2c/EiW3KqPNT9ZKl07ehoAGi4C5/01Cfbblndcapg=
233236
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
234237
github.com/klauspost/compress v1.10.7/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
@@ -545,6 +548,7 @@ golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxb
545548
golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
546549
golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
547550
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
551+
golang.org/x/tools v0.0.0-20181030221726-6c7e314b6563/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
548552
golang.org/x/tools v0.0.0-20190110163146-51295c7ec13a/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
549553
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
550554
golang.org/x/tools v0.0.0-20190221204921-83362c3779f5/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=

internal/tools/tools.go

+1
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,5 @@ import (
2121
_ "github.com/golangci/golangci-lint/cmd/golangci-lint"
2222
_ "github.com/itchyny/gojq"
2323
_ "golang.org/x/tools/cmd/stringer"
24+
_ "github.com/gogo/protobuf/protoc-gen-gogofast"
2425
)

0 commit comments

Comments
 (0)