Skip to content
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.

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

Add support for query functions to custom_query_range #241

Open
NoahZielke opened this issue May 6, 2022 · 6 comments
Open

Add support for query functions to custom_query_range #241

NoahZielke opened this issue May 6, 2022 · 6 comments

Comments

@NoahZielke
Copy link

NoahZielke commented May 6, 2022

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Contributor

mahtin commented Oct 30, 2022

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
Copy link
Owner

4n4nd commented Oct 31, 2022

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
Copy link

bdulive commented Dec 11, 2022

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants