Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

Conversation

@nxtcoder17
Copy link
Member

@nxtcoder17 nxtcoder17 commented May 21, 2024

Resolves kloudlite/kloudlite#215

  • observability API now communicates to target clusters via global VPN proxy, to fetch the list of currently running pods.

  • metrics and logs will now be shown only for currently running pods

- metrics will also be shown only for currently running pods
- repos.MatchFilter `NotInArray` support
@nxtcoder17 nxtcoder17 requested a review from karthik1729 as a code owner May 21, 2024 05:39
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @nxtcoder17 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

}

func (repo *dbRepo[T]) MergeMatchFilters(filter Filter, mFilter map[string]MatchFilter) Filter {
func (repo *dbRepo[T]) MergeMatchFilters(filter Filter, matchFilters ...map[string]MatchFilter) Filter {
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider logging a warning instead of using fmt.Printf.

Using a logging library instead of fmt.Printf for warnings would be more consistent with best practices and allow for better log management.

Suggested change
func (repo *dbRepo[T]) MergeMatchFilters(filter Filter, matchFilters ...map[string]MatchFilter) Filter {
import (
"log"
)
func (repo *dbRepo[T]) MergeMatchFilters(filter Filter, matchFilters ...map[string]MatchFilter) Filter {
if filter == nil {
filter = map[string]any{}
log.Println("Warning: filter was nil, initialized to an empty map")
}

@@ -0,0 +1,10 @@
package functions

func Reduce[T any, V any](items []T, reducerFn func(V, T), value V) V {
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider renaming reducerFn to reducer for consistency.

The parameter name reducerFn could be simplified to reducer for consistency with common naming conventions.

Suggested change
func Reduce[T any, V any](items []T, reducerFn func(V, T), value V) V {
func Reduce[T any, V any](items []T, reducer func(V, T), value V) V {

PromOperatorNotMatchRegex = PromOperator("!~")
)

type PromValue struct {
Copy link

Choose a reason for hiding this comment

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

issue (testing): Missing test for PromValue struct and its usage

Unit tests should be added to verify the correct usage of the PromValue struct, especially in the buildPromQuery and queryProm functions.

@nxtcoder17 nxtcoder17 merged commit e17741a into main May 21, 2024
@nxtcoder17 nxtcoder17 deleted the fix/observability branch May 21, 2024 05:41
abdheshnayak pushed a commit that referenced this pull request Nov 5, 2024
- observability API now communicates to target clusters via global VPN proxy, to fetch the list of currently running pods.

- metrics and logs will now be shown only for currently running pods
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Dashboard] Show Metrics only for currently running pods

2 participants