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

Two issues when we migrate from prometheus to victoriametrics datasource #205

Open
chenlujjj opened this issue Sep 27, 2024 · 9 comments
Open
Assignees

Comments

@chenlujjj
Copy link

chenlujjj commented Sep 27, 2024

Hi team, we're migrating the datasource from prometheus to victoriametrics, but find two issues:

  • The query will be lost or broken after switching datasource

before:
image

after:
image

  • The default "Type" is instant when creating alert rules using prometheus datasource, but is range when using victoriametrics datasource. We'd like the default value is instant to prevent big query load.
    image

Can you help to take a look into them? Thanks in advance

@hagen1778
Copy link
Contributor

@Loori-R could you please take a look? The first one is very annoying issue

@Loori-R
Copy link
Contributor

Loori-R commented Oct 7, 2024

Unfortunately, we are unable to prevent the query from being lost when switching the datasource in alert rules. This behavior is controlled by Grafana's internal code. When a datasource is switched, Grafana checks the type (which corresponds to the plugin ID that we cannot modify). If the type differs from the previous one, Grafana resets the query by invoking the newModel() function.
You can find the relevant code in Grafana's repository here:

if (settings.type === previousSettings?.type) {
  return copyModel(item, settings);
}
return newModel(item, settings);

The newModel() function initializes a new query model without preserving the existing expression, which is stored in model.expr. Here is the newModel() function for context:

function newModel(item: AlertQuery, settings: DataSourceInstanceSettings): Omit<AlertQuery, 'datasource'> {
  return {
    refId: item.refId,
    relativeTimeRange: item.relativeTimeRange,
    queryType: '',
    datasourceUid: settings.uid,
    model: {
      refId: item.refId,
      hide: false,
      datasource: getDataSourceRef(settings),
    },
  };
}

As you can see, the model.expr field containing the query expression is not retained when creating a new model upon changing the datasource.

@chenlujjj
Copy link
Author

@Loori-R Thank you.

we are unable to prevent the query from being lost when switching the datasource in alert rules.

Beside the alert rules, we find that things seem different for the dashboard panels or metric explorer.
Before:
WeChatWorkScreenshot_25a27c46-91cd-4341-89df-d3c243027a80
After:
image

You can see that the query expression is retained, but not completely right. The metric name is put as the __name__ label and the label name containing dot is not handled correctly.

hagen1778 added a commit that referenced this issue Oct 8, 2024
* feature: set default Type to instant for alert rules #205

* Update CHANGELOG.md

---------

Co-authored-by: Roman Khavronenko <[email protected]>
@Loori-R
Copy link
Contributor

Loori-R commented Oct 8, 2024

How it works:

The datasource uses the importFromAbstractQueries and exportToAbstractQueries methods to convert abstract queries into valid expressions and vice versa. When exporting from Prometheus using exportToAbstractQueries, the label names become distorted.

What we can do:

We can adjust our implementation to correctly handle labels with dots during import. This will preserve labels with dots when switching from VictoriaMetrics to Prometheus.

Why this isn't a complete solution:

This only resolves the issue in one direction. To fully address the problem, the exportToAbstractQueries method in the Prometheus datasource must correctly handle labels with dots in their names.

@chenlujjj
Copy link
Author

@Loori-R Understood, thank you very much!

@hagen1778
Copy link
Contributor

This only resolves the issue in one direction. To fully address the problem, the exportToAbstractQueries method in the Prometheus datasource must correctly handle labels with dots in their names.

@Loori-R is this a complex task? Do you think you can submit a PR to grafana with fix?

@dmitryk-dk
Copy link
Contributor

Hi @chenlujjj ! This feature was implemented in the new release. Please check it, and if you find any problem, please reopen the issue.

@Loori-R
Copy link
Contributor

Loori-R commented Oct 15, 2024

The following functionality has been implemented and included in the release: "set the default query type to instant when creating alerting rules"

However, I am reopening this issue because there is still a problem with dots in the labels when switching between VictoriaMetrics and Prometheus.

@Loori-R Loori-R reopened this Oct 15, 2024
@hagen1778
Copy link
Contributor

Related change grafana/grafana#95163

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

4 participants