linkerd
/
linkerd2
mirror of https://github.com/linkerd/linkerd2.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix bug in the public-api where conduit stat params were ignored (#971) 

* Fix bug where we were dropping parts of the StatSummaryRequest
* Add tests for prometheus query strings and for failed cases

Problem
In #928 I rewrote the stat api to handle 'all' as a resource type. To query for all resource types, 
we would copy the Resource, LabelSelector and TimeWindow of the original request, and then 
go through all the resource types and set Resource.Type for each resource we wanted to get.
The bug is that while we copy over some fields of the original request, we didn't copy over all 
of them - namely Resource.Name and the Outbound resource. So the Stat endpoint would 
ignore any --to or --from flags, and would ignore requests for a specific named resource.

Solution
Copy over all fields from the request.

I've also added tests for this case. In this process I've refactored the stat_summary_test code 
to make it a bit easier to read/use.
This commit is contained in:
Risha Mars 2018-05-18 16:06:06 -07:00 committed by GitHub
parent a067d3918c
commit 1e6434f6de
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 214 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
controller/api/public/stat_summary.go
View File

@ -7,6 +7,7 @@ import (
"strings"
"time"
proto "github.com/golang/protobuf/proto"
"github.com/prometheus/common/model"
"github.com/runconduit/conduit/controller/api/util"
pb "github.com/runconduit/conduit/controller/gen/public"
@ -85,16 +86,8 @@ func (s *grpcServer) StatSummary(ctx context.Context, req *pb.StatSummaryRequest
resultChan := make(chan resourceResult)
for _, resource := range resourcesToQuery {
statReq := &pb.StatSummaryRequest{
Selector: &pb.ResourceSelection{
Resource: &pb.Resource{
Type: resource,
Namespace: req.Selector.Resource.Namespace,
},
LabelSelector: req.Selector.LabelSelector,
},
TimeWindow: req.TimeWindow,
}
statReq := proto.Clone(req).(*pb.StatSummaryRequest)
statReq.Selector.Resource.Type = resource
go func() {
resultChan <- s.resourceQuery(ctx, statReq)
@ -300,7 +293,6 @@ func buildRequestLabels(req *pb.StatSummaryRequest) (model.LabelSet, model.Label
labelNames = promLabelNames(req.Selector.Resource)
labels = labels.Merge(promLabels(req.Selector.Resource))
labels = labels.Merge(promDirectionLabels("inbound"))
}
return labels, labelNames

279
controller/api/public/stat_summary_test.go
View File

@ -18,30 +18,100 @@ import (
)
type statSumExpected struct {
err error
k8sRes []string
promRes model.Value
req pb.StatSummaryRequest
res pb.StatSummaryResponse
err error
k8sConfigs []string // k8s objects to seed the lister
mockPromResponse model.Value // mock out a prometheus query response
expectedPrometheusQueries []string // queries we expect public-api to issue to prometheus
req pb.StatSummaryRequest // the request we would like to test
expectedResponse pb.StatSummaryResponse // the stat response we expect
}
func prometheusMetric(resName string, resType string, resNs string, classification string) model.Vector {
return model.Vector{
genPromSample(resName, resType, resNs, classification),
}
}
func genPromSample(resName string, resType string, resNs string, classification string) *model.Sample {
return &model.Sample{
Metric: model.Metric{
model.LabelName(resType): model.LabelValue(resName),
"namespace": model.LabelValue(resNs),
"classification": model.LabelValue(classification),
},
Value: 123,
Timestamp: 456,
}
}
func genStatSummaryResponse(resName, resType, resNs string, meshedPods uint64, runningPods uint64) pb.StatSummaryResponse {
return pb.StatSummaryResponse{
Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205
Ok: &pb.StatSummaryResponse_Ok{
StatTables: []*pb.StatTable{
&pb.StatTable{
Table: &pb.StatTable_PodGroup_{
PodGroup: &pb.StatTable_PodGroup{
Rows: []*pb.StatTable_PodGroup_Row{
&pb.StatTable_PodGroup_Row{
Resource: &pb.Resource{
Namespace: resNs,
Type: resType,
Name: resName,
},
Stats: &pb.BasicStats{
SuccessCount: 123,
FailureCount: 0,
LatencyMsP50: 123,
LatencyMsP95: 123,
LatencyMsP99: 123,
},
TimeWindow: "1m",
MeshedPodCount: meshedPods,
RunningPodCount: runningPods,
},
},
},
},
},
},
},
},
}
}
func genEmptyResponse() pb.StatSummaryResponse {
return pb.StatSummaryResponse{
Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205
Ok: &pb.StatSummaryResponse_Ok{
StatTables: []*pb.StatTable{},
},
},
}
}
func createFakeLister(t *testing.T, k8sConfigs []string) *k8s.Lister {
k8sObjs := []runtime.Object{}
for _, res := range k8sConfigs {
decode := scheme.Codecs.UniversalDeserializer().Decode
obj, _, err := decode([]byte(res), nil, nil)
if err != nil {
t.Fatalf("could not decode yml: %s", err)
}
k8sObjs = append(k8sObjs, obj)
}
clientSet := fake.NewSimpleClientset(k8sObjs...)
return k8s.NewLister(clientSet)
}
func testStatSummary(t *testing.T, expectations []statSumExpected) {
for _, exp := range expectations {
k8sObjs := []runtime.Object{}
for _, res := range exp.k8sRes {
decode := scheme.Codecs.UniversalDeserializer().Decode
obj, _, err := decode([]byte(res), nil, nil)
if err != nil {
t.Fatalf("could not decode yml: %s", err)
}
k8sObjs = append(k8sObjs, obj)
}
clientSet := fake.NewSimpleClientset(k8sObjs...)
lister := k8s.NewLister(clientSet)
lister := createFakeLister(t, exp.k8sConfigs)
mockProm := &MockProm{Res: exp.mockPromResponse}
fakeGrpcServer := newGrpcServer(
&MockProm{Res: exp.promRes},
mockProm,
tap.NewTapClient(nil),
lister,
"conduit",
@ -57,11 +127,23 @@ func testStatSummary(t *testing.T, expectations []statSumExpected) {
t.Fatalf("Expected error: %s, Got: %s", exp.err, err)
}
unsortedStatTables := rsp.GetOk().StatTables
sort.Sort(byStatResult(unsortedStatTables))
if len(exp.expectedPrometheusQueries) > 0 {
sort.Strings(exp.expectedPrometheusQueries)
sort.Strings(mockProm.QueriesExecuted)
if !reflect.DeepEqual(exp.res.GetOk().StatTables, unsortedStatTables) {
t.Fatalf("Expected: %+v, Got: %+v", &exp.res, rsp)
if !reflect.DeepEqual(exp.expectedPrometheusQueries, mockProm.QueriesExecuted) {
t.Fatalf("Prometheus queries incorrect. \nExpected: %+v \nGot: %+v",
exp.expectedPrometheusQueries, mockProm.QueriesExecuted)
}
}
if len(exp.expectedResponse.GetOk().StatTables) > 0 {
unsortedStatTables := rsp.GetOk().StatTables
sort.Sort(byStatResult(unsortedStatTables))
if !reflect.DeepEqual(exp.expectedResponse.GetOk().StatTables, unsortedStatTables) {
t.Fatalf("Expected: %+v\n Got: %+v", &exp.expectedResponse, rsp)
}
}
}
}
@ -92,7 +174,7 @@ func TestStatSummary(t *testing.T) {
expectations := []statSumExpected{
statSumExpected{
err: nil,
k8sRes: []string{`
k8sConfigs: []string{`
apiVersion: apps/v1beta2
kind: Deployment
metadata:
@ -143,17 +225,7 @@ status:
phase: Completed
`,
},
promRes: model.Vector{
&model.Sample{
Metric: model.Metric{
"deployment": "emoji",
"namespace": "emojivoto",
"classification": "success",
},
Value: 123,
Timestamp: 456,
},
},
mockPromResponse: prometheusMetric("emoji", "deployment", "emojivoto", "success"),
req: pb.StatSummaryRequest{
Selector: &pb.ResourceSelection{
Resource: &pb.Resource{
@ -163,39 +235,100 @@ status:
},
TimeWindow: "1m",
},
res: pb.StatSummaryResponse{
Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205
Ok: &pb.StatSummaryResponse_Ok{
StatTables: []*pb.StatTable{
&pb.StatTable{
Table: &pb.StatTable_PodGroup_{
PodGroup: &pb.StatTable_PodGroup{
Rows: []*pb.StatTable_PodGroup_Row{
&pb.StatTable_PodGroup_Row{
Resource: &pb.Resource{
Namespace: "emojivoto",
Type: "deployments",
Name: "emoji",
},
Stats: &pb.BasicStats{
SuccessCount: 123,
FailureCount: 0,
LatencyMsP50: 123,
LatencyMsP95: 123,
LatencyMsP99: 123,
},
TimeWindow: "1m",
MeshedPodCount: 1,
RunningPodCount: 2,
},
},
},
},
},
},
expectedResponse: genStatSummaryResponse("emoji", "deployments", "emojivoto", 1, 2),
},
}
testStatSummary(t, expectations)
})
t.Run("Queries prometheus for a specific resource if name is specified", func(t *testing.T) {
expectations := []statSumExpected{
statSumExpected{
err: nil,
k8sConfigs: []string{`
apiVersion: v1
kind: Pod
metadata:
name: emojivoto-1
namespace: emojivoto
labels:
app: emoji-svc
annotations:
conduit.io/proxy-version: testinjectversion
status:
phase: Running
`,
},
mockPromResponse: prometheusMetric("emojivoto-1", "pod", "emojivoto", "success"),
req: pb.StatSummaryRequest{
Selector: &pb.ResourceSelection{
Resource: &pb.Resource{
Name: "emojivoto-1",
Namespace: "emojivoto",
Type: pkgK8s.Pods,
},
},
TimeWindow: "1m",
},
expectedPrometheusQueries: []string{
`histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="inbound", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`,
`histogram_quantile(0.95, sum(irate(response_latency_ms_bucket{direction="inbound", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`,
`histogram_quantile(0.99, sum(irate(response_latency_ms_bucket{direction="inbound", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (le, namespace, pod))`,
`sum(increase(response_total{direction="inbound", namespace="emojivoto", pod="emojivoto-1"}[1m])) by (namespace, pod, classification)`,
},
expectedResponse: genStatSummaryResponse("emojivoto-1", "pods", "emojivoto", 1, 1),
},
}
testStatSummary(t, expectations)
})
t.Run("Queries prometheus for outbound metrics if from resource is specified, ignores resource name", func(t *testing.T) {
expectations := []statSumExpected{
statSumExpected{
err: nil,
k8sConfigs: []string{`
apiVersion: v1
kind: Pod
metadata:
name: emojivoto-1
namespace: emojivoto
labels:
app: emoji-svc
annotations:
conduit.io/proxy-version: testinjectversion
status:
phase: Running
`,
},
mockPromResponse: model.Vector{
genPromSample("emojivoto-2", "pod", "emojivoto", "success"),
},
req: pb.StatSummaryRequest{
Selector: &pb.ResourceSelection{
Resource: &pb.Resource{
Name: "emojivoto-1",
Namespace: "emojivoto",
Type: pkgK8s.Pods,
},
},
TimeWindow: "1m",
Outbound: &pb.StatSummaryRequest_FromResource{
FromResource: &pb.Resource{
Name: "emojivoto-2",
Namespace: "emojivoto",
Type: pkgK8s.Pods,
},
},
},
expectedPrometheusQueries: []string{
`histogram_quantile(0.5, sum(irate(response_latency_ms_bucket{direction="outbound", namespace="emojivoto", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`,
`histogram_quantile(0.95, sum(irate(response_latency_ms_bucket{direction="outbound", namespace="emojivoto", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`,
`histogram_quantile(0.99, sum(irate(response_latency_ms_bucket{direction="outbound", namespace="emojivoto", pod="emojivoto-2"}[1m])) by (le, dst_namespace, dst_pod))`,
`sum(increase(response_total{direction="outbound", namespace="emojivoto", pod="emojivoto-2"}[1m])) by (dst_namespace, dst_pod, classification)`,
},
expectedResponse: genEmptyResponse(),
},
}
@ -206,7 +339,7 @@ status:
expectations := []statSumExpected{
statSumExpected{
err: nil,
k8sRes: []string{`
k8sConfigs: []string{`
apiVersion: apps/v1beta2
kind: Deployment
metadata:
@ -257,17 +390,7 @@ status:
phase: Running
`,
},
promRes: model.Vector{
&model.Sample{
Metric: model.Metric{
"deployment": "emoji-deploy",
"namespace": "emojivoto",
"classification": "success",
},
Value: 123,
Timestamp: 456,
},
},
mockPromResponse: prometheusMetric("emoji-deploy", "deployment", "emojivoto", "success"),
req: pb.StatSummaryRequest{
Selector: &pb.ResourceSelection{
Resource: &pb.Resource{
@ -277,7 +400,7 @@ status:
},
TimeWindow: "1m",
},
res: pb.StatSummaryResponse{
expectedResponse: pb.StatSummaryResponse{
Response: &pb.StatSummaryResponse_Ok_{ // https://github.com/golang/protobuf/issues/205
Ok: &pb.StatSummaryResponse_Ok{
StatTables: []*pb.StatTable{
@ -397,7 +520,7 @@ status:
clientSet := fake.NewSimpleClientset()
lister := k8s.NewLister(clientSet)
fakeGrpcServer := newGrpcServer(
&MockProm{Res: exp.promRes},
&MockProm{Res: exp.mockPromResponse},
tap.NewTapClient(nil),
lister,
"conduit",

11
controller/api/public/test_helper.go
View File

@ -3,6 +3,7 @@ package public
import (
"context"
"io"
"sync"
"time"
"github.com/prometheus/client_golang/api/prometheus/v1"
@ -100,14 +101,22 @@ func (a *MockApi_TapByResourceClient) Recv() (*common.TapEvent, error) {
//
type MockProm struct {
Res model.Value
Res model.Value
QueriesExecuted []string // expose the queries our Mock Prometheus receives, to test query generation
rwLock sync.Mutex
}
// satisfies v1.API
func (m *MockProm) Query(ctx context.Context, query string, ts time.Time) (model.Value, error) {
m.rwLock.Lock()
defer m.rwLock.Unlock()
m.QueriesExecuted = append(m.QueriesExecuted, query)
return m.Res, nil
}
func (m *MockProm) QueryRange(ctx context.Context, query string, r v1.Range) (model.Value, error) {
m.rwLock.Lock()
defer m.rwLock.Unlock()
m.QueriesExecuted = append(m.QueriesExecuted, query)
return m.Res, nil
}
func (m *MockProm) LabelValues(ctx context.Context, label string) (model.LabelValues, error) {