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

Added incrimented field for each request #2

Closed
wants to merge 1 commit into from
Closed

Added incrimented field for each request #2

wants to merge 1 commit into from

Conversation

nucleusv
Copy link
Contributor

@nucleusv nucleusv commented May 8, 2015

Added incrementing requestId variable for each request, not always write good due to async requests.

Removed condole.log()

Also removed debug console logging
@alexanderzobnin
Copy link
Collaborator

I have already removed console.log(). Please, fetch your repo for new changes.

@alexanderzobnin
Copy link
Collaborator

Note about "id" parameter from Zabbix API docs

“id”: 1 - this field can be used to tie every JSON request to it's response. The response will have the same “id” as provided by the request. It is useful when you are sending multiple requests at once. These are not required to be unique or sequential;

For this reason I don't use increment or other id calculation.

@nucleusv
Copy link
Contributor Author

nucleusv commented May 8, 2015

Alex, look at errors,
try to create a new graph and add metrics.

I add new metrics, but I have only one line
https://yadi.sk/i/r_VaNA4agWkL5
https://yadi.sk/i/oGGMCsDBgWkQD
https://yadi.sk/i/6_DTlPligWkRJ

@alexanderzobnin
Copy link
Collaborator

I think this is because different data types in one graph. I note this moment in code:

 ZabbixAPIDatasource.prototype.performTimeSeriesQuery = function(items, start, end) {
      var item_ids = items.map(function (item, index, array) {
        return item.itemid;
      });

      // TODO: if different value types passed?
      //       Perform multiple api request.
      var history_type = items[0].value_type;

I'll try to fix this problem in this release.

@alexanderzobnin
Copy link
Collaborator

Need to group requests with same data type and do multiple api request. Just with incremental id ))

@nucleusv
Copy link
Contributor Author

nucleusv commented May 8, 2015

I think to use incremental Id is not best way, that is why id field must be unique, I think it is better to use Math.random() * 10000 function ?

@alexanderzobnin
Copy link
Collaborator

“id”: 1 - this field can be used to tie every JSON request to it's response.

id must be unique within one http request. We may use for example id: 1 for integer data and id: 2 for float.

@nucleusv
Copy link
Contributor Author

nucleusv commented May 8, 2015

To be honest, I think it is not needed in angularjs apps, you need to rewrite function history get that do async request to api, and when all subrequest functions return result merge them in one array and return.

And with async you don't need to handle id field.

@nucleusv
Copy link
Contributor Author

nucleusv commented May 8, 2015

In function
ZabbixAPIDatasource.prototype.performTimeSeriesQuery = function(items, start, end)

you need to run over all array items, and create new hash history_type[type] = array_of_items

then you need to run over history_type hash and

for in history_type as key=> value {

do data json for each request
performQuery = function(){ this.doZabbixAPIRequest(data1) } for the first hash element

and then

performQuery.then(function(){this.doZabbixAPIRequest(dataN) })

}

return perfromQuery()

////
It is just a logic not a code :)

@alexanderzobnin
Copy link
Collaborator

Ok, I thought the same. I'll do it tomorrow.

@nucleusv
Copy link
Contributor Author

nucleusv commented May 8, 2015

Great!

And for future I hope this patch will be in zabbix release soon: https://support.zabbix.com/browse/ZBXNEXT-1193

If we ask data more than 24 hours we should take data from trend(s).get method instead from history.get
What do you think?

@alexanderzobnin
Copy link
Collaborator

Yes, I also notice this problem. It complicated work with long-time graphs. This patch will solve problem.

@alexanderzobnin
Copy link
Collaborator

I fixed bug with different data types in one graph. See release-1.0 branch. Please test on your zabbix installation.

@nucleusv
Copy link
Contributor Author

Alex, your code like a charm 👍 I will test it tommorrow.

Another thought have come to my mind:

According to data types in zabbix
<select class="input select" id="value_type" name="value_type" size="1">
<option value="3" selected="selected">Числовой (целое положительное)</option>
<option value="0">Числовой (с плавающей точкой)</option>
<option value="1">Символ</option><option value="2">Журнал (лог)</option>
<option value="4">Текст</option>
</select>

This method must return only items which has a value type of float or integer:

ZabbixAPIDatasource.prototype.performItemSuggestQuery 

params: {
      output: ['name', 'key_', 'value_type', 'delay'],
      sortfield: 'name',
      hostids: hostid,
      filter: {
                 value_type: [0,3]
      }
    }

from zabbix docs:

filter - object
Return only those results that exactly match the given filter.

Accepts an array, where the keys are property names, and the values are either a single value or an array of values to match against.

Supports additional filters:
host - technical name of the host that the item belongs to.

https://www.zabbix.com/documentation/2.4/manual/api/reference/item/get

`

@alexanderzobnin
Copy link
Collaborator

Thanks. It is a good idea for graphs - non-numeric items just not needed.

@nucleusv
Copy link
Contributor Author

Alex, do you have your own zabbix installation?
Can you add trends api patch to it?
If not I will create VM on ditigatal patch for testing ?

@nucleusv
Copy link
Contributor Author

Any success with version for grafana-2 ?

@alexanderzobnin
Copy link
Collaborator

I have production zabbix server but in isolated network without Internet access.
In Grafana-2 I have some troubles with configuration page and data collecting via grafana backend. Plugin work only in grafna-1.9 like style (request data via frontend). In other respects this version very similar inside.

@nucleusv nucleusv closed this May 10, 2015
@nucleusv nucleusv deleted the release-1.0 branch May 10, 2015 21:54
@nucleusv
Copy link
Contributor Author

Alex, please add filter to return only host groups with real servers, host groups with templates only are not needed I thinl

// Get the list of host groups
ZabbixAPIDatasource.prototype.performHostGroupSuggestQuery = function() {
  var data = {
    jsonrpc: '2.0',
    method: 'hostgroup.get',
    params: {
      output: ['name'],
      real_hosts: true,   //Return only host groups that contain hosts
      sortfield: 'name'
    },
    auth: this.auth,
    id: ++this.requestId
  };

  return this.doZabbixAPIRequest(data);
};

@nucleusv
Copy link
Contributor Author

Added pull request with additonal filters.

Found a lot of bugs, with visualization something strange happens, and there are still errors, will do screenshots.

What about your zabbix, can you applly patch to your zabbix installation.
I will do you it to my. And we can test trends ?

@alexanderzobnin
Copy link
Collaborator

Please, open issue with screenshots.
Trends is good idea, I'll try to test it later.
Probably, I need to create development roadmap with next goals.

leleobhz referenced this pull request in ZenithTecnologia/grafana-zabbix Jul 7, 2022
leleobhz referenced this pull request in ZenithTecnologia/grafana-zabbix Jul 7, 2022
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

Successfully merging this pull request may close these issues.

2 participants