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

Update grpc & http endpoint to query operations by service & spanKind #1943

Merged
merged 6 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
18 changes: 14 additions & 4 deletions cmd/query/app/grpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,25 @@ func (g *GRPCHandler) GetServices(ctx context.Context, r *api_v2.GetServicesRequ
}

// GetOperations is the GRPC handler to fetch operations.
func (g *GRPCHandler) GetOperations(ctx context.Context, r *api_v2.GetOperationsRequest) (*api_v2.GetOperationsResponse, error) {
service := r.Service
operations, err := g.queryService.GetOperations(ctx, service)
func (g *GRPCHandler) GetOperations(ctx context.Context, r *api_v2.GetOperationsRequest) (
*api_v2.GetOperationsResponse, error) {
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
operations, err := g.queryService.GetOperations(ctx, spanstore.OperationQueryParameters{
ServiceName: r.Service,
SpanKind: r.SpanKind,
})
if err != nil {
g.logger.Error("Error fetching operations", zap.Error(err))
return nil, err
}

return &api_v2.GetOperationsResponse{Operations: operations}, nil
result := make([]*api_v2.Operation, len(operations))
for i, operation := range operations {
result[i] = &api_v2.Operation{
Name: operation.Name,
SpanKind: operation.SpanKind,
}
}
return &api_v2.GetOperationsResponse{Operations: result}, nil
}

// GetDependencies is the GRPC handler to fetch dependencies.
Expand Down
3 changes: 2 additions & 1 deletion cmd/query/app/grpc_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,8 @@ func TestGetOperationsSuccessGRPC(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, len(expectedOperations), len(res.Operations))
for i, actualOp := range res.Operations {
assert.Equal(t, expectedOperations[i].Name, actualOp)
assert.Equal(t, expectedOperations[i].Name, actualOp.Name)
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, expectedOperations[i].SpanKind, actualOp.SpanKind)
}
})
}
Expand Down
37 changes: 32 additions & 5 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,28 @@ func (aH *APIHandler) getOperationsLegacy(w http.ResponseWriter, r *http.Request
vars := mux.Vars(r)
// given how getOperationsLegacy is bound to URL route, serviceParam cannot be empty
service, _ := url.QueryUnescape(vars[serviceParam])
operations, err := aH.queryService.GetOperations(r.Context(), service)
//for backwards compatibility, we will retrieve operations with all span kind
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
operations, err := aH.queryService.GetOperations(r.Context(),
spanstore.OperationQueryParameters{
ServiceName: service,
SpanKind: "",
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
})

if aH.handleError(w, err, http.StatusInternalServerError) {
return
}
//only return unique operation names
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
set := make(map[string]struct{})
for _, operation := range operations {
set[operation.Name] = struct{}{}
}
var operationNames []string
for operation := range set {
operationNames = append(operationNames, operation)
}
structuredRes := structuredResponse{
Data: operations,
Total: len(operations),
Data: operationNames,
Total: len(operationNames),
}
aH.writeJSON(w, r, &structuredRes)
}
Expand All @@ -175,12 +190,24 @@ func (aH *APIHandler) getOperations(w http.ResponseWriter, r *http.Request) {
return
}
}
operations, err := aH.queryService.GetOperations(r.Context(), service)
spanKind := r.FormValue(spanKindParam)
operations, err := aH.queryService.GetOperations(
r.Context(),
spanstore.OperationQueryParameters{ServiceName: service, SpanKind: spanKind},
)

if aH.handleError(w, err, http.StatusInternalServerError) {
return
}
data := make([]*ui.Operation, len(operations))
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
for i, operation := range operations {
data[i] = &ui.Operation{
Name: operation.Name,
SpanKind: operation.SpanKind,
}
}
structuredRes := structuredResponse{
Data: operations,
Data: data,
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
Total: len(operations),
}
aH.writeJSON(w, r, &structuredRes)
Expand Down
32 changes: 24 additions & 8 deletions cmd/query/app/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,16 +484,27 @@ func TestGetOperationsSuccess(t *testing.T) {
server, readMock, _ := initializeTestServer()
defer server.Close()
expectedOperations := []spanstore.Operation{{Name: ""}, {Name: "get", SpanKind: "server"}}
readMock.On("GetOperations", mock.AnythingOfType("*context.valueCtx"),
spanstore.OperationQueryParameters{ServiceName: "abc/trifle"}).Return(expectedOperations, nil).Once()
readMock.On(
"GetOperations",
mock.AnythingOfType("*context.valueCtx"),
spanstore.OperationQueryParameters{ServiceName: "abc/trifle"},
).Return(expectedOperations, nil).Once()

var response struct {
Operations []*ui.Operation `json:"data"`
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
Total int `json:"total"`
Limit int `json:"limit"`
Offset int `json:"offset"`
Errors []structuredError `json:"errors"`
}

var response structuredResponse
err := getJSON(server.URL+"/api/operations?service=abc%2Ftrifle", &response)
assert.NoError(t, err)
for i, s := range response.Data.([]interface{}) {
assert.Equal(t, expectedOperations[i].Name, s.(string))
assert.Equal(t, len(expectedOperations), len(response.Operations))
for i, op := range response.Operations {
assert.Equal(t, expectedOperations[i].Name, op.Name)
assert.Equal(t, expectedOperations[i].SpanKind, op.SpanKind)
}

}

func TestGetOperationsNoServiceName(t *testing.T) {
Expand Down Expand Up @@ -522,16 +533,21 @@ func TestGetOperationsLegacySuccess(t *testing.T) {
server, readMock, _ := initializeTestServer()
defer server.Close()
expectedOperationNames := []string{"", "get"}
expectedOperations := []spanstore.Operation{{Name: ""}, {Name: "get", SpanKind: "server"}}
expectedOperations := []spanstore.Operation{
{Name: ""},
{Name: "get", SpanKind: "server"},
{Name: "get", SpanKind: "client"}}

readMock.On(
"GetOperations",
mock.AnythingOfType("*context.valueCtx"),
mock.AnythingOfType("spanstore.OperationQueryParameters")).Return(expectedOperations, nil).Once()

var response structuredResponse
err := getJSON(server.URL+"/api/services/abc%2Ftrifle/operations", &response)

assert.NoError(t, err)
actualOperations := make([]string, len(expectedOperations))
actualOperations := make([]string, len(expectedOperationNames))
for i, s := range response.Data.([]interface{}) {
actualOperations[i] = s.(string)
}
Expand Down
1 change: 1 addition & 0 deletions cmd/query/app/query_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const (
minDurationParam = "minDuration"
maxDurationParam = "maxDuration"
serviceParam = "service"
spanKindParam = "spanKind"
endTimeParam = "end"
prettyPrintParam = "prettyPrint"
)
Expand Down
20 changes: 5 additions & 15 deletions cmd/query/app/querysvc/query_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,11 @@ func (qs QueryService) GetServices(ctx context.Context) ([]string, error) {
}

// GetOperations is the queryService implementation of spanstore.Reader.GetOperations
func (qs QueryService) GetOperations(ctx context.Context, service string) ([]string, error) {
operations, err := qs.spanReader.GetOperations(ctx, spanstore.OperationQueryParameters{
ServiceName: service,
})

// TODO: remove below and simply return the result from query service
if err != nil {
return nil, err
}

names := make([]string, len(operations))
for i, operation := range operations {
names[i] = operation.Name
}
return names, err
func (qs QueryService) GetOperations(
ctx context.Context,
query spanstore.OperationQueryParameters,
) ([]spanstore.Operation, error) {
return qs.spanReader.GetOperations(ctx, query)
}

// FindTraces is the queryService implementation of spanstore.Reader.FindTraces
Expand Down
14 changes: 8 additions & 6 deletions cmd/query/app/querysvc/query_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,19 @@ func TestGetServices(t *testing.T) {
// Test QueryService.GetOperations() for success.
func TestGetOperations(t *testing.T) {
qs, readMock, _ := initializeTestService()
expectedOperationNames := []string{"", "get"}
expectedOperations := []spanstore.Operation{{Name: ""}, {Name: "get", SpanKind: "server"}}
readMock.On("GetOperations",
expectedOperations := []spanstore.Operation{{Name: "", SpanKind: ""}, {Name: "get", SpanKind: ""}}
operationQuery := spanstore.OperationQueryParameters{ServiceName: "abc/trifle"}
readMock.On(
"GetOperations",
mock.AnythingOfType("*context.valueCtx"),
mock.AnythingOfType("spanstore.OperationQueryParameters")).Return(expectedOperations, nil).Once()
operationQuery,
).Return(expectedOperations, nil).Once()

type contextKey string
ctx := context.Background()
actualOperations, err := qs.GetOperations(context.WithValue(ctx, contextKey("foo"), "bar"), "abc/trifle")
actualOperations, err := qs.GetOperations(context.WithValue(ctx, contextKey("foo"), "bar"), operationQuery)
assert.NoError(t, err)
assert.Equal(t, expectedOperationNames, actualOperations)
assert.Equal(t, expectedOperations, actualOperations)
}

// Test QueryService.FindTraces() for success.
Expand Down
6 changes: 6 additions & 0 deletions model/json/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,9 @@ type DependencyLink struct {
Child string `json:"child"`
CallCount uint64 `json:"callCount"`
}

// Operation defines the data in the operation response when query operation by service and span kind
type Operation struct {
Name string `json:"name"`
SpanKind string `json:"spanKind"`
}
9 changes: 8 additions & 1 deletion model/proto/api_v2/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,17 @@ message GetServicesResponse {

message GetOperationsRequest {
string service = 1;
string span_kind = 2;
}

message Operation {
string name = 1;
string span_kind = 2;
}

message GetOperationsResponse {
repeated string operations = 1;
repeated string operationNames = 1; //deprecated
guo0693 marked this conversation as resolved.
Show resolved Hide resolved
repeated Operation operations = 2;
}

message GetDependenciesRequest {
Expand Down
Loading