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

feat(kuma-cp) add GlobalInsights api endpoint #3018

Merged
merged 2 commits into from
Oct 27, 2021

Conversation

bartsmykla
Copy link
Contributor

@bartsmykla bartsmykla commented Oct 27, 2021

Summary

/global-insights is an endpoint which will return aggregated
statistics for global resources, like Zone, ZoneIngress or
Mesh. It's purpose is simmilar to the MeshInsights, but for
global resources, and will be used in our GUI to simplify
displaying statistics of these objects.

I've spent to much time thinking about the approach for adding
this endpoint and finally decided to go with the easiest one,
which is just asking for Zones, ZoneIngresses and Meshes
every time endpoint is hitted as the GlobalInsights is different
than other resources like MeshInsights as there always will be
one. The assumption is also, that there won't be a lot of
the resources we'll be asking for every endpoint hit.

Other solutions considered were:

  1. Adding GlobalInsights as the real resource, like
    MeshInsights, but then we would have to create separate k8s'
    CRD, for just the resource which would exist only one at a time.
    Also, as it's the global resource, reconciliation of it would
    be more complex.
  2. Storing the internal data in Config resource (which maps to
    json string in k8s' ConfigMap), but I really think it wouldn't
    be right, as I think it would be not obvious and harder to
    maintain in the future. The complexity of reconciliation would
    be the same as in the option above.

I think this solution is very easy, and if anyhow it will become
performance bottleneck, we can just improve it/refactor it, without
breaking backward compatibility.

Full changelog

no changelog

Issues resolved

no issue resolved

Documentation

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

`/global-insights` is an endpoint which will return aggregated
statistics for global resources, like `Zone`, `ZoneIngress` or
`Mesh`. It's purpose is simmilar to the `MeshInsights`, but for
global resources, and will be used in our GUI to simplify
displaying statistics of these objects.

I've spent to much time thinking about the approach for adding
this endpoint and finally decided to go with the easiest one,
which is just asking for `Zone`s, `ZoneIngress`es and `Mesh`es
every time endpoint is hitted as the `GlobalInsights` is different
than other resources like `MeshInsights` as there always will be
one. The assumption is also, that there won't be a lot of
the resources we'll be asking for every endpoint hit.

Other solutions considered were:
1. Adding `GlobalInsights` as the real resource, like
   `MeshInsights`, but then we would have to create separate k8s'
   CRD, for just the resource which would exist only one at a time.
   Also, as it's the global resource, reconciliation of it would
   be more complex.
2. Storing the internal data in Config resource (which maps to
   json string in k8s' ConfigMap), but I really think it wouldn't
   be right, as I think it would be not obvious and narder to
   maintain in the future. The complexity of reconciliation would
   be the same as in the option above.

I think this solution is very easy, and if anyhow it will become
performance bottleneck, we can just improve it/refactor it, without
breaking backward compatibility.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla bartsmykla requested a review from a team as a code owner October 27, 2021 06:24
@codecov-commenter
Copy link

Codecov Report

Merging #3018 (d58a417) into master (9aeea34) will increase coverage by 0.03%.
The diff coverage is 68.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3018      +/-   ##
==========================================
+ Coverage   52.25%   52.29%   +0.03%     
==========================================
  Files         913      914       +1     
  Lines       52906    52941      +35     
==========================================
+ Hits        27645    27684      +39     
+ Misses      23059    23050       -9     
- Partials     2202     2207       +5     
Impacted Files Coverage Δ
pkg/api-server/global_insights_endpoints.go 64.51% <64.51%> (ø)
pkg/api-server/server.go 82.09% <100.00%> (+0.31%) ⬆️
pkg/core/resources/model/rest/resource.go 67.94% <0.00%> (-1.29%) ⬇️
pkg/xds/generator/direct_access_proxy_generator.go 82.75% <0.00%> (-1.15%) ⬇️
pkg/dns/vips_allocator.go 74.00% <0.00%> (+1.33%) ⬆️
pkg/core/resources/manager/cache.go 88.31% <0.00%> (+2.59%) ⬆️
pkg/core/bootstrap/autoconfig.go 54.46% <0.00%> (+8.03%) ⬆️
pkg/plugins/runtime/gateway/route/sorter.go 71.79% <0.00%> (+10.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9aeea34...d58a417. Read the comment docs.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla bartsmykla merged commit 636e31c into kumahq:master Oct 27, 2021
@bartsmykla bartsmykla deleted the feat/global-insights branch October 27, 2021 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants