Skip to content

Commit b5480e9

Browse files
committed
Fix bugs in the code and support a different ruler uri
Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>
1 parent c047583 commit b5480e9

11 files changed

Lines changed: 549 additions & 279 deletions

File tree

direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClient.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,15 @@ Map<String, List<MetricMetadata>> getAllMetrics(Map<String, String> queryParams)
5858
JSONObject getAlerts() throws IOException;
5959

6060
/**
61-
* Get all recording and alerting rules. Returns the raw response body since the format varies by
62-
* backend: Prometheus returns JSON, Cortex/Thanos returns YAML, and AMP returns JSON.
61+
* Get all recording and alerting rules, normalized to a consistent JSON format. Handles
62+
* Prometheus JSON, Cortex/Thanos YAML, and AMP JSON responses, returning them all as a
63+
* {"groups":[...]} structure.
6364
*
6465
* @param queryParams Map of query parameters to include in the request
65-
* @return String containing the raw response body
66+
* @return JSONObject with {"groups":[...]} structure
6667
* @throws IOException If there is an issue with the request
6768
*/
68-
String getRules(Map<String, String> queryParams) throws IOException;
69+
JSONObject getRules(Map<String, String> queryParams) throws IOException;
6970

7071
/**
7172
* Get all alerts from Alertmanager.
@@ -110,15 +111,16 @@ Map<String, List<MetricMetadata>> getAllMetrics(Map<String, String> queryParams)
110111
String createAlertmanagerSilences(String silenceJson) throws IOException;
111112

112113
/**
113-
* Get rules for a specific namespace from the Cortex/Thanos Ruler API. The response is returned
114-
* as a raw string since the Ruler API returns YAML (Cortex/Thanos) or JSON (AMP).
114+
* Get rules for a specific namespace, normalized to a consistent JSON format. Handles
115+
* Cortex/Thanos YAML and AMP JSON responses, returning them all as a {"groups":[...]} structure.
115116
*
116117
* @param namespace The rules namespace
117118
* @param queryParams Map of query parameters to include in the request
118-
* @return String containing the raw response body (YAML or JSON)
119+
* @return JSONObject with {"groups":[...]} structure
119120
* @throws IOException If there is an issue with the request
120121
*/
121-
String getRulesByNamespace(String namespace, Map<String, String> queryParams) throws IOException;
122+
JSONObject getRulesByNamespace(String namespace, Map<String, String> queryParams)
123+
throws IOException;
122124

123125
/**
124126
* Create or update a rule group in a namespace via the Cortex/Thanos Ruler API.

direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java

Lines changed: 134 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import com.fasterxml.jackson.core.type.TypeReference;
99
import com.fasterxml.jackson.databind.ObjectMapper;
10+
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
1011
import java.io.IOException;
1112
import java.net.URI;
1213
import java.net.URLEncoder;
@@ -36,30 +37,44 @@
3637
public class PrometheusClientImpl implements PrometheusClient {
3738

3839
private static final Logger logger = LogManager.getLogger(PrometheusClientImpl.class);
40+
private static final ObjectMapper YAML_MAPPER = new ObjectMapper(new YAMLFactory());
3941

4042
private final OkHttpClient prometheusHttpClient;
4143
private final OkHttpClient alertmanagerHttpClient;
4244

4345
private final URI prometheusUri;
4446
private final URI alertmanagerUri;
47+
private final URI rulerUri;
4548

4649
public PrometheusClientImpl(OkHttpClient prometheusHttpClient, URI prometheusUri) {
4750
this(
4851
prometheusHttpClient,
4952
prometheusUri,
5053
prometheusHttpClient,
51-
URI.create(prometheusUri.toString().replaceAll("/$", "") + "/alertmanager"));
54+
URI.create(prometheusUri.toString().replaceAll("/$", "") + "/alertmanager"),
55+
prometheusUri);
5256
}
5357

5458
public PrometheusClientImpl(
5559
OkHttpClient prometheusHttpClient,
5660
URI prometheusUri,
5761
OkHttpClient alertmanagerHttpClient,
5862
URI alertmanagerUri) {
63+
this(prometheusHttpClient, prometheusUri, alertmanagerHttpClient, alertmanagerUri,
64+
prometheusUri);
65+
}
66+
67+
public PrometheusClientImpl(
68+
OkHttpClient prometheusHttpClient,
69+
URI prometheusUri,
70+
OkHttpClient alertmanagerHttpClient,
71+
URI alertmanagerUri,
72+
URI rulerUri) {
5973
this.prometheusHttpClient = prometheusHttpClient;
6074
this.prometheusUri = prometheusUri;
6175
this.alertmanagerHttpClient = alertmanagerHttpClient;
6276
this.alertmanagerUri = alertmanagerUri;
77+
this.rulerUri = rulerUri;
6378
}
6479

6580
private String paramsToQueryString(Map<String, String> queryParams) {
@@ -229,38 +244,18 @@ public JSONObject getAlerts() throws IOException {
229244
}
230245

231246
@Override
232-
public String getRules(Map<String, String> queryParams) throws IOException {
247+
public JSONObject getRules(Map<String, String> queryParams) throws IOException {
233248
String queryString = this.paramsToQueryString(queryParams);
234249
String queryUrl =
235250
String.format(
236251
"%s/api/v1/rules%s", prometheusUri.toString().replaceAll("/$", ""), queryString);
237-
logger.debug("Making Ruler GET request for all rules: {}", queryUrl);
252+
logger.debug("Making Ruler GET request for all rules");
238253
Request request = new Request.Builder().url(queryUrl).build();
239-
Response response =
254+
try (Response response =
240255
AccessController.doPrivilegedChecked(
241-
() -> this.prometheusHttpClient.newCall(request).execute());
242-
243-
if (response.isSuccessful()) {
244-
String bodyString = Objects.requireNonNull(response.body()).string();
245-
// Try to extract data from Prometheus JSON format; return raw body otherwise
246-
try {
247-
JSONObject jsonObject = new JSONObject(bodyString);
248-
if ("success".equals(jsonObject.optString("status"))
249-
&& jsonObject.has("data")) {
250-
return jsonObject.getJSONObject("data").toString();
251-
}
252-
} catch (JSONException e) {
253-
// Not JSON (e.g. Cortex YAML) — fall through
254-
}
255-
return bodyString;
256-
} else {
257-
String errorBody = response.body() != null ? response.body().string() : "No response body";
258-
logger.error(
259-
"Ruler GET request failed with code: {}, error body: {}", response.code(), errorBody);
260-
throw new PrometheusClientException(
261-
String.format(
262-
"Ruler request failed with code: %s. Error details: %s",
263-
response.code(), errorBody));
256+
() -> this.prometheusHttpClient.newCall(request).execute())) {
257+
String body = readRulerResponse(response, "GET all rules");
258+
return normalizeRulesResponse(body, null);
264259
}
265260
}
266261

@@ -341,31 +336,22 @@ public String createAlertmanagerSilences(String silenceJson) throws IOException
341336
}
342337

343338
@Override
344-
public String getRulesByNamespace(String namespace, Map<String, String> queryParams)
339+
public JSONObject getRulesByNamespace(String namespace, Map<String, String> queryParams)
345340
throws IOException {
346341
String queryString = this.paramsToQueryString(queryParams);
347342
String queryUrl =
348343
String.format(
349344
"%s/api/v1/rules/%s%s",
350-
prometheusUri.toString().replaceAll("/$", ""),
345+
rulerUri.toString().replaceAll("/$", ""),
351346
URLEncoder.encode(namespace, StandardCharsets.UTF_8),
352347
queryString);
353-
logger.debug("Making Ruler GET request for namespace: {}", queryUrl);
348+
logger.debug("Making Ruler GET request for namespace");
354349
Request request = new Request.Builder().url(queryUrl).build();
355-
Response response =
350+
try (Response response =
356351
AccessController.doPrivilegedChecked(
357-
() -> this.prometheusHttpClient.newCall(request).execute());
358-
359-
if (response.isSuccessful()) {
360-
return Objects.requireNonNull(response.body()).string();
361-
} else {
362-
String errorBody = response.body() != null ? response.body().string() : "No response body";
363-
logger.error(
364-
"Ruler GET request failed with code: {}, error body: {}", response.code(), errorBody);
365-
throw new PrometheusClientException(
366-
String.format(
367-
"Ruler request failed with code: %s. Error details: %s",
368-
response.code(), errorBody));
352+
() -> this.prometheusHttpClient.newCall(request).execute())) {
353+
String body = readRulerResponse(response, "GET namespace " + namespace);
354+
return normalizeRulesResponse(body, namespace);
369355
}
370356
}
371357

@@ -374,30 +360,20 @@ public String createOrUpdateRuleGroup(String namespace, String yamlBody) throws
374360
String queryUrl =
375361
String.format(
376362
"%s/api/v1/rules/%s",
377-
prometheusUri.toString().replaceAll("/$", ""),
363+
rulerUri.toString().replaceAll("/$", ""),
378364
URLEncoder.encode(namespace, StandardCharsets.UTF_8));
379-
logger.debug("Making Ruler POST request to create/update rule group: {}", queryUrl);
365+
logger.debug("Making Ruler POST request to create/update rule group");
380366
Request request =
381367
new Request.Builder()
382368
.url(queryUrl)
383369
.header("Content-Type", "application/yaml")
384370
.post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8)))
385371
.build();
386-
Response response =
372+
try (Response response =
387373
AccessController.doPrivilegedChecked(
388-
() -> this.prometheusHttpClient.newCall(request).execute());
389-
390-
if (response.isSuccessful()) {
391-
String body = response.body() != null ? response.body().string() : "";
374+
() -> this.prometheusHttpClient.newCall(request).execute())) {
375+
String body = readRulerResponse(response, "POST create/update rule group");
392376
return body.isEmpty() ? "{\"status\":\"success\"}" : body;
393-
} else {
394-
String errorBody = response.body() != null ? response.body().string() : "No response body";
395-
logger.error(
396-
"Ruler POST request failed with code: {}, error body: {}", response.code(), errorBody);
397-
throw new PrometheusClientException(
398-
String.format(
399-
"Ruler request failed with code: %s. Error details: %s",
400-
response.code(), errorBody));
401377
}
402378
}
403379

@@ -406,24 +382,15 @@ public String deleteRuleNamespace(String namespace) throws IOException {
406382
String queryUrl =
407383
String.format(
408384
"%s/api/v1/rules/%s",
409-
prometheusUri.toString().replaceAll("/$", ""),
385+
rulerUri.toString().replaceAll("/$", ""),
410386
URLEncoder.encode(namespace, StandardCharsets.UTF_8));
411-
logger.debug("Making Ruler DELETE request for namespace: {}", queryUrl);
387+
logger.debug("Making Ruler DELETE request for namespace");
412388
Request request = new Request.Builder().url(queryUrl).delete().build();
413-
Response response =
389+
try (Response response =
414390
AccessController.doPrivilegedChecked(
415-
() -> this.prometheusHttpClient.newCall(request).execute());
416-
417-
if (response.isSuccessful()) {
418-
return "{\"status\":\"success\"}";
419-
} else {
420-
String errorBody = response.body() != null ? response.body().string() : "No response body";
421-
logger.error(
422-
"Ruler DELETE request failed with code: {}, error body: {}", response.code(), errorBody);
423-
throw new PrometheusClientException(
424-
String.format(
425-
"Ruler request failed with code: %s. Error details: %s",
426-
response.code(), errorBody));
391+
() -> this.prometheusHttpClient.newCall(request).execute())) {
392+
String body = readRulerResponse(response, "DELETE namespace " + namespace);
393+
return body.isEmpty() ? "{\"status\":\"success\"}" : body;
427394
}
428395
}
429396

@@ -432,28 +399,116 @@ public String deleteRuleGroup(String namespace, String groupName) throws IOExcep
432399
String queryUrl =
433400
String.format(
434401
"%s/api/v1/rules/%s/%s",
435-
prometheusUri.toString().replaceAll("/$", ""),
402+
rulerUri.toString().replaceAll("/$", ""),
436403
URLEncoder.encode(namespace, StandardCharsets.UTF_8),
437404
URLEncoder.encode(groupName, StandardCharsets.UTF_8));
438-
logger.debug("Making Ruler DELETE request for group: {}", queryUrl);
405+
logger.debug("Making Ruler DELETE request for group");
439406
Request request = new Request.Builder().url(queryUrl).delete().build();
440-
Response response =
407+
try (Response response =
441408
AccessController.doPrivilegedChecked(
442-
() -> this.prometheusHttpClient.newCall(request).execute());
409+
() -> this.prometheusHttpClient.newCall(request).execute())) {
410+
String body = readRulerResponse(response, "DELETE group " + groupName);
411+
return body.isEmpty() ? "{\"status\":\"success\"}" : body;
412+
}
413+
}
443414

415+
/**
416+
* Reads a Ruler API response, returning the body string on success or throwing on failure.
417+
* Consolidates the error-handling pattern shared by all Ruler methods.
418+
*
419+
* @param response The HTTP response
420+
* @param operationDescription Description for log messages (e.g., "GET all rules")
421+
* @return The response body as a string (empty string if body is null)
422+
* @throws IOException If there is an issue reading the response
423+
*/
424+
private String readRulerResponse(Response response, String operationDescription)
425+
throws IOException {
444426
if (response.isSuccessful()) {
445-
return "{\"status\":\"success\"}";
427+
return Objects.requireNonNull(response.body(), "Ruler response body is null").string();
446428
} else {
447429
String errorBody = response.body() != null ? response.body().string() : "No response body";
448430
logger.error(
449-
"Ruler DELETE request failed with code: {}, error body: {}", response.code(), errorBody);
431+
"Ruler {} request failed with code: {}, error body: {}",
432+
operationDescription,
433+
response.code(),
434+
errorBody);
450435
throw new PrometheusClientException(
451436
String.format(
452437
"Ruler request failed with code: %s. Error details: %s",
453438
response.code(), errorBody));
454439
}
455440
}
456441

442+
/**
443+
* Normalizes a raw rule response body into a consistent {"groups":[...]} JSONObject. Handles
444+
* three response formats:
445+
*
446+
* <ul>
447+
* <li>Prometheus/AMP JSON: {"status":"success","data":{"groups":[...]}} - extracts data
448+
* <li>Cortex/Thanos YAML (all rules): Map of namespace to list of rule groups
449+
* <li>Cortex/Thanos YAML (single namespace): List of rule groups
450+
* </ul>
451+
*
452+
* @param body The raw response body string
453+
* @param namespace Optional namespace name used as the "file" field on groups from YAML
454+
* single-namespace responses. When null, indicates the body may be a YAML map of namespaces.
455+
* @return JSONObject with {"groups":[...]} structure
456+
*/
457+
@SuppressWarnings("unchecked")
458+
private JSONObject normalizeRulesResponse(String body, String namespace) {
459+
if (body.isEmpty()) {
460+
return new JSONObject().put("groups", new JSONArray());
461+
}
462+
463+
// 1. Try Prometheus/AMP JSON format first
464+
try {
465+
JSONObject jsonObject = new JSONObject(body);
466+
if ("success".equals(jsonObject.optString("status")) && jsonObject.has("data")) {
467+
return jsonObject.getJSONObject("data");
468+
}
469+
if (jsonObject.has("groups")) {
470+
return jsonObject;
471+
}
472+
} catch (JSONException e) {
473+
// Not JSON — fall through to YAML parsing
474+
}
475+
476+
// 2. Parse as YAML (Cortex/Thanos format)
477+
try {
478+
Object parsed = YAML_MAPPER.readValue(body, Object.class);
479+
JSONArray groupsArray = new JSONArray();
480+
addGroupsFromParsed(parsed, namespace, groupsArray);
481+
return new JSONObject().put("groups", groupsArray);
482+
} catch (Exception e) {
483+
logger.warn(
484+
"Failed to parse rules response body, returning empty groups: {}", e.getMessage());
485+
return new JSONObject().put("groups", new JSONArray());
486+
}
487+
}
488+
489+
@SuppressWarnings("unchecked")
490+
private void addGroupsFromParsed(Object parsed, String namespace, JSONArray groupsArray) {
491+
if (parsed instanceof Map) {
492+
// All-namespaces format: Map<String, List<RuleGroup>>
493+
Map<String, Object> namespacesMap = (Map<String, Object>) parsed;
494+
for (Map.Entry<String, Object> entry : namespacesMap.entrySet()) {
495+
if (entry.getValue() instanceof List) {
496+
addGroupsFromParsed(entry.getValue(), entry.getKey(), groupsArray);
497+
}
498+
}
499+
} else if (parsed instanceof List) {
500+
// Single-namespace format: List<RuleGroup>
501+
List<Map<String, Object>> groups = (List<Map<String, Object>>) parsed;
502+
for (Map<String, Object> group : groups) {
503+
JSONObject groupObj = new JSONObject(group);
504+
if (namespace != null) {
505+
groupObj.put("file", namespace);
506+
}
507+
groupsArray.put(groupObj);
508+
}
509+
}
510+
}
511+
457512
/**
458513
* Reads and processes an Alertmanager API response.
459514
*

direct-query-core/src/main/java/org/opensearch/sql/prometheus/query/PrometheusQueryHandler.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,15 @@ public GetDirectQueryResourcesResponse<?> getResources(
148148
}
149149
case RULES:
150150
{
151-
String rulesBody;
151+
JSONObject rules;
152152
if (request.getResourceName() != null && !request.getResourceName().isEmpty()) {
153-
rulesBody =
153+
rules =
154154
client.getRulesByNamespace(
155155
request.getResourceName(), request.getQueryParams());
156156
} else {
157-
rulesBody = client.getRules(request.getQueryParams());
157+
rules = client.getRules(request.getQueryParams());
158158
}
159-
return GetDirectQueryResourcesResponse.withStringList(List.of(rulesBody));
159+
return GetDirectQueryResourcesResponse.withMap(rules.toMap());
160160
}
161161
case ALERTMANAGER_ALERTS:
162162
{

0 commit comments

Comments
 (0)