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

Add namespace as a resource type in public-api #760

Merged
merged 4 commits into from
Apr 13, 2018
Merged

Conversation

siggy
Copy link
Member

@siggy siggy commented Apr 13, 2018

The cli and public-api only supported deployments as a resource type.

This change adds support for namespace as a resource type in the cli and
public-api. This also change includes:

  • cli statsummary now prints -'s when objects are not in the mesh
  • cli statsummary prints No resources found. when applicable
  • removed out- from cli statsummary flags, and analogous proto changes
  • switched public-api to use native prometheus label types
  • misc error handling and logging fixes

Part of #627

Signed-off-by: Andrew Seigner siggy@buoyant.io

Example cli output:

$ conduit statsummary ns
NAME          MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99
conduit          4/4   100.00%   5.7rps          17ms         137ms         230ms
default          0/0         -        -             -             -             -
docker           0/2         -        -             -             -             -
emojivoto        4/4    91.84%   4.9rps           1ms          17ms          35ms
kube-public      0/0         -        -             -             -             -
kube-system      0/6         -        -             -             -             -

$ conduit statsummary deploy -n emojivoto --to emoji
NAME   MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99
web       1/1   100.00%   1.9rps           2ms           4ms          18ms

$ conduit statsummary deploy -n emojivoto --to emojivoto --to-resource ns
NAME       MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99
vote-bot      1/1    88.03%   1.9rps           5ms          15ms          19ms
web           1/1    77.97%   1.0rps           2ms           2ms           2ms

$ conduit statsummary deploy -n foo
No resources found.

@kl This is going to conflict with #759. I lifted a couple code snippets from your PR around flag descriptions and filtering out rows on outbound requests. We made identical proto changes. The changes in controller/api/public/stat_summary.go to accommodate namespaces are significant though, let's figure out a merge strategy.

@siggy siggy added this to the 0.4.0 milestone Apr 13, 2018
@siggy siggy self-assigned this Apr 13, 2018
@siggy siggy requested a review from klingerf April 13, 2018 12:36
@siggy siggy added the review label Apr 13, 2018
@klingerf klingerf force-pushed the siggy/ns-stats branch 2 times, most recently from cf06fd8 to ceed490 Compare April 13, 2018 19:14
siggy and others added 4 commits April 13, 2018 16:02
The cli and public-api only supported deployments as a resource type.

This change adds support for namespace as a resource type in the cli and
public-api. This also change includes:
- cli statsummary now prints `-`'s when objects are not in the mesh
- cli statsummary prints `No resources found.` when applicable
- removed `out-` from cli statsummary flags, and analagous proto changes
- switched public-api to use native prometheus label types
- misc error handling and logging fixes

Part of #627

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>
Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>
Signed-off-by: Kevin Lingerfelt <kl@buoyant.io>
Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

⭐️ Nice work, @siggy! Looks good to me.

@siggy siggy merged commit 77fb6d3 into master Apr 13, 2018
@siggy siggy deleted the siggy/ns-stats branch April 13, 2018 23:53
@siggy siggy removed the review label Apr 13, 2018
@siggy
Copy link
Member Author

siggy commented Apr 13, 2018

Awesome pairing on this @klingerf !! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants