Array indexes mix-up in curl_json plugin #2266
Assignees
Labels
Comments
|
Thank you @madchrist for reporting this issue and providing a test case. I was able to reproduce the problem with that. I think I know what the issue is: the array index is not initialized correctly, resulting in an off by one error. Unfortunately the code is a mess and desperately needs some refactoring. The main challenge is going to be not to break some other corner case. The best way forward may be to implement some unit tests for the parser first and fix this in a second step. |
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
Previously, the "key" was loaded by calling cj_cb_map_key() from
cj_cb_inc_array_index(). That means that the key for the previous element
was loaded as the array index was updated for the next element, resulting
in an off-by-one error. Also the key was not unset in time, resulting in
two metrics with the same identifier being created.
This patch fixes this with the following changes:
* cj_advance_array() (nee cj_cb_inc_array_index()) now loads the key for
the new index position instead of the previous one.
* The initial "0" key is loaded from cj_cb_start_array().
* cj_advance_array() always updates the key. The "update_key" argument
has been removed.
* Refactoring: key loading has been moved out of cj_cb_map_key() and
into its own function, cj_load_key().
* The unit tests have been expanded to cover this case.
Fixes: collectd#2266
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
This adds various tests involving arrays. Issue: collectd#2266
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
Previously, the "key" was loaded by calling cj_cb_map_key() from
cj_cb_inc_array_index(). That means that the key for the previous element
was loaded as the array index was updated for the next element, resulting
in an off-by-one error. Also the key was not unset in time, resulting in
two metrics with the same identifier being created.
This patch fixes this with the following changes:
* cj_advance_array() (nee cj_cb_inc_array_index()) now loads the key for
the new index position instead of the previous one.
* The initial "0" key is loaded from cj_cb_start_array().
* cj_advance_array() always updates the key. The "update_key" argument
has been removed.
* Refactoring: key loading has been moved out of cj_cb_map_key() and
into its own function, cj_load_key().
Unit tests are in a separate commit for easier cherry-picking.
Fixes: collectd#2266
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
Previously, the "key" was loaded by calling cj_cb_map_key() from
cj_cb_inc_array_index(). That means that the key for the previous element
was loaded as the array index was updated for the next element, resulting
in an off-by-one error. Also the key was not unset in time, resulting in
two metrics with the same identifier being created.
This patch fixes this with the following changes:
* cj_advance_array() (nee cj_cb_inc_array_index()) now loads the key for
the new index position instead of the previous one.
* The initial "0" key is loaded from cj_cb_start_array().
* cj_advance_array() always updates the key. The "update_key" argument
has been removed.
* Refactoring: key loading has been moved out of cj_cb_map_key() and
into its own function, cj_load_key().
Unit tests are in a separate commit for easier cherry-picking.
Fixes: collectd#2266
octo
added a commit
to octo/collectd
that referenced
this issue
May 16, 2017
This adds various tests involving arrays. Issue: collectd#2266
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug description
I'm trying to fetch two values from a json array with curl_json
I want "datapoints/0/1" and "datapoints/0/2"
"datapoints/0/1" works normaly but "datapoints/0/2" will have the value of "datapoints/0/3"
Also, the RRD for "datapoints/0/1" seems to be used twice in one interval, i'm guessing "datapoint/0/2" is somehow added to this RRD but is rejected because too recent
remote json content
{"datapoints":[[10,11,12,13,14,15,16]]}collectd.conf extract
syslog output
Wrong values in the RRDs
gauge-eleven.rrd should contain 11
gauge-twelve.rrd should contain 12
The text was updated successfully, but these errors were encountered: