Add the ability to query stats by authority #1181

rmars · 2018-06-21T21:29:26Z

Adds "authority" as a Resource of Type authority. This is the first non-k8s resource we've added.
This required some refactoring in stat_summary.go since our current method of returning results is very dependent on kubernetes objects.
30d7373 and ffdec41 are small refactors to stat_summary.go to try to clean it up before adding authority

CLI examples:

$ conduit stat authority/10.1.1.241:3000 --namespace conduit 
NAME              MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
10.1.1.241:3000      0/0   100.00%   0.3rps           5ms          10ms          10ms        0%

$ conduit stat deploy --to authority/voting-svc.emojivoto:8080 --namespace emojivoto 
NAME   MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
web       2/2    63.33%   1.0rps           5ms          10ms          10ms        0%

$ conduit stat authority --namespace conduit 
NAME              MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
10.1.1.238:9994      0/0   100.00%   0.1rps           5ms          10ms          10ms        0%
10.1.1.239:9995      0/0   100.00%   0.1rps           5ms          10ms          10ms        0%
10.1.1.239:9996      0/0   100.00%   0.1rps           5ms          10ms          10ms        0%
10.1.1.239:9998      0/0   100.00%   0.1rps           5ms          10ms          10ms        0%
10.1.1.239:9999      0/0   100.00%   0.1rps           5ms          10ms          10ms        0%
10.1.1.240:9090      0/0   100.00%   0.2rps           5ms          10ms          10ms        0%
10.1.1.241:3000      0/0   100.00%   0.3rps           5ms           9ms          10ms        0%

Questions:

If we're querying by authority, should we require a namespace be set at all? Authorities can be non-k8s and have no ns
Should we be setting “dst_namespace” on —to authority requests? Currently, a request such as

conduit stat deploy --to authority/voting-svc.emojivoto:8080 --namespace emojivoto

Will generate the prometheus query

sum(increase(response_total{authority="voting-svc.emojivoto:8080", direction="outbound", dst_namespace="emojivoto", namespace="emojivoto"}[1m])) by (namespace, deployment, classification, tls)

This is because we set dst_namespace to the namespace of the request, but for an authority, I'm not sure we need the dst_namespace as well.

Follow ups:

Remove MESHED column from the CLI output for authority queries
Display authority stats in the Web UI
Should we move ResourceTypesToProxyLabels out of k8s.go?
(This applies to a bunch of our stats) Are all combinations of to/from with authority vetted? Do the invalid ones return errors?

Fixes #1147

Add documentation to Resource so we can use it for authority Handle non-k8s resource requests Rewrite stat summary fetching and parsing to handle non-k8s resources Requesting an authority as primary breakout now works

klingerf

Nice! Excited to see these stats. I had a few bits of miscellaneous feedback.

I'm a bit worried that it's going to get confusing to have both a CanonicalResourceType func and a CanonicalKubernetesNameFromFriendlyName func in the long run. I think my preference would be to just modify and rename the existing methods in pkg/k8s, and not add new methods that wrap those methods. It's ok by me that "authority" isn't actually a kubernetes resource. If it would be less confusion, we could move all of the code and constants to a new package, like pkg/resource or something like that.

klingerf · 2018-06-21T21:55:30Z

controller/api/public/test_helper.go

+		TimeWindow: "1m",
+	}
+
+	if !counts.OmitPodCounts {


TIOLI I think you could avoid needing this OmitPodCounts field altogether if you updated this method to take a pointer to PodCounts:

func GenStatSummaryResponse(resName, resType, resNs string, counts *PodCounts) pb.StatSummaryResponse {

And then here just checked to make sure it isn't nil:

if counts != nil { statTableRow.MeshedPodCount = counts.MeshedPods statTableRow.RunningPodCount = counts.RunningPods statTableRow.FailedPodCount = counts.FailedPods }

That way, in tests where you don't want counts, you could just pass nil instead of PodCounts{OmitPodCounts: true}.

klingerf · 2018-06-21T21:56:53Z

proto/public.proto

+  // This can be:
+  // - "all" -- includes all Kubernetes resource types only
+  // - "authority" -- a special resource type derived from request `:authority` values
+  // - Otherwise, the resource type may be any Kubernetes resource (e.g. "namespace", "deployment").


klingerf · 2018-06-21T22:01:21Z

controller/api/util/api_utils.go

+
+// An authority can only receive traffic, not send it
+func ValidateToResourceType(resourceType string) (string, error) {
+	return CanonicalResourceType(resourceType)


It looks like ValidateToResourceType and CanonicalResourceType are exact aliases of the same function. What's the goal of having both of these? Maybe we can just use CanonicalResourceType everywhere?

klingerf · 2018-06-21T22:08:50Z

cli/cmd/stat.go

-		return k8s.ShortNameFromCanonicalKubernetesName(resourceType) + "/"
+		// the type doesn't come from the user here; skip err check
+		canonicalType, _ := util.CanonicalResourceType(resourceType)
+		return canonicalType + "/"


I think maybe we should be using util.ShortNameFromCanonicalName here? That seems more equivalent to what we were doing previously. Plus it looks like that func is unused.

With this change, we're now seeing the full resource name in the stat all output:

$ bin/conduit -c kltest stat all -n kltest NAME MESHED SUCCESS RPS LATENCY_P50 LATENCY_P95 LATENCY_P99 SECURED deployments/controller 1/1 100.00% 1.2rps 5ms 10ms 10ms 0% deployments/grafana 1/1 100.00% 0.3rps 5ms 10ms 10ms 0% deployments/prometheus 1/1 100.00% 0.6rps 5ms 10ms 10ms 0% deployments/web 1/1 100.00% 0.3rps 5ms 10ms 10ms 0%

Compared to:

$ conduit -c kltest stat all -n kltest NAME MESHED SUCCESS RPS LATENCY_P50 LATENCY_P95 LATENCY_P99 SECURED deploy/controller 1/1 100.00% 1.2rps 5ms 10ms 10ms 0% deploy/grafana 1/1 100.00% 0.3rps 5ms 10ms 10ms 0% deploy/prometheus 1/1 100.00% 0.5rps 5ms 9ms 10ms 0% deploy/web 1/1 100.00% 0.3rps 5ms 10ms 10ms 0%

ah! yeah! thanks!

I created ShortNameFromCanonicalName for this purpose and somehow forgot to use it :/

klingerf · 2018-06-21T22:18:01Z

controller/api/util/api_utils.go

+	return CanonicalResourceType(resourceType)
+}
+
+func ValidateFromResourceType(resourceType string) (string, error) {


It looks like ValidateToResourceType and ValidateFromResourceType methods don't need to be exported, since they're not called outside of this package.

klingerf · 2018-06-21T22:26:51Z

controller/api/util/api_utils.go

@@ -176,6 +179,30 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest
 	return statRequest, nil
 }

+// Ensures a valid resource type - a canonical k8s object, or 'authority'
+func CanonicalResourceType(resourceType string) (string, error) {
+	if resourceType == Authority {


For consistency with CanonicalKubernetesNameFromFriendlyName, I think this method should also recognize "au" as a valid resourceType string.

rmars · 2018-06-21T23:38:12Z

Yeah, that's true. It would be less confusing in the long run to do away with the wrapper methods. I'll fold those into the k8s methods.

…resources

klingerf

Thanks for making those updates! I had one pretty substantial refactor to suggest in stat_summary.go, but the rest of this setup is looking good, and these authority stats are incredibly awesome.

$ ./bin/go-run cli stat au -n conduit-egress-test --to au
NAME              MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
www.httpbin.org      0/0   100.00%   0.5rps         150ms         195ms         199ms        0%

klingerf · 2018-06-22T20:07:30Z

controller/api/util/api_utils.go

@@ -21,6 +21,9 @@ import (
 var (
 	defaultMetricTimeWindow = "1m"

+	Authority          = "authority" // non-k8s resource type
+	AuthorityShortName = "au"
+


I'd rather not re-define these strings outside of pkg/k8s/k8s.go. Instead, I think you can remove them if you make the change to validateFromResourceType that I suggest below.

klingerf · 2018-06-22T20:17:21Z

controller/api/util/api_utils.go

+	if resourceType == Authority || resourceType == AuthorityShortName {
+		return "", errors.New("cannot query traffic --from an authority")
+	}
+	return k8s.CanonicalResourceNameFromFriendlyName(resourceType)


I think we can avoid defining the Authority and AuthorityShortName consts in this file if we change this method to:

func validateFromResourceType(resourceType string) (string, error) { name, err := k8s.CanonicalResourceNameFromFriendlyName(resourceType) if err != nil { return "", err } if name == k8s.Authority { return "", errors.New("cannot query traffic --from an authority") } return name, nil }

klingerf · 2018-06-22T22:36:07Z

controller/api/public/stat_summary.go

@@ -128,37 +131,50 @@ func statSummaryError(req *pb.StatSummaryRequest, message string) *pb.StatSummar
 	}
 }

-func (s *grpcServer) resourceQuery(ctx context.Context, req *pb.StatSummaryRequest) resourceResult {
+func (s *grpcServer) getKubernetesObjectStats(req *pb.StatSummaryRequest) (map[pb.Resource]k8sStat, error) {


This is a valiant effort to reuse all of the existing k8s resource code, but I think that authorities are different enough from k8s resources that we'd be better off just forking to a different method entirely in the case of authorities. I recommend leaving the resourceQuery method mostly in tact, but renaming it to k8sResourceQuery or something similar. And then we could add a nonK8sResourceQuery method that looks like this:

func (s *grpcServer) nonK8sResourceQuery(ctx context.Context, req *pb.StatSummaryRequest) resourceResult { rows := make([]*pb.StatTable_PodGroup_Row, 0) requestMetrics, err := s.getPrometheusMetrics(ctx, req, req.TimeWindow) if err != nil { return resourceResult{err: err} } for key, stats := range requestMetrics { resource := key row := pb.StatTable_PodGroup_Row{ Resource: &resource, TimeWindow: req.TimeWindow, Stats: stats, } rows = append(rows, &row) } rsp := pb.StatTable{ Table: &pb.StatTable_PodGroup_{ PodGroup: &pb.StatTable_PodGroup{ Rows: rows, }, }, } return resourceResult{res: &rsp} }

And then above, on line 96 where we fire off the queries in separate goroutines, we can do something like:

go func() { if resource == k8s.Authority { resultChan <- s.nonK8sResourceQuery(ctx, statReq) } else { resultChan <- s.k8sResourceQuery(ctx, statReq) } }()

Yeah! You're right, just splitting these out into separate cases makes them easier to follow, and we don't have to keep doing the k8s/nonk8s checks everywhere

klingerf · 2018-06-22T22:42:25Z

controller/api/public/stat_summary.go

@@ -292,19 +364,21 @@ func buildRequestLabels(req *pb.StatSummaryRequest) (labels model.LabelSet, labe



I think we also need to make a special exception when the ToResource is an authority, since we won't have any dst_authority labels, and we won't have dst_ns labels when the authority is external to the k8s cluster. So I recommend changing the ToResource case to:

case *pb.StatSummaryRequest_ToResource: labelNames = promLabelNames(req.Selector.Resource) if req.GetSelector().GetResource().GetType() != k8s.Authority { labels = labels.Merge(promDstLabels(out.ToResource)) } labels = labels.Merge(promLabels(req.Selector.Resource)) labels = labels.Merge(promDirectionLabels("outbound"))

I've refactored some of the label methods, maybe this will help. They are kind of complicated.

I think we can defer the setting of the labels to promDstLabels, since if we're querying a ToAuthority, we'd still need to set the label {authority=to_authority_name}, which won't be set by req.Selector.Resource.Name (we need to get it from out.ToResource.Name)

Currently promDstLabels does take care of checking to make sure we don't add a dst_authority or a dst_ns in the case of a ToAuthority query. And it sets the {authority=to_authority} that we want to filter by:

// query a named resource func promDstLabels(resource *pb.Resource) model.LabelSet { set := model.LabelSet{} if resource.Name != "" { if resource.Type == k8s.Authority { set[promResourceType(resource)] = model.LabelValue(resource.Name) } else { set["dst_"+promResourceType(resource)] = model.LabelValue(resource.Name) if shouldAddNamespaceLabel(resource) { set[dstNamespaceLabel] = model.LabelValue(resource.Namespace) } } } return set }

klingerf

Thanks for making those updates! I had just a few more comments below, and then this should be good to go.

klingerf · 2018-06-26T17:50:10Z

cli/cmd/stat.go

@@ -303,7 +303,8 @@ func getNamePrefix(resourceType string) string {
 	if resourceType == "" {
 		return ""
 	} else {
-		return k8s.ShortNameFromCanonicalKubernetesName(resourceType) + "/"
+		canonicalType := k8s.ShortNameFromCanonicalResourceName(resourceType)
+		return canonicalType + "/"


Let's also update the long description for the stat command to include "authority" as a valid resource type. On line 62 of this file:

* authorities * deployments * namespaces * pods * replicationcontrollers * services (only supported if a --from is also specified, or as a --to) * all (all resource types, not supported in --from or --to)

Can you also remove the quotes around --from and --to in the description for "services", to match the description for "all"?

Also, while you're there, can you fix this issue with the examples:

Examples: * deploy * deploy/my-deploy * deploy my-deploy * ns/my-ns * all

"deploy/my-deploy" is listed twice. Maybe that should be "rc/my-replication-controller"?

Related: do you think it makes sense to add authority to the output of conduit stat all?

oh, yeah, I think it does!

klingerf · 2018-06-26T18:14:27Z

controller/api/public/stat_summary.go

 	}
-	return strings.Join(values, "/")
+
+	key.Name = resourceKeys[resourceName]


I don't think this method is properly formulating keys for namespace-type resources. The stat integration test is failing on this branch with:

--- FAIL: TestCliStatForConduitNamespace (20.21s) --- PASS: TestCliStatForConduitNamespace/test_conduit_stat_deploy (0.09s) --- FAIL: TestCliStatForConduitNamespace/test_conduit_stat_namespace (20.12s) stat_test.go:88: Expected success rate [100.00%] for [kltest], got [-]

The issue is that this method returns an empty key for the conduit namespace, whereas is should be returning a key with {Name: <conduit-ns>}.

I think we could fix and simplify this with something like:

func metricToKey(metric model.Metric, groupBy model.LabelNames) pb.Resource { // i.e. produce a key using the resource name and namespace key := pb.Resource{} for _, k := range groupBy { item := string(k) if _, ok := k8s.ProxyLabelsToResourceTypes[item]; ok { if item == "namespace" && len(groupBy) == 2 { key.Namespace = string(metric[k]) } else { key.Name = string(metric[k]) } } } return key }

Ah, and actually, if you make that change, then I don't think you need ProxyLabelsToResourceTypes at all, since groupBy will always contain either "namespace", or the string representing the name. In which case you could simplify even further to:

func metricToKey(metric model.Metric, groupBy model.LabelNames) pb.Resource { key := pb.Resource{} for _, k := range groupBy { if string(k) == "namespace" && len(groupBy) == 2 { key.Namespace = string(metric[k]) } else { key.Name = string(metric[k]) } } return key }

That would also allow you to remove ProxyLabelsToResourceTypes and reverseMap from pkg/k8s/k8s.go.

rmars · 2018-06-26T20:31:17Z

Added to stat all!

$ go run cli/main.go stat all --kubeconfig ~/.kube/config --api-addr localhost:8085 -n conduit
NAME                             MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
deploy/controller                   1/1   100.00%   1.2rps           5ms           9ms          10ms        0%
deploy/grafana                      1/1   100.00%   0.3rps           5ms          10ms          10ms        0%
deploy/prometheus                   1/1   100.00%   0.2rps           5ms          10ms          10ms        0%
deploy/web                          1/1   100.00%   0.3rps           8ms          19ms          20ms        0%

NAME                             MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
po/controller-5d46776897-w9b44      1/1   100.00%   1.2rps           5ms           9ms          10ms        0%
po/grafana-666799f878-vjqzw         1/1   100.00%   0.3rps           5ms          10ms          10ms        0%
po/prometheus-8458f9d59f-56g8j      1/1   100.00%   0.2rps           5ms          10ms          10ms        0%
po/web-86b8564b59-dn9tb             1/1   100.00%   0.3rps           8ms          19ms          20ms        0%

NAME                             MESHED   SUCCESS   RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
svc/api                             1/1         -     -             -             -             -         -
svc/grafana                         1/1         -     -             -             -             -         -
svc/prometheus                      1/1         -     -             -             -             -         -
svc/proxy-api                       1/1         -     -             -             -             -         -
svc/web                             1/1         -     -             -             -             -         -

NAME                             MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   SECURED
au/10.1.1.247:9994                  0/0   100.00%   0.3rps           8ms          19ms          20ms        0%
au/10.1.1.248:9090                  0/0   100.00%   0.2rps           5ms          10ms          10ms        0%
au/10.1.1.249:9995                  0/0   100.00%   0.3rps           5ms           9ms          10ms        0%
au/10.1.1.249:9996                  0/0   100.00%   0.3rps           5ms           9ms          10ms        0%
au/10.1.1.249:9998                  0/0   100.00%   0.3rps           5ms           9ms          10ms        0%
au/10.1.1.249:9999                  0/0   100.00%   0.3rps           5ms           9ms          10ms        0%
au/10.1.2.8:3000                    0/0   100.00%   0.3rps           5ms          10ms          10ms        0%

* Enable Authorities in web UI, under /authorities

klingerf

⭐️ Awesome. I just had a few more tiny nits, but this looks great!

klingerf · 2018-06-26T23:29:56Z

cli/cmd/stat.go

@@ -200,8 +203,12 @@ func writeStatsToBuffer(resp *pb.StatSummaryResponse, reqResourceType string, w
 				maxNamespaceLength = len(namespace)
 			}

+			meshedCount := fmt.Sprintf("%d/%d", r.MeshedPodCount, r.RunningPodCount)
+			if resourceKey == k8s.Authorities {


Thanks for adding this! Rather than checking the resource type, I think it might be more future-proof to check if r.RunningPodCount == 0.

that's fair. I'd originally wanted to keep the 0/0 as a clearer indicator of 0 running pods, as opposed to - which I wanted to imply N/A. (Like, a 0 value vs a null value) What do you think?

Hmm, yeah, I hadn't thought about that. Like, if we scaled a deployment down to 0 replicas? In that case I think you're totally right that 0/0 would be more useful. Totally fine to leave this as is then.

klingerf · 2018-06-27T00:01:06Z

web/app/js/components/MetricsTable.jsx

      this.api.ConduitLink
    ));

    let locale = {
-      emptyText: `No ${this.props.resource}s detected.`
+      emptyText: `No ${resource}s detected.`


Should we use friendlyTitle here?

klingerf · 2018-06-27T00:02:28Z

web/app/js/components/Sidebar.jsx

+                <Icon type="cloud-o" />
+                <span>Authorities</span>
+              </ConduitLink>
+            </Menu.Item>


Rather than adding this as a top-level menu item, it feels like it belongs in the resource sub-menu, alongside deployments, replication controllers, and pods.

klingerf

Glad to see integration tests! I had just a few more comments about getting the integration tests to pass in all environments.

klingerf · 2018-06-27T22:17:09Z

controller/api/public/stat_summary.go

-	}
-	if resource.Type != k8s.Namespaces && resource.Namespace != "" {
-		set[dstNamespaceLabel] = model.LabelValue(resource.Namespace)
+		if resource.Type == k8s.Authorities {


TIOLI you could use the isNonK8sResourceQuery method that you're adding here.

klingerf · 2018-06-27T22:57:08Z

controller/api/public/stat_summary.go

 	for _, k := range groupBy {
-		// return namespace/resource
-		values = append(values, string(metric[k]))
+		if string(k) == "namespace" && len(groupBy) == 2 && resourceNs == "" {


Hmm, I'm unfortunately having a pretty hard time reasoning about this method. I think we should go back to assuming that the groupBy slice is ordered, and that the namespace returned with the metric is the namespace to use when formulating the key. That will allow you to always use the last element in the slice as the key for the Name field, without needing to worry about the value of any of the elements in the slice. You could then rewrite as:

func metricToKey(req *pb.StatSummaryRequest, metric model.Metric, groupBy model.LabelNames) pb.Resource { key := pb.Resource{ Type: req.GetSelector().GetResource().GetType(), Name: string(metric[groupBy[len(groupBy)-1]]), } if len(groupBy) == 2 { key.Namespace = string(metric[groupBy[0]]) } return key }

That's more consistent with the original implementation, and afaict it works for all types including authorities.

klingerf · 2018-06-27T23:02:06Z

test/stat/stat_test.go

@@ -96,6 +105,18 @@ func TestCliStatForConduitNamespace(t *testing.T) {
 				TestHelper.GetConduitNamespace(): "4/4",
 			},
 		},
+		{
+			args: []string{"stat", "po", "-n", TestHelper.GetConduitNamespace(), "--to", "au/prometheus.conduit.svc.cluster.local:9090"},


Unfortunately "au/prometheus.conduit.svc.cluster.local:9090" only works when conduit is running in the default "conduit" namespace, which won't be the case in CI. Instead, I think you should define a separate prometheusAuthority var above:

prometheusAuthority := "prometheus." + TestHelper.GetConduitNamespace() + ".svc.cluster.local:9090"

And then use that here:

args: []string{"stat", "po", "-n", TestHelper.GetConduitNamespace(), "--to", "au/" + prometheusAuthority},

klingerf · 2018-06-27T23:03:57Z

test/stat/stat_test.go

+			expectedRows: map[string]string{
+				"prometheus.conduit.svc.cluster.local:9090": "-",
+			},
+		},


I think this test case would be more valuable if it were the inverse of the previous test. Specifically, the previous test requests stats from pods to a specified authority, and I think this one should test stats from authorities to a specified pod. I'd rewrite this test case as:

{ args: []string{"stat", "au", "-n", TestHelper.GetConduitNamespace(), "--to", "po/" + prometheusPod}, expectedRows: map[string]string{ prometheusAuthority: "-", }, },

klingerf · 2018-06-27T23:05:20Z

test/stat/stat_test.go

@@ -53,6 +53,15 @@ func TestCliStatForConduitNamespace(t *testing.T) {
 	}
 	prometheusPod := pods[0]

+	controllerPods, err := TestHelper.GetPodsForDeployment(TestHelper.GetConduitNamespace(), "controller")


Super minor, but you could reuse the pods and err variables from above, avoiding the needs for the new controllerPods var. I'd rewrite this as:

pods, err = TestHelper.GetPodsForDeployment(TestHelper.GetConduitNamespace(), "controller") if err != nil { t.Fatalf("Failed to get pods for controller: %s", err) } if len(pods) != 1 { t.Fatalf("Expected 1 pod for controller, got %d", len(pods)) } controllerPod := pods[0]

klingerf

⭐️ Thanks for adding those additional tests! This looks great.

klingerf · 2018-06-28T01:46:09Z

controller/api/public/stat_summary_test.go

+				},
+				mockPromResponse: model.Vector{
+					genPromSample("10.1.1.239:9995", "authority", "conduit", "success"),
+				},


TIOLI, since genPromSample doesn't generate data with dst_* labels, the data that it does generate isn't returned. So I think it might be clearer to not generate any prometheus data at all. The test will still pass if you change this too:

mockPromResponse: model.Vector{},

Same goes for the second test that you added too.

ahhhh thanks!

klingerf

⭐️ Looks great, thanks for fixing that link issue!

rmars added 4 commits June 21, 2018 14:27

Refactor: Combine objectMap and podStatsMap into one map

30d7373

Refactor: Use pb.Resource as a key to stats, rather than a string

ffdec41

Add Authority as a non-k8s Resource type

cafeffd

Add documentation to Resource so we can use it for authority Handle non-k8s resource requests Rewrite stat summary fetching and parsing to handle non-k8s resources Requesting an authority as primary breakout now works

Add tests for stat summary with resource type authority

63eac7e

rmars added area/controller area/cli area/telemetry labels Jun 21, 2018

rmars self-assigned this Jun 21, 2018

rmars requested review from klingerf, siggy and olix0r June 21, 2018 21:29

Ensure authority works with the CLI

5ec1f3f

rmars force-pushed the rmars/authority branch from 107a981 to 5ec1f3f Compare June 21, 2018 21:35

klingerf reviewed Jun 21, 2018

View reviewed changes

rmars added 2 commits June 21, 2018 16:32

Use pointer to PodCounts instead of OmitPodCounts

ad887bb

address review feedback

d4011c1

rmars added 2 commits June 21, 2018 16:53

change the CanonicalKubernetesName... functions to be used for other …

dec5153

…resources

Remove namespace from authority requests

ec2f248

klingerf reviewed Jun 22, 2018

View reviewed changes

klingerf mentioned this pull request Jun 22, 2018

Enable Authorities in web UI, under /authorities #1189

Merged

rmars added 4 commits June 25, 2018 09:51

Don't redefine Authority in util

73d4720

split up resourceQuery into k8sResourceQuery and nonK8sResourceQuery

53dced6

Refactor label name methods

1648c63

Adjust stat summary tests to include ns for authority queries

ae565fd

klingerf reviewed Jun 26, 2018

View reviewed changes

rmars added 3 commits June 26, 2018 11:29

Add authority to conduit stat

9754aae

Simplify metricToKey

afd9d16

Add authorities to conduit stat all

0530011

rmars added 5 commits June 26, 2018 15:58

s/Authority/Authorities

c4a4cdd

s/authority/authorities

32c5585

More s/Authority/Authorities

340c7d1

Enable Authorities in web UI, under /authorities (#1189)

5f2a184

* Enable Authorities in web UI, under /authorities

Merge branch 'master' into rmars/authority

eaa8258

rmars added this to the 0.5.0 milestone Jun 26, 2018

klingerf approved these changes Jun 27, 2018

View reviewed changes

rmars added 3 commits June 27, 2018 10:16

Make friendlyTitle a helper so it can be shared

ac4eb02

Use friendlyTitle in Namespace.jsx

09944c8

Merge branch 'master' into rmars/authority

f4fe2a0

olix0r mentioned this pull request Jun 27, 2018

Fully-implemented, resource-aware conduit stat #627

Closed

12 tasks

rmars added 3 commits June 27, 2018 12:16

Key the k8s object query and the prometheus query by the entire Resource

5b99845

Add some integration tests for authority

5516fe1

Fix namespaces not populating for --all-namespace queries

710283a

klingerf reviewed Jun 27, 2018

View reviewed changes

rmars added 6 commits June 27, 2018 17:22

Fix integration tests for stat

006e859

Add a stat test for --to resource queries

bfaeb4a

Add a stat test for --to resource queries

d892478

Add test for stat where --namespace and --to-namespace differ

b6c99c3

Fix test title

0b2fa8a

try to make metricToKey not totally horrible

fbb46cd

klingerf approved these changes Jun 28, 2018

View reviewed changes

Change mock response

1a53255

rmars mentioned this pull request Jun 28, 2018

Add Grafana dashboard for authorities #1225

Closed

rmars added 3 commits June 28, 2018 12:04

Don't link to grafana for authorities - dashboard not there yet

33f3e04

Fix authority check for displaying links

78a0cea

Add comments to note the assumtpions in metricToKey, clarify fn names

08d546b

klingerf approved these changes Jun 28, 2018

View reviewed changes

rmars merged commit 68586fe into master Jun 28, 2018

rmars deleted the rmars/authority branch June 28, 2018 21:31

		@@ -292,19 +364,21 @@ func buildRequestLabels(req *pb.StatSummaryRequest) (labels model.LabelSet, labe

Add the ability to query stats by authority #1181

Add the ability to query stats by authority #1181

Conversation

rmars commented Jun 21, 2018 • edited Loading

klingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmars commented Jun 21, 2018

klingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmars commented Jun 26, 2018

klingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klingerf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klingerf left a comment

Choose a reason for hiding this comment

rmars commented Jun 21, 2018 •

edited

Loading