Skip to content

fix(plugin-prometheus): Use appendPath to build URI in Prometheus connector #26733

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.

Sign up for GitHub

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

Open
keytouch wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(plugin-prometheus): Use appendPath to build URI in Prometheus connector #26733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 3 additions & 8 deletions presto-prometheus/src/main/java/com/facebook/presto/plugin/prometheus/PrometheusClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package com.facebook.presto.plugin.prometheus;

import com.facebook.airlift.http.client.HttpUriBuilder;
import com.facebook.airlift.json.JsonCodec;
import com.facebook.airlift.log.Logger;
import com.facebook.presto.common.type.DoubleType;
Expand All @@ -38,7 +39,6 @@
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.security.KeyManagementException;
Expand Down Expand Up @@ -92,13 +92,8 @@ public PrometheusClient(PrometheusConnectorConfig config, JsonCodec<Map<String,

private static URI getPrometheusMetricsURI(URI prometheusUri)
{
try {
// endpoint to retrieve metric names from Prometheus
return new URI(prometheusUri.getScheme(), prometheusUri.getAuthority(), prometheusUri.getPath() + METRICS_ENDPOINT, null, null);
}
catch (URISyntaxException e) {
throw new RuntimeException(e);
}
// endpoint to retrieve metric names from Prometheus
return HttpUriBuilder.uriBuilderFrom(prometheusUri).appendPath(METRICS_ENDPOINT).build();
Copy link
Contributor

@sourcery-ai sourcery-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

issue (bug_risk): Preserving query/fragment from the base URI may change the effective metrics endpoint.

The old code built a URI from only scheme, authority, and path + METRICS_ENDPOINT, dropping any query/fragment from prometheusUri. The new use of uriBuilderFrom(prometheusUri) keeps query/fragment/user-info and then appends the metrics path, so any configured query params or fragment will now be sent to the metrics endpoint. If the intent is a clean metrics URL, consider clearing query/fragment when building it (e.g., via replacePath plus explicitly clearing query).

}

public Set<String> getTableNames(String schema)
Expand Down
4 changes: 2 additions & 2 deletions ...rometheus/src/main/java/com/facebook/presto/plugin/prometheus/PrometheusSplitManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,13 @@ static String decimalSecondString(long millis)
return new BigDecimal(Long.toString(millis)).divide(new BigDecimal(1000L)).toPlainString();
}

// URIBuilder handles URI encode
// HttpUriBuilder handles URI encode
private static URI buildQuery(URI baseURI, String time, String metricName, Duration queryChunkSizeDuration)
throws URISyntaxException
{
return HttpUriBuilder
.uriBuilderFrom(baseURI)
.replacePath("api/v1/query")
.appendPath("api/v1/query")
.addParameter("query", metricName + "[" + queryChunkSizeDuration.roundTo(queryChunkSizeDuration.getUnit()) +
Duration.timeUnitToString(queryChunkSizeDuration.getUnit()) + "]")
.addParameter("time", time)
Expand Down
Loading