mirror of
https://github.com/meilisearch/MeiliSearch
synced 2024-11-26 14:54:27 +01:00
Merge #4619
4619: Use http path pattern instead of full path in metrics r=irevoire a=gh2k # Pull Request ## Related issue Fixes #3983 ## What does this PR do? - This records only the HTTP pattern in metrics instead of the full path An alternative solution was proposed in #4145, but this doesn't really fix the root cause of the issue. The problem I'm experiencing at my end is that by using the full path, the number of labels is far too high to be useful. It is normal practice to use the path with variable placeholders, instead of the fully-expanded path. The example given in the ticket was endpoints under `/tasks`, but this can also be a very significant problem under `/indexes/{index-uid}/documents`. e.g.: <img width="1510" alt="Screenshot 2024-05-03 at 12 14 36" src="https://github.com/meilisearch/meilisearch/assets/6530014/1df2ec19-5f69-4164-90d2-f65c59f9b544"> This patch replaces the fully-expanded path with the matched pattern. The linked PR also mentions paths under other routes, e.g. `/static`, but this feels like a separate concern and these can be stripped out at the Prometheus end by filters if they are unwanted. The most important thing is to make the paths usable so that we can still get stats on e.g. the number of document deletes we see. ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: Simon Detheridge <s@sd.ai> Co-authored-by: Tamo <tamo@meilisearch.com>
This commit is contained in:
commit
ecb5c506b3
@ -59,10 +59,12 @@ where
|
|||||||
let request_path = req.path();
|
let request_path = req.path();
|
||||||
let is_registered_resource = req.resource_map().has_resource(request_path);
|
let is_registered_resource = req.resource_map().has_resource(request_path);
|
||||||
if is_registered_resource {
|
if is_registered_resource {
|
||||||
|
let request_pattern = req.match_pattern();
|
||||||
|
let metric_path = request_pattern.as_ref().map_or(request_path, String::as_str);
|
||||||
let request_method = req.method().to_string();
|
let request_method = req.method().to_string();
|
||||||
histogram_timer = Some(
|
histogram_timer = Some(
|
||||||
crate::metrics::MEILISEARCH_HTTP_RESPONSE_TIME_SECONDS
|
crate::metrics::MEILISEARCH_HTTP_RESPONSE_TIME_SECONDS
|
||||||
.with_label_values(&[&request_method, request_path])
|
.with_label_values(&[&request_method, metric_path])
|
||||||
.start_timer(),
|
.start_timer(),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
@ -376,12 +376,6 @@ async fn get_version(
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Serialize)]
|
|
||||||
struct KeysResponse {
|
|
||||||
private: Option<String>,
|
|
||||||
public: Option<String>,
|
|
||||||
}
|
|
||||||
|
|
||||||
pub async fn get_health(
|
pub async fn get_health(
|
||||||
req: HttpRequest,
|
req: HttpRequest,
|
||||||
index_scheduler: Data<IndexScheduler>,
|
index_scheduler: Data<IndexScheduler>,
|
||||||
|
Loading…
Reference in New Issue
Block a user