4n4nd / prometheus-api-client-python

A python wrapper for the prometheus http api
MIT License
234 stars 77 forks source link

Add support for query functions to custom_query_range #241

Open NoahZielke opened 2 years ago

NoahZielke commented 2 years ago

I found that I was unable to use query functions on get_metric_range because of how the chunk is implemented.

Is there another way to use these functions or does this support need to be added? I don't mind trying to add it if it needs it.

chauhankaranraj commented 2 years ago

Hi @NoahZielke thanks for bringing this up! I believe you're right, I'm also unable to use query functions in get_metric_range_data like this:

start_time = parse_datetime("3d")
end_time = parse_datetime("now")
chunk_size = parse_timedelta("now", "1d")

query = "rate(scrape_duration_seconds[5m])"

metric_data = pc.get_metric_range_data(
    query,
    start_time=start_time,
    end_time=end_time,
    chunk_size=chunk_size,
)

And yes, it's likely because of the way chunk size is implemented.

Is there another way to use these functions

I was able to work around this problem by using custom_query_range instead like this:

start_time = parse_datetime("3d")
end_time = parse_datetime("now")

query = "rate(scrape_duration_seconds[5m])"

metric_data = pc.custom_query_range(
    query,
    start_time=start_time,
    end_time=end_time,
    step="1d",
)

Does this work for your use case too?

or does this support need to be added? I don't mind trying to add it if it needs it.

Whether get_metric_range_data should be updated to support this functionality, or users should use custom_query_range for such cases would be up for discussion imo /cc @4n4nd

NoahZielke commented 2 years ago

Hi @NoahZielke thanks for bringing this up! I believe you're right, I'm also unable to use query functions in get_metric_range_data like this:

start_time = parse_datetime("3d")
end_time = parse_datetime("now")
chunk_size = parse_timedelta("now", "1d")

query = "rate(scrape_duration_seconds[5m])"

metric_data = pc.get_metric_range_data(
    query,
    start_time=start_time,
    end_time=end_time,
    chunk_size=chunk_size,
)

And yes, it's likely because of the way chunk size is implemented.

Is there another way to use these functions

I was able to work around this problem by using custom_query_range instead like this:

start_time = parse_datetime("3d")
end_time = parse_datetime("now")

query = "rate(scrape_duration_seconds[5m])"

metric_data = pc.custom_query_range(
    query,
    start_time=start_time,
    end_time=end_time,
    step="1d",
)

Does this work for your use case too?

or does this support need to be added? I don't mind trying to add it if it needs it.

Whether get_metric_range_data should be updated to support this functionality, or users should use custom_query_range for such cases would be up for discussion imo /cc @4n4nd

Yes, using custom_query_range was the workaround I used also. It isn't as confusing in hindsight, but it was initially, and it's interesting that I wasn't the only one with the issue. Thanks

mahtin commented 1 year ago

Question: If I use metric_data = prom.get_metric_range_data(...) then I can use metric_object_list = MetricsList(metric_data) call. However, If I use metric_data = prom.custom_query_range(...) then the MetricsList() call fails with KeyError: '__name__'. I can fix this by running this loop before calling MetricsList().

    for r in metric_data:
        r['metric']["__name__"] = metric_name

Where metric_name is part of my query string.

Is this right? Or am I crossing the streams (so to speak).

Thanks in advance

4n4nd commented 1 year ago

Is this right? Or am I crossing the streams (so to speak).

This is the "right" way to do it since Prometheus doesn't return a metric name when you use functions. I wasn't sure if the library should add the query string as the name by default. wdyt @chauhankaranraj?

bdulive commented 1 year ago

Is this right? Or am I crossing the streams (so to speak).

This is the "right" way to do it since Prometheus doesn't return a metric name when you use functions. I wasn't sure if the library should add the query string as the name by default. wdyt @chauhankaranraj?

I encountered same issue with query fuction when using custom_query_range(). Is it possible to add a parameter for custom_query_range() to specify the missing metric name ?

chauhankaranraj commented 1 year ago

Is this right? Or am I crossing the streams (so to speak).

This is the "right" way to do it since Prometheus doesn't return a metric name when you use functions. I wasn't sure if the library should add the query string as the name by default. wdyt @chauhankaranraj?

Hm that's an interesting problem. Do we know if users tend to pass the return value of custom_query_range() to MetricsList()? If yes, and if that is one of the intended use cases of the MetricsList class, then I think we should make the query string as the name by default.