GoshPosh / grafana-meta-queries

Grafana plugin for calculating time period metrics like week over week, month over month and year over year etc.
Apache License 2.0
285 stars 71 forks source link

Fixed Timeshift and Moving Average on 0.0.5 #128

Closed drsoares closed 2 years ago

drsoares commented 3 years ago

Issue fix, #122. Inside TimeShift functions and MovingAverage there were some results not wrapped into promises leading into the following issues:

Screenshot 2021-03-09 at 21 55 35 Screenshot 2021-03-09 at 21 56 08

Datasource: OpenTSDB

Gauravshah commented 3 years ago

@ShilpaSivanesan had we not fixed this as part of previous PR ?

ShilpaSivanesan commented 3 years ago

@ShilpaSivanesan had we not fixed this as part of previous PR ?

Yes I added this at this.query here is the PR https://github.com/GoshPosh/grafana-meta-queries/pull/118

@drsoares added this specifically for timeshift. But our fix should handle this

drsoares commented 3 years ago

Those two parts were missing and not working for OpenTSDB at least. Inside timeshift and movingaverage methods, please double check. I'm assuming that 118 is already released on 0.0.5 version.

drsoares commented 3 years ago

Some other datasources are experiencing the same problem.

https://github.com/GoshPosh/grafana-meta-queries/issues/124#issuecomment-796764442

xpatos commented 3 years ago

I can confirm that this fix works in my case for elastic search. I just applied the fix in Chrome Dev tools. Please merge this PR.

Gauravshah commented 3 years ago

@drsoares can you help me understand why does this fix things ? we are converting all before sending it to grafana https://github.com/GoshPosh/grafana-meta-queries/blob/c9042c96b8503cea337188ad543a7c55086a26ce/src/datasource.js#L89

earth08 commented 3 years ago

Not working on grafana 7.4.5 with influxdb

xpatos commented 3 years ago

@drsoares Can you please help @Gauravshah understand what is the problem so our graphs can start working again.

earth08 commented 3 years ago

I started using autohome-comparequeries after waiting so long

drsoares commented 3 years ago

Apologies for the long absence. @earth08 do you mean this change or the plugin before the change?

@Gauravshah I think the best explanation is trying to reproduce the error with the mentioned datasources. (influx, opentsdb and elasticsearch) I wonder if there's any in which this plugin is working, not sure if there are any behavioural diferences bettween the query datasources from different plugins, the contract is the same and never returns a promise, it results an observable. I've just let that if statement to make it consistent with the remaining plugin. Cheers

earth08 commented 3 years ago

@drsoares I have never got it working, In my case arithmetic is working, but timeshif doesnt

drsoares commented 3 years ago

@earth08 but have you tried with this patch? Without these changes only arithmetic was working for me as well. Cheers

earth08 commented 3 years ago

@drsoares which patch, As I only see 0.0.5 version And time shift is not working

giom-l commented 3 years ago

I confirm this fix perfectly works with grafana 7.5.2 and metaqueries 0.0.5 (tested with timeshift)

xpatos commented 3 years ago

Since @Gauravshah seems reluctant to merge this fix, can we create a fork or use a branch or something. Many graphs in our company is broken due to this bug. It has soon been two months since this bug was reported.

garyhodgson commented 3 years ago

Can also confirm that manually applying the change in this PR and restarting grafana got timeshift to work again (grafana 7.5.5)

earth08 commented 3 years ago

How to apply and what change are u talking about?

garyhodgson commented 3 years ago

How to apply and what change are u talking about?

I just applied the changes detailed here: https://github.com/GoshPosh/grafana-meta-queries/pull/128/files

elluishinojos commented 2 years ago

Any updates on this? This plugin is really useful, but there is no way to manually update a plugin at Grafana Cloud and this is supposed to be the only plugin available that is supposed to allow to do time shifts

Gauravshah commented 2 years ago

@drsoares can you remove some of recent commits so that this PR can be merged. we have another competing PR here https://github.com/GoshPosh/grafana-meta-queries/pull/132 was trying to avoid merge that

ShilpaSivanesan commented 2 years ago

@drsoares could you please pull the latest master and update this PR Also please push dist/datasource.js

ShilpaSivanesan commented 2 years ago

Fixed here https://github.com/GoshPosh/grafana-meta-queries/pull/141

giom-l commented 2 years ago

I still don't understand why this issue is closed. I installed tag 0.0.8 some minutes ago, and I still hit the same issue with timeshift function.

runRequest.catchError TypeError: data is undefined
    promise https://grafana/public/plugins/goshposh-metaqueries-datasource/datasource.js:322

When I apply the patch stated above in datasource.js replace

 return ds.query(options);

with

var ds_res = ds.query(options);
if (ds_res.then){
    return ds_res;
}
else {
    return ds_res.toPromise();
}

on lines 301 & 232 everything is working really fine.

Unless I'm missing something, #141 does not fix that point.

giom-l commented 2 years ago

I added a new PR to integrate changes proposed by @drsoares based on uptodate master branch. I also added some stuffs in docker-compose for testing

145