Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions pkg/handler/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (h *Handlers) GetClusters(ctx context.Context) func(w http.ResponseWriter,
// Fetch and merge values for K8S_ClusterName
values, code, err := h.getLabelValues(ctx, clients, fields.Cluster)
if err != nil {
writeError(w, code, "Error while fetching label cluster values from Loki: "+err.Error())
writeError(w, code, err.Error())
return
}

Expand Down Expand Up @@ -65,14 +65,14 @@ func (h *Handlers) GetZones(ctx context.Context) func(w http.ResponseWriter, r *
// Fetch and merge values for SrcK8S_Zone and DstK8S_Zone
values1, code, err := h.getLabelValues(ctx, clients, fields.SrcZone)
if err != nil {
writeError(w, code, "Error while fetching label source zone values from Loki: "+err.Error())
writeError(w, code, err.Error())
return
}
values = append(values, values1...)

values2, code, err := h.getLabelValues(ctx, clients, fields.DstZone)
if err != nil {
writeError(w, code, "Error while fetching label destination zone values from Loki: "+err.Error())
writeError(w, code, err.Error())
return
}
values = append(values, values2...)
Expand Down Expand Up @@ -104,14 +104,14 @@ func (h *Handlers) GetNamespaces(ctx context.Context) func(w http.ResponseWriter
// Fetch and merge values for SrcK8S_Namespace and DstK8S_Namespace
values1, code, err := h.getLabelValues(ctx, clients, fields.SrcNamespace)
if err != nil {
writeError(w, code, "Error while fetching label source namespace values from Loki: "+err.Error())
writeError(w, code, err.Error())
return
}
values = append(values, values1...)

values2, code, err := h.getLabelValues(ctx, clients, fields.DstNamespace)
if err != nil {
writeError(w, code, "Error while fetching label destination namespace values from Loki: "+err.Error())
writeError(w, code, err.Error())
return
}
values = append(values, values2...)
Expand All @@ -123,10 +123,27 @@ func (h *Handlers) GetNamespaces(ctx context.Context) func(w http.ResponseWriter

func (h *Handlers) getLabelValues(ctx context.Context, cl clients, label string) ([]string, int, error) {
if h.PromInventory != nil && h.PromInventory.LabelExists(label) {
return prometheus.GetLabelValues(ctx, cl.promAdmin, label, nil)
resp, code, err := prometheus.GetLabelValues(ctx, cl.promAdmin, label, nil)
if err != nil {
if code == http.StatusUnauthorized || code == http.StatusForbidden {
// In case this was a prometheus 401 / 403 error, the query is repeated with Loki
// This is because multi-tenancy is currently not managed for prom datasource, hence such queries have to go with Loki
// Unfortunately we don't know a safe and generic way to pre-flight check if the user will be authorized
hlog.Info("Retrying with Loki...")
// continuing with loki below
} else {
return nil, code, fmt.Errorf("error while fetching label %s values from Prometheus: %w", label, err)
}
Comment on lines +128 to +136
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we'll need that case as we get https://github.com/netobserv/network-observability-console-plugin/pull/589/files#diff-017f9ad356145a3e9c8d08ec33d1b5e694bc4820f301ad6d5c6470005eaf8a38 changes 🤔

The namespace was missing in the query params so we always query the admin prom

Copy link
Member Author

@jotak jotak Sep 9, 2024

Choose a reason for hiding this comment

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

the error is not only in dev perspective but also in admin perspective, when logged in with a non-cluster-admin.. so I think it's still relevant for that case

Copy link
Contributor

Choose a reason for hiding this comment

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

correct, let's keep it then !

} else {
return resp, code, nil
}
}
if h.Cfg.IsLokiEnabled() {
return getLokiLabelValues(h.Cfg.Loki.URL, cl.loki, label)
resp, code, err := getLokiLabelValues(h.Cfg.Loki.URL, cl.loki, label)
if err != nil {
return nil, code, fmt.Errorf("error while fetching label %s values from Loki: %w", label, err)
}
return resp, code, nil
}
// Loki disabled AND label not managed in metrics => send an error
return nil, http.StatusBadRequest, fmt.Errorf("label %s not found in Prometheus metrics", label)
Expand Down Expand Up @@ -186,7 +203,7 @@ func (h *Handlers) getNamesForPrefix(ctx context.Context, cl clients, prefix, ki
searchField = prefix + fields.Name
}

if h.Cfg.IsPromEnabled() {
if h.Cfg.IsPromEnabled() && h.PromInventory.LabelExists(searchField) {
// Label match query (any metric)
q := prometheus.QueryFilters("", filts)
return prometheus.GetLabelValues(ctx, cl.promAdmin, searchField, []string{q})
Expand Down
29 changes: 17 additions & 12 deletions pkg/prometheus/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,7 @@ func executeQueryRange(ctx context.Context, cl api.Client, q *Query) (pmod.Value
}
if err != nil {
log.Tracef("Error:\n%v", err)
code = http.StatusServiceUnavailable
var promError *v1.Error
if errors.As(err, &promError) {
if promError.Type == v1.ErrClient && strings.Contains(promError.Msg, "401") {
code = http.StatusUnauthorized
} else if promError.Type == v1.ErrClient && strings.Contains(promError.Msg, "403") {
code = http.StatusForbidden
}
}
code = translateErrorCode(err)
return nil, code, fmt.Errorf("error from Prometheus query: %w", err)
}

Expand Down Expand Up @@ -125,16 +117,29 @@ func GetLabelValues(ctx context.Context, cl api.Client, label string, match []st
log.Debugf("GetLabelValues: %s", label)
v1api := v1.NewAPI(cl)
result, warnings, err := v1api.LabelValues(ctx, label, match, time.Now().Add(-3*time.Hour), time.Now())
if err != nil {
return nil, http.StatusServiceUnavailable, err
}
if len(warnings) > 0 {
log.Infof("GetLabelValues warnings: %v", warnings)
}
if err != nil {
code := translateErrorCode(err)
return nil, code, fmt.Errorf("could not get label values: %w", err)
}
log.Tracef("Result:\n%v", result)
var asStrings []string
for _, s := range result {
asStrings = append(asStrings, string(s))
}
return asStrings, http.StatusOK, nil
}

func translateErrorCode(err error) int {
var promError *v1.Error
if errors.As(err, &promError) {
if promError.Type == v1.ErrClient && strings.Contains(promError.Msg, "401") {
return http.StatusUnauthorized
} else if promError.Type == v1.ErrClient && strings.Contains(promError.Msg, "403") {
return http.StatusForbidden
}
}
return http.StatusServiceUnavailable
}
4 changes: 1 addition & 3 deletions web/src/components/toolbar/filters/autocomplete-filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ export const AutocompleteFilter: React.FC<AutocompleteFilterProps> = ({
setCurrentValue(newValue);
filterDefinition
.getOptions(newValue)
.then(opts => {
setOptions(opts);
})
.then(setOptions)
.catch(err => {
const errorMessage = getHTTPErrorDetails(err);
setMessageWithDelay(errorMessage);
Expand Down
14 changes: 10 additions & 4 deletions web/src/model/filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,16 @@
}

export const createFilterValue = (def: FilterDefinition, value: string): Promise<FilterValue> => {
return def.getOptions(value).then(opts => {
const option = opts.find(opt => opt.name === value || opt.value === value);
return option ? { v: option.value, display: option.name } : { v: value };
});
return def
.getOptions(value)
.then(opts => {
const option = opts.find(opt => opt.name === value || opt.value === value);
return option ? { v: option.value, display: option.name } : { v: value };
})
.catch(_ => {

Check warning on line 101 in web/src/model/filters.ts

View workflow job for this annotation

GitHub Actions / Build, lint, test frontend

'_' is defined but never used
// In case of error, still create the minimal possible FilterValue
return { v: value };
});
};

export const hasEnabledFilterValues = (filter: Filter) => {
Expand Down
Loading