From a8856861f1823e06dde67a5e0865d53eb19cb733 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 6 Dec 2022 08:48:59 -0500 Subject: [PATCH] Fix change to get metrics route signature - The change was responding with an error instead of an empty response Signed-off-by: Sebastian Malton --- src/main/get-metrics.injectable.ts | 2 +- .../prometheus/helm-provider.injectable.ts | 7 + .../prometheus/lens-provider.injectable.ts | 7 + .../operator-provider.injectable.ts.ts | 7 + src/main/prometheus/provider.ts | 4 +- .../stacklight-provider.injectable.ts | 7 + .../metrics/add-metrics-route.injectable.ts | 138 ++++++++++++------ 7 files changed, 126 insertions(+), 46 deletions(-) diff --git a/src/main/get-metrics.injectable.ts b/src/main/get-metrics.injectable.ts index 0ceaf01735..88eab26e63 100644 --- a/src/main/get-metrics.injectable.ts +++ b/src/main/get-metrics.injectable.ts @@ -7,7 +7,7 @@ import type { Cluster } from "../common/cluster/cluster"; import type { RequestMetricsParams } from "../common/k8s-api/endpoints/metrics.api/request-metrics.injectable"; import k8sRequestInjectable from "./k8s-request.injectable"; -export type GetMetrics = (cluster: Cluster, prometheusPath: string, queryParams: RequestMetricsParams & { query: string }) => Promise; +export type GetMetrics = (cluster: Cluster, prometheusPath: string, queryParams: RequestMetricsParams & { query: string }) => Promise; const getMetricsInjectable = getInjectable({ id: "get-metrics", diff --git a/src/main/prometheus/helm-provider.injectable.ts b/src/main/prometheus/helm-provider.injectable.ts index 953bd68277..967dc6f88e 100644 --- a/src/main/prometheus/helm-provider.injectable.ts +++ b/src/main/prometheus/helm-provider.injectable.ts @@ -6,6 +6,7 @@ import type { PrometheusProvider } from "./provider"; import { createPrometheusProvider, bytesSent, findFirstNamespacedService, prometheusProviderInjectionToken } from "./provider"; import { getInjectable } from "@ogre-tools/injectable"; +import assert from "assert"; export const getHelmLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): PrometheusProvider["getQuery"] => ( (opts, queryName) => { @@ -105,6 +106,9 @@ export const getHelmLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): case "ingress": switch (queryName) { case "bytesSentSuccess": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentSuccess' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentSuccess' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, @@ -112,6 +116,9 @@ export const getHelmLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): statuses: "^2\\\\d*", }); case "bytesSentFailure": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentFailure' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentFailure' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, diff --git a/src/main/prometheus/lens-provider.injectable.ts b/src/main/prometheus/lens-provider.injectable.ts index c523c72582..b688d44130 100644 --- a/src/main/prometheus/lens-provider.injectable.ts +++ b/src/main/prometheus/lens-provider.injectable.ts @@ -6,6 +6,7 @@ import { bytesSent, prometheusProviderInjectionToken, findNamespacedService, createPrometheusProvider } from "./provider"; import type { PrometheusProvider } from "./provider"; import { getInjectable } from "@ogre-tools/injectable"; +import assert from "assert"; export const getLensLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): PrometheusProvider["getQuery"] => ( (opts, queryName) => { @@ -105,6 +106,9 @@ export const getLensLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): case "ingress": switch (queryName) { case "bytesSentSuccess": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentSuccess' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentSuccess' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, @@ -112,6 +116,9 @@ export const getLensLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): statuses: "^2\\\\d*", }); case "bytesSentFailure": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentFailure' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentFailure' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, diff --git a/src/main/prometheus/operator-provider.injectable.ts.ts b/src/main/prometheus/operator-provider.injectable.ts.ts index 5215f2dff2..eebf06ee26 100644 --- a/src/main/prometheus/operator-provider.injectable.ts.ts +++ b/src/main/prometheus/operator-provider.injectable.ts.ts @@ -6,6 +6,7 @@ import type { PrometheusProvider } from "./provider"; import { bytesSent, createPrometheusProvider, findFirstNamespacedService, prometheusProviderInjectionToken } from "./provider"; import { getInjectable } from "@ogre-tools/injectable"; +import assert from "assert"; export const getOperatorLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): PrometheusProvider["getQuery"] => ( (opts, queryName) => { @@ -105,6 +106,9 @@ export const getOperatorLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string case "ingress": switch (queryName) { case "bytesSentSuccess": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentSuccess' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentSuccess' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, @@ -112,6 +116,9 @@ export const getOperatorLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string statuses: "^2\\\\d*", }); case "bytesSentFailure": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentFailure' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentFailure' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, diff --git a/src/main/prometheus/provider.ts b/src/main/prometheus/provider.ts index 8944c4c0ed..51f75f573e 100644 --- a/src/main/prometheus/provider.ts +++ b/src/main/prometheus/provider.ts @@ -22,7 +22,7 @@ export interface PrometheusProvider { readonly name: string; readonly isConfigurable: boolean; - getQuery(opts: Record, queryName: string): string; + getQuery(opts: Partial>, queryName: string): string; getPrometheusService(client: CoreV1Api): Promise; } @@ -31,7 +31,7 @@ export interface CreatePrometheusProviderOpts { readonly name: string; readonly isConfigurable: boolean; - getQuery(opts: Record, queryName: string): string; + getQuery(opts: Partial>, queryName: string): string; getService(client: CoreV1Api): Promise; } diff --git a/src/main/prometheus/stacklight-provider.injectable.ts b/src/main/prometheus/stacklight-provider.injectable.ts index ca7a946466..b40ae0f661 100644 --- a/src/main/prometheus/stacklight-provider.injectable.ts +++ b/src/main/prometheus/stacklight-provider.injectable.ts @@ -6,6 +6,7 @@ import type { PrometheusProvider } from "./provider"; import { bytesSent, createPrometheusProvider, findFirstNamespacedService, prometheusProviderInjectionToken } from "./provider"; import { getInjectable } from "@ogre-tools/injectable"; +import assert from "assert"; export const getStacklightLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: string }): PrometheusProvider["getQuery"] => ( (opts, queryName) => { @@ -105,6 +106,9 @@ export const getStacklightLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: stri case "ingress": switch (queryName) { case "bytesSentSuccess": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentSuccess' for category='ingress'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentSuccess' for category='ingress'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, @@ -112,6 +116,9 @@ export const getStacklightLikeQueryFor = ({ rateAccuracy }: { rateAccuracy: stri statuses: "^2\\\\d*", }); case "bytesSentFailure": + assert(opts.ingress, "Missing option 'ingress' for query='bytesSentFailure'"); + assert(opts.namespace, "Missing option 'namespace' for query='bytesSentFailure'"); + return bytesSent({ rateAccuracy, ingress: opts.ingress, diff --git a/src/main/routes/metrics/add-metrics-route.injectable.ts b/src/main/routes/metrics/add-metrics-route.injectable.ts index ef963ae407..f47f80397a 100644 --- a/src/main/routes/metrics/add-metrics-route.injectable.ts +++ b/src/main/routes/metrics/add-metrics-route.injectable.ts @@ -8,50 +8,88 @@ import { getRouteInjectable } from "../../router/router.injectable"; import type { ClusterPrometheusMetadata } from "../../../common/cluster-types"; import { ClusterMetadataKey } from "../../../common/cluster-types"; import type { Cluster } from "../../../common/cluster/cluster"; -import { clusterRoute } from "../../router/route"; -import { isObject } from "lodash"; -import { isRequestError, object } from "../../../common/utils"; +import { payloadValidatedClusterRoute } from "../../router/route"; +import { getOrInsertWith, isRequestError, object } from "../../../common/utils"; import type { GetMetrics } from "../../get-metrics.injectable"; import getMetricsInjectable from "../../get-metrics.injectable"; import loggerInjectable from "../../../common/logger.injectable"; +import type { AsyncResult } from "../../../common/utils/async-result"; +import Joi from "joi"; // This is used for backoff retry tracking. -const ATTEMPTS = [false, false, false, false, true]; +const MAX_ATTEMPTS = 5; -const loadMetricsFor = (getMetrics: GetMetrics) => async (promQueries: string[], cluster: Cluster, prometheusPath: string, queryParams: Partial>): Promise => { +const loadMetricsFor = (getMetrics: GetMetrics) => async (promQueries: string[], cluster: Cluster, prometheusPath: string, queryParams: Partial>): Promise> => { const queries = promQueries.map(p => p.trim()); - const loaders = new Map>(); + const loaders = new Map>>(); - async function loadMetric(query: string): Promise { - async function loadMetricHelper(): Promise { - for (const [attempt, lastAttempt] of ATTEMPTS.entries()) { // retry - try { - return await getMetrics(cluster, prometheusPath, { query, ...queryParams }); - } catch (error) { - if ( - !isRequestError(error) - || lastAttempt - || ( - !lastAttempt && ( - typeof error.statusCode === "number" && - 400 <= error.statusCode && error.statusCode < 500 - ) + async function loadMetric(query: string): Promise> { + async function loadMetricHelper(attempt: number): Promise> { + const lastAttempt = attempt === MAX_ATTEMPTS; + + try { + return { + callWasSuccessful: true, + response: await getMetrics(cluster, prometheusPath, { query, ...queryParams }), + }; + } catch (error) { + if ( + !isRequestError(error) + || lastAttempt + || ( + !lastAttempt && ( + typeof error.statusCode === "number" && + 400 <= error.statusCode && error.statusCode < 500 ) - ) { - throw new Error("Metrics not available", { cause: error }); - } - - await new Promise(resolve => setTimeout(resolve, (attempt + 1) * 1000)); // add delay before repeating request + ) + ) { + return { + callWasSuccessful: false, + error: new Error("Metrics not available", { cause: error }), + }; } + + await new Promise(resolve => setTimeout(resolve, (attempt + 1) * 1000)); // add delay before repeating request } + + return loadMetricHelper(attempt + 1); } - return loaders.get(query) ?? loaders.set(query, loadMetricHelper()).get(query); + return getOrInsertWith(loaders, query, () => loadMetricHelper(0)); } - return Promise.all(queries.map(loadMetric)); + const responses: unknown[] = []; + + for await (const result of queries.map(loadMetric)) { + if (result.callWasSuccessful) { + responses.push(result.response); + } else { + return result; + } + } + + return { + callWasSuccessful: true, + response: responses, + }; }; +type ClusterMetricsPayload = string | string[] | Partial>>>; + +const clusterMetricsPayloadValidator = Joi.alternatives( + Joi.string(), + Joi.array().items(Joi.string()), + Joi.object() + .pattern( + Joi.string(), + Joi.object() + .pattern( + Joi.string(), + Joi.string(), + ), + ), +); + const addMetricsRouteInjectable = getRouteInjectable({ id: "add-metrics-route", @@ -60,9 +98,10 @@ const addMetricsRouteInjectable = getRouteInjectable({ const loadMetrics = loadMetricsFor(getMetrics); const logger = di.inject(loggerInjectable); - return clusterRoute({ + return payloadValidatedClusterRoute({ method: "post", path: `${apiPrefix}/metrics`, + payloadValidator: clusterMetricsPayloadValidator, })(async ({ cluster, payload, query }) => { const queryParams: Partial> = Object.fromEntries(query.entries()); const prometheusMetadata: ClusterPrometheusMetadata = {}; @@ -95,33 +134,46 @@ const addMetricsRouteInjectable = getRouteInjectable({ // return data in same structure as query if (typeof payload === "string") { - const [data] = await loadMetrics([payload], cluster, prometheusPath, queryParams); + const result = await loadMetrics([payload], cluster, prometheusPath, queryParams); - return { response: data }; + if (result.callWasSuccessful) { + return { response: result.response[0] }; + } + + logger.warn(`[METRICS-ROUTE]: failed to get metrics for clusterId=${cluster.id}:`, result.error); + + return { response: {}}; } if (Array.isArray(payload)) { - const data = await loadMetrics(payload, cluster, prometheusPath, queryParams); + const result = await loadMetrics(payload, cluster, prometheusPath, queryParams); - return { response: data }; + if (result.callWasSuccessful) { + return { response: result.response }; + } + + logger.warn(`[METRICS-ROUTE]: failed to get metrics for clusterId=${cluster.id}:`, result.error); + + return { response: {}}; } - if (isObject(payload)) { - const data = payload as Record>; - const queries = object.entries(data) - .map(([queryName, queryOpts]) => ( - provider.getQuery(queryOpts, queryName) - )); + // Last option + const queries = object.entries(payload) + .map(([queryName, queryOpts]) => ( + provider.getQuery(queryOpts, queryName) + )); - const result = await loadMetrics(queries, cluster, prometheusPath, queryParams); - const response = object.fromEntries(object.keys(data).map((metricName, i) => [metricName, result[i]])); + const result = await loadMetrics(queries, cluster, prometheusPath, queryParams); - prometheusMetadata.success = true; + if (!result.callWasSuccessful) { + logger.warn(`[METRICS-ROUTE]: failed to get metrics for clusterId=${cluster.id}:`, result.error); - return { response }; + return { response: {}}; } - return { response: {}}; + const response = object.fromEntries(object.keys(payload).map((metricName, i) => [metricName, result.response[i]])); + + return { response }; }); }, });