Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace domain model with Protobuf/gogo-generated model #856

Merged
merged 12 commits into from
Jun 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ codecov:
notify:
require_ci_to_pass: yes

# see https://docs.codecov.io/docs/ignoring-paths
ignore:
- "jaeger.pb*.go"
68 changes: 66 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ TOP_PKGS := $(shell glide novendor | grep -v -e ./thrift-gen/... -e swagger-gen.
STORAGE_PKGS = ./plugin/storage/integration/...

# all .go files that don't exist in hidden directories
ALL_SRC := $(shell find . -name "*.go" | grep -v -e vendor -e thrift-gen -e swagger-gen -e examples -e doc.go \
ALL_SRC := $(shell find . -name "*.go" | grep -v -e vendor -e thrift-gen -e swagger-gen -e examples -e doc.go -e jaeger_test.pb.go \
-e ".*/\..*" \
-e ".*/_.*" \
-e ".*/mocks.*")
Expand Down Expand Up @@ -92,6 +92,8 @@ cover: nocover
@echo pre-compiling tests
@time go test -i $(ALL_PKGS)
@./scripts/cover.sh $(shell go list $(TOP_PKGS))
grep -E -v 'jaeger.pb.*.go' cover.out > cover-nogen.out
mv cover-nogen.out cover.out
go tool cover -html=cover.out -o cover.html

.PHONY: nocover
Expand All @@ -113,7 +115,7 @@ lint-gas:
lint: lint-gas
$(GOVET) $(TOP_PKGS)
@cat /dev/null > $(LINT_LOG)
@$(foreach pkg, $(TOP_PKGS), $(GOLINT) $(pkg) | grep -v -e pkg/es/wrapper.go -e /mocks/ -e thrift-gen -e thrift-0.9.2 >> $(LINT_LOG) || true;)
@$(foreach pkg, $(TOP_PKGS), $(GOLINT) $(pkg) | grep -v -e pkg/es/wrapper.go -e /mocks/ -e thrift-gen -e thrift-0.9.2 -e jaeger_test.pb.go >> $(LINT_LOG) || true;)
@[ ! -s "$(LINT_LOG)" ] || (echo "Lint Failures" | cat - $(LINT_LOG) && false)
@$(GOFMT) -e -s -l $(ALL_SRC) > $(FMT_LOG)
@./scripts/updateLicenses.sh >> $(FMT_LOG)
Expand Down Expand Up @@ -291,3 +293,65 @@ generate-mocks: install-mockery
.PHONY: echo-version
echo-version:
@echo $(GIT_CLOSEST_TAG)

.PHONY: proto
proto:
# Generate gogo, gRPC-Gateway, swagger, go-validators 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 these
# folders and use the first one it finds.
#
# --gogo_out generates GoGo Protobuf output with gRPC plugin enabled.
# --grpc-gateway_out generates gRPC-Gateway output.
# --swagger_out generates an OpenAPI 2.0 specification for our gRPC-Gateway endpoints.
# --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.
#
# $$GOPATH/src is the output directory. It is relative to the GOPATH/src directory
# since we've specified a go_package option relative to that directory.
#
# model/proto/jaeger.proto is the location of the protofile we use.
#
# TODO use Docker container instead of installed protoc
# (https://medium.com/@linchenon/generate-grpc-and-protobuf-libraries-with-containers-c15ba4e4f3ad)
#
protoc \
-I model/proto \
-I vendor/github.com/grpc-ecosystem/grpc-gateway/ \
-I vendor/github.com/gogo/googleapis/ \
-I vendor/ \
--gogo_out=plugins=grpc,\
Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types,\
Mgoogle/api/annotations.proto=github.com/gogo/googleapis/google/api:\
$$GOPATH/src/github.com/jaegertracing/jaeger/model/ \
--grpc-gateway_out=\
Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types,\
Mgoogle/api/annotations.proto=github.com/gogo/googleapis/google/api:\
$$GOPATH/src/github.com/jaegertracing/jaeger/model \
--swagger_out=model/proto/openapi/ \
model/proto/jaeger.proto

# Workaround for https://github.com/grpc-ecosystem/grpc-gateway/issues/229.
sed -i '' "s/empty.Empty/types.Empty/g" model/jaeger.pb.gw.go

protoc \
-I model/proto \
--go_out=$$GOPATH/src/github.com/jaegertracing/jaeger/model/prototest/ \
model/proto/jaeger_test.proto

.PHONY: proto-install
proto-install:
go install \
./vendor/github.com/gogo/protobuf/protoc-gen-gogo \
./vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway \
./vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get this on my Debian desktop when trying this command:

$ make proto-install
go install \
	./vendor/github.com/gogo/protobuf/protoc-gen-gogo \
	./vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway \
	./vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger
vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/main.go:18:2: cannot find package "github.com/golang/glog" in any of:
	/home/ihier/proj/go/src/github.com/jaegertracing/jaeger/vendor/github.com/golang/glog (vendor tree)
	/usr/local/go/src/github.com/golang/glog (from $GOROOT)
	/home/ihier/proj/go/src/github.com/golang/glog (from $GOPATH)
make: *** [Makefile:352: proto-install] Error 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After finally getting it to work and generating Python code, I get this clearly invalid import:

from github.com.gogo.protobuf.gogoproto import gogo_pb2 as github_dot_com_dot_gogo_dot_protobuf_dot_gogop
roto_dot_gogo__pb2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with C++:

#include "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options/annotations.pb.h"
#include "github.com/gogo/protobuf/gogoproto/gogo.pb.h"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may have to strip them post-generation. E.g. in python these two extra imports are not actually used.

# ./vendor/github.com/mwitkow/go-proto-validators/protoc-gen-govalidators \
# ./vendor/github.com/rakyll/statik
4 changes: 2 additions & 2 deletions cmd/collector/app/sanitizer/utf8_sanitizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ func (s *utf8Sanitizer) Sanitize(span *model.Span) *model.Span {
func (s *utf8Sanitizer) logSpan(span *model.Span, message string, field zapcore.Field) {
s.logger.Info(
message,
zap.String("traceId", span.TraceID.String()),
zap.String("spanID", span.SpanID.String()), field)
zap.String("trace_id", span.TraceID.String()),
zap.String("span_id", span.SpanID.String()), field)
}

func sanitizeKV(keyValues model.KeyValues) {
Expand Down
10 changes: 6 additions & 4 deletions cmd/query/app/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"testing"
"time"

"github.com/gogo/protobuf/jsonpb"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -183,14 +184,15 @@ func TestGetTrace(t *testing.T) {
}

makeMockTrace := func(t *testing.T) *model.Trace {
bytes, err := json.Marshal(mockTrace)
out := new(bytes.Buffer)
err := new(jsonpb.Marshaler).Marshal(out, mockTrace)
require.NoError(t, err)
var trace *model.Trace
require.NoError(t, json.Unmarshal(bytes, &trace))
var trace model.Trace
require.NoError(t, jsonpb.Unmarshal(out, &trace))
trace.Spans[1].References = []model.SpanRef{
{TraceID: model.NewTraceID(0, 0)},
}
return trace
return &trace
}

extractTraces := func(t *testing.T, response *structuredResponse) []ui.Trace {
Expand Down
Loading