Skip to content

Commit

Permalink
Auto-generate gogo annotations for api_v3 (#6233)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Part of #4150

## Description of the changes
- Upgrade to new IDL that removes gogo annotation from
api_v3/query.proto
- Add a pre-processor to inject annotations during build
- Fix tests
- Add start/end time to GetTrace test (which was failing in #6228)

## How was this change tested?
- 

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Nov 21, 2024
1 parent f43870b commit c00931f
Show file tree
Hide file tree
Showing 10 changed files with 2,506 additions and 154 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ cmd/tracegen/tracegen-*
crossdock/crossdock-*

run-crossdock.log
proto-gen/.patched-otel-proto/
__pycache__
.asset-manifest.json
deploy/
Expand Down
14 changes: 11 additions & 3 deletions Makefile.Protobuf.mk
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ PATCHED_OTEL_PROTO_DIR = proto-gen/.patched-otel-proto

PROTO_INCLUDES := \
-Iidl/proto/api_v2 \
-Iidl/proto/api_v3 \
-Imodel/proto/metrics \
-I/usr/include/github.com/gogo/protobuf

Expand Down Expand Up @@ -127,9 +126,18 @@ proto-zipkin:
# Note that the .pb.go types must be generated into the same internal package $(API_V3_PATH)
# where a manually defined traces.go file is located.
API_V3_PATH=cmd/query/app/internal/api_v3
API_V3_PATCHED_DIR=proto-gen/.patched/api_v3
API_V3_PATCHED=$(API_V3_PATCHED_DIR)/query_service.proto
.PHONY: patch-api-v3
patch-api-v3:
mkdir -p $(API_V3_PATCHED_DIR)
cat idl/proto/api_v3/query_service.proto | \
$(SED) -f ./proto-gen/patch-api-v3.sed \
> $(API_V3_PATCHED)

.PHONY: proto-api-v3
proto-api-v3:
$(call proto_compile, $(API_V3_PATH), idl/proto/api_v3/query_service.proto, -Iidl/opentelemetry-proto)
proto-api-v3: patch-api-v3
$(call proto_compile, $(API_V3_PATH), $(API_V3_PATCHED), -I$(API_V3_PATCHED_DIR) -Iidl/opentelemetry-proto)
@echo "🏗️ replace TracesData with internal custom type"
$(SED) -i 's/v1.TracesData/TracesData/g' $(API_V3_PATH)/query_service.pb.go
@echo "🏗️ remove OTEL import because we're not using any other OTLP types"
Expand Down
39 changes: 11 additions & 28 deletions cmd/query/app/apiv3/grpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"errors"
"fmt"

"github.com/gogo/protobuf/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

Expand Down Expand Up @@ -51,44 +50,28 @@ func (h *Handler) FindTraces(request *api_v3.FindTracesRequest, stream api_v3.Qu
if query == nil {
return status.Error(codes.InvalidArgument, "missing query")
}
if query.GetStartTimeMin() == nil ||
query.GetStartTimeMax() == nil {
if query.GetStartTimeMin().IsZero() ||
query.GetStartTimeMax().IsZero() {
return errors.New("start time min and max are required parameters")
}

queryParams := &spanstore.TraceQueryParameters{
ServiceName: query.GetServiceName(),
OperationName: query.GetOperationName(),
Tags: query.GetAttributes(),
NumTraces: int(query.GetNumTraces()),
NumTraces: int(query.GetSearchDepth()),
}
if query.GetStartTimeMin() != nil {
startTimeMin, err := types.TimestampFromProto(query.GetStartTimeMin())
if err != nil {
return err
}
queryParams.StartTimeMin = startTimeMin
if ts := query.GetStartTimeMin(); !ts.IsZero() {
queryParams.StartTimeMin = ts
}
if query.GetStartTimeMax() != nil {
startTimeMax, err := types.TimestampFromProto(query.GetStartTimeMax())
if err != nil {
return err
}
queryParams.StartTimeMax = startTimeMax
if ts := query.GetStartTimeMax(); !ts.IsZero() {
queryParams.StartTimeMax = ts
}
if query.GetDurationMin() != nil {
durationMin, err := types.DurationFromProto(query.GetDurationMin())
if err != nil {
return err
}
queryParams.DurationMin = durationMin
if d := query.GetDurationMin(); d != 0 {
queryParams.DurationMin = d
}
if query.GetDurationMax() != nil {
durationMax, err := types.DurationFromProto(query.GetDurationMax())
if err != nil {
return err
}
queryParams.DurationMax = durationMax
if d := query.GetDurationMax(); d != 0 {
queryParams.DurationMax = d
}

traces, err := h.QueryService.FindTraces(stream.Context(), queryParams)
Expand Down
23 changes: 9 additions & 14 deletions cmd/query/app/apiv3/grpc_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"errors"
"net"
"testing"
"time"

"github.com/gogo/protobuf/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -124,7 +124,9 @@ func TestGetTraceTraceIDError(t *testing.T) {
}, nil).Once()

getTraceStream, err := tsc.client.GetTrace(context.Background(), &api_v3.GetTraceRequest{
TraceId: "Z",
TraceId: "Z",
StartTime: time.Now().Add(-2 * time.Hour),
EndTime: time.Now(),
})
require.NoError(t, err)
recv, err := getTraceStream.Recv()
Expand All @@ -150,10 +152,8 @@ func TestFindTraces(t *testing.T) {
ServiceName: "myservice",
OperationName: "opname",
Attributes: map[string]string{"foo": "bar"},
StartTimeMin: &types.Timestamp{},
StartTimeMax: &types.Timestamp{},
DurationMin: &types.Duration{},
DurationMax: &types.Duration{},
StartTimeMin: time.Now().Add(-2 * time.Hour),
StartTimeMax: time.Now(),
},
})
require.NoError(t, err)
Expand All @@ -172,10 +172,7 @@ func TestFindTracesQueryNil(t *testing.T) {
assert.Nil(t, recv)

responseStream, err = tsc.client.FindTraces(context.Background(), &api_v3.FindTracesRequest{
Query: &api_v3.TraceQueryParameters{
StartTimeMin: nil,
StartTimeMax: nil,
},
Query: &api_v3.TraceQueryParameters{},
})
require.NoError(t, err)
recv, err = responseStream.Recv()
Expand All @@ -190,10 +187,8 @@ func TestFindTracesStorageError(t *testing.T) {

responseStream, err := tsc.client.FindTraces(context.Background(), &api_v3.FindTracesRequest{
Query: &api_v3.TraceQueryParameters{
StartTimeMin: &types.Timestamp{},
StartTimeMax: &types.Timestamp{},
DurationMin: &types.Duration{},
DurationMax: &types.Duration{},
StartTimeMin: time.Now().Add(-2 * time.Hour),
StartTimeMax: time.Now(),
},
})
require.NoError(t, err)
Expand Down
Loading

0 comments on commit c00931f

Please sign in to comment.