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

Return an actual prometheus version in /buildinfo to accelerate dashboard performence #5370

Closed
noamApps opened this issue Nov 22, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@noamApps
Copy link

Describe the bug

When registering VictoriaMetrics as a Prometheus datasource in Grafana, Unless stated explicitly in the Performence dropdown, Grafana tries to auto discover the Prometheus version by firing a request to buildinfo.

Currently, VictoriaMetrics returns a success message in the response with no version, I think this makes Grafana assumes the worst and treat the registered Prometheus datasource as an old version Prometheus.

Specifically, this causes Grafana to use the much slower and inefficient /series api instead of label/values api for label_values queries which are used in dashboard variables, this can cause significant slowdowns in dashboard performance, especially in load times.

In one environment, by explicitly setting the Prometheus datasource as a latest version, I was able to drop the dashboard load time from 30seconds to 160ms, as response size shrunk from 43mb unzipped to 6.7kb

To Reproduce

  1. Register a VictoriaMetrics as Prometheus datasource in Grafana without stating version in the Performence section
  2. Create a variable that uses label_values that returns a lot of values
  3. Dashboard loading should take a lot of times, specifically firing requests to /series api that return huge payload after a waiting quite a bit
  4. Change the datasource settings so its explicitly marked as a new Promtheus
  5. Dashboard should load much faster, /series requests are now using labels api

Version

victoria-metrics-20231002-214928-tags-v1.94.0-0-gf13a96f42

Logs

No response

Screenshots

No response

Used command-line flags

No response

Additional information

No response

@noamApps noamApps added the bug Something isn't working label Nov 22, 2023
@Amper Amper self-assigned this Nov 22, 2023
@Amper
Copy link
Contributor

Amper commented Nov 22, 2023

Hey, @noamApps.
From my testing, the version from buildInfo is only used when the datasource is created and only after manually specifying the Prometheus type (doc):

  • New datasource:
    image
  • Setting Prometheus type without version in buildinfo:
    image
  • Setting Prometheus type with version in buildinfo:
    image

But in both cases, you can set the version manually.

I will discuss with the team, but i suppose that most likely only the following should be done as part of this issue:

  • Describe in the documentation the recommendation to set the prometheusType and prometheusVersion parameters
  • Set these parameters in our helm charts

@noamApps
Copy link
Author

Hi @Amper , thanks for replying quickly, indeed I fixed it for my case by manually setting the Prometheus version when registering the datasource.

Not sure what happens in a case you don't mention Prometheus type in the performance tab at all, my assumption is it will still assume the worst.

Definitely worth mentioning in docs and add to chart, but I wonder if in most scenarios people will just add prometheus datasource without modifying the Perf tab, and in provisioning scenarios, will not define it in the API request to Grafana.

The benefit of returning a meaningful buildinfo that corresponds to the prometheus api supported by VictoriaMetrics will be that it will leave no room for unoptimized datasource registration

@Amper
Copy link
Contributor

Amper commented Nov 23, 2023

The benefit of returning a meaningful buildinfo that corresponds to the prometheus api supported by VictoriaMetrics will be that it will leave no room for unoptimized datasource registration

From my testing it turns out that it doesn't help. I tried adding a version to buildInfo and tested grafana:

  • buildInfo is only used during the setup phase when creating a data source
  • If you don't set Prometheus type in the settings (leave everything by default), buildInfo is not used at all.

@noamApps
Copy link
Author

Got it, Then I think I will open another ticket to Grafana to query buildinfo when registering Prometheus datasource regardless of the Perf tab setup.

I really appreciate the quick response 🚀

@Amper Amper added enhancement New feature or request and removed bug Something isn't working labels Nov 23, 2023
@valyala
Copy link
Collaborator

valyala commented Nov 23, 2023

@Amper , @noamApps , I think an important point is missing in this feature request - the version of Grafana in question. It would be great if VictoriaMetrics could mimic Prometheus for Grafana without the need in manual datasource config changes, if this allows improving performance.

According to the latest source code, Grafana uses some technique to determine whether it can use /api/v1/labels with match[]:
https://github.com/grafana/grafana/blob/11d4f604f5b11c2f323ac9a34acadc47cc66be5a/public/app/plugins/datasource/prometheus/datasource.ts#L176-L188

@noamApps
Copy link
Author

Hi @valyala,
Definitely, my tests were done on Grafana v10.1.2
I agree that out-of-the-box optimized behavior will probably eliminate degraded performance for unaware users

Amper added a commit that referenced this issue Dec 7, 2023
…API for using more efficient API in Grafana for receiving label values, added additional info about setup Grafana datasource (#5370)
valyala pushed a commit that referenced this issue Dec 7, 2023
…API for using more efficient API in Grafana for receiving label values, added additional info about setup Grafana datasource (#5370) (#5437)
valyala added a commit that referenced this issue Dec 7, 2023
This reduces added image sizes by 3x

Updates #5437
Updates #5370
valyala pushed a commit that referenced this issue Dec 7, 2023
…API for using more efficient API in Grafana for receiving label values, added additional info about setup Grafana datasource (#5370) (#5437)
valyala added a commit that referenced this issue Dec 7, 2023
This reduces added image sizes by 3x

Updates #5437
Updates #5370
@valyala
Copy link
Collaborator

valyala commented Dec 7, 2023

@noamApps , could you build single-node VictoriaMetrics from the commit 02a0a7f according to these docs and verify whether this helps Grafana switching to an optimized API for labels` retrieval?

@hagen1778 , @Loori-R , could you verify whether VictoriaMetrics datasource for Grafana uses the optimized API for labels` retrieval by default? If this isn't the case, then it should be fixed there.

@valyala
Copy link
Collaborator

valyala commented Dec 13, 2023

VictoriaMetrics should return version 2.24.0 in the response to /api/v1/status/buildinfo starting from the release v1.96.0. This should be enough to trigger optimized API calls for labels' auto-suggestion at Grafana. Closing the issue as fixed then.

@belm0
Copy link
Contributor

belm0 commented Jan 9, 2024

I understand this approach of returning a Prometheus version, and it avoids having to upstream anything to Grafana, but it's not ideal.

For example, let's say Prometheus one day adds a feature that VictoriaMetrics already has (I think it was the case for label/.../values with match), and Grafana starts using it based on the version. But a VM instance would have to be redeployed after a source code change to the published version, even though the instance already has the capability.

(Or less likely scenario: Grafana wants to use a feature that only Victoria Metrics API has...)

Grafana is already supporting capability checks of various Prometheus-compatible backends like Mimir, Cortext, Thanos. It wouldn't be hard to upstream a PR that adds Victoria Metrics and supports the native version, right?

@hagen1778
Copy link
Collaborator

It wouldn't be hard to upstream a PR that adds Victoria Metrics and supports the native version, right?

At least we can try! @Loori-R would you mind filing such PR?

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

No branches or pull requests

5 participants