> Beware including the request path in your metric. If the actual value
supplied by the requestor is used, e.g.
Thanks, saved me from a future headache there. My intentions were to copy
the labels from the default metrics supplied from app.UseHttpMetrics(). I
see you can build a list using ActionDescriptorCollectionProvider, however,
my route values get changed at each action method with different appendages
which is probably not a good pattern. I don't want to write a complicated
method there to account for it. Instead, I'm going to assume it's safe to
check the value of the action and controller name since I'm using
app.UseRouting() before the middleware in question here gets called.
string? controllerName =
httpContext.Request.RouteValues["controller"]?.ToString( );
string? actionName =
httpContext.Request.RouteValues["action"]?.ToString( );
string? endpoint = httpContext.Request.Path.Value;
if (controllerName is null || actionName is null)
{
endpoint = null;
}
^I changed 'path' to 'endpoint' to make it consistent with the label name.
On Wednesday, August 17, 2022 at 3:24:17 AM UTC-4 Brian Candler wrote:
> Glad it's working.
>
> Prometheus treats empty string as label not being present at all, but it's
> not recommended to have inconsistent sets of labels on your metrics, as it
> makes them harder to aggregate over. A concrete value like "-" is better.
>
> Beware including the request path in your metric. If the actual value
> supplied by the requestor is used, e.g.
>
> http_requests{code="404",action="-",controller="-",path="/askdjnkj"} 1
>
> then you put yourself at risk of a cardinality explosion. It's a very easy
> DoS attack vector - and even if unintentional, don't be surprised if people
> port-scanning your server try all sorts of paths. Sanitise the path label
> to permitted/expected values only, or leave it out (since the path has
> already been mapped to controller and action, right?)
>
> If you do want to log the actual path presented, then really you need to
> use a logging system like Loki.
>
> On Tuesday, 16 August 2022 at 19:49:36 UTC+1 [email protected] wrote:
>
>> Looks like I answered my question I believe hasn't been authorized yet,
>> or maybe I forgot to click 'post message'. But the solution is to pass the
>> label values as empty strings or with a hyphen placeholder like Brian
>> Candler suggests. Prometheus will pick these up and not show the label
>> name/value pair where an empty string or hyphen is for the value. Passing
>> in null values or creating separate metrics of the same name throw
>> exceptions.
>>
>> Here is a snippet of my code for the nullable label names:
>>
>> string[]? labels = new string[] { statusCode, method,
>> actionName ?? "", controllerName ?? "", path ?? "" };
>>
>> I then pass the labels to a function that increments the counter by the
>> *length* parameter:
>>
>> public void RegisterResponse(double length, params string[]
>> labelValues)
>> {
>> _responseCounter.WithLabels(labelValues).Inc( );
>> _responseLength.WithLabels(labelValues).Inc(length);
>> }
>>
>> On Tuesday, August 16, 2022 at 3:05:52 AM UTC-4 Brian Candler wrote:
>>
>>> > How do you deal with dynamic label names without having to create
>>> separate metrics counters?
>>>
>>> You have to have separate counters.
>>>
>>> Every combination of (metric name + set of label names/values) creates a
>>> distinct timeries, i.e. a different metric, by definition. Therefore you
>>> need different counters for each distinct combination of label values.
>>>
>>> Labels are not just somewhere to store a string. If you put
>>> high-cardinality values in a label, i.e. values which change from request
>>> to request and are not bounded to a small subset of values, then you will
>>> risk a "cardinality explosion" where you have millions of timeseries all
>>> with a single value. Under these conditions, Prometheus will crash and
>>> burn.
>>>
>>> In your case, if you're using controller name and action name for
>>> labels, then I think it should be OK as those are values chosen from a
>>> small set defined in code. For counting requests which don't have these
>>> fields, then use a dummy value like "-" for those labels.
>>>
>>> On Monday, 15 August 2022 at 22:34:16 UTC+1 [email protected] wrote:
>>>
>>>> In the default metrics exported with app.usehttpmetrics(), certain
>>>> metrics, such as http_requests_received_total will have scraped data that
>>>> only contain ''code' and 'method' labels which I see is defined here
>>>> in HttpRequestLabelNames.cs
>>>> <https://github.com/prometheus-net/prometheus-net/blob/38d45fa0a102e9afc76f031675aa0719d4f063d1/Prometheus.AspNetCore/HttpMetrics/HttpRequestLabelNames.cs>.
>>>>
>>>> I'm trying to replicate the same thing with custom middleware that records
>>>> the response body content length. But when requests with bad query params
>>>> return 404 responses there is no controller, endpoint, or action fields in
>>>> HttpContext and I can't use my custom counters with those label names
>>>> specified. How do you deal with dynamic label names without having to
>>>> create separate metrics counters?
>>>
>>>
--
You received this message because you are subscribed to the Google Groups
"Prometheus Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/prometheus-users/4aba3786-739b-4cb4-b020-010af8e32487n%40googlegroups.com.
