timelion secondary y axis removes configuration of the first one

[ppisljar]

in the latest master when you set a secondary y axis on timelion it would remove all the configuration you set on the first one (so the first one behaves like a default axis)

for example
(.static(4000), .static(5000), .static(6000)).yaxis(min=5000, position=right), .static(3000).yaxis(2, position=left)

will show both axes on the left (position=right on the first axis will get ignored)

[ppisljar]

@rashidkpc the issue seems to be that somewhere (if you can help me point where i would really appreciate it) on the way from server to client undefined's get converted to null's ...

_.merge function here will overwrite all values from the first array with nulls from the second (but it wouldn't do so with undefined) ... the null condition doesnt work on array elements or nested objects.

[manuelbernhardt]

I don't know if this is the exact same bug, but there seems to be a rather odd behaviour when having multiple y-axis (we have 7 in one graph) - the position in which the series are being declared influences the rendering. I.e. series declared in the beginning will see their yaxis options being ignored.

[ppisljar]

@manuelbernhardt seems to be the same issue

[manuelbernhardt]

It's actually pretty easy to reproduce this one:

.static(70).yaxis(2, min=0, max=100, label="70") .static(2).yaxis(3, min=0, max=5, label="2")  .static(20).yaxis(1, min=0, max=100, label="20")

Moving the first static(70) to the last position will make it behave correctly, but then one of the other axes won't see its option honoured.

Note that I observe this behaviour both on the 5.x branch and on a deployed 5.0 version of Kibana

[manuelbernhardt]

I found the cause. For some reason, the _global.yaxes parameter of the series looks like this

s1: [null, null, { ... }]
s2: [{ ... }]
s3: [null, { ... }]

The result that needs to be passed down to flot needs to be an yaxes array in which each object is at the Nth position, where N is the value of yaxis in the series.

Quoting the lodash doc: Subsequent sources overwrite property assignments of previous sources. This is to say that the current merge code overwrites subsequent values (array values get overwritten by subsequent nulls when two arrays get merged).

I'm seeing if I can come up with a solution, but it will most certainly contain a special treatment for yaxis rather than a global merge

[ppisljar]

@manuelbernhardt _merge would work if those values would be undefined instead of null.
that is the case on the server side ... but somewhere along passing it down to client undefineds get converted to nulls ...
i didn't find exactly where this happens, but that would probably be a good place to fix this.

[manuelbernhardt]

This is where the data is pulled from the server I think.

AFAIK undefined is not part of JSON. So even if the response of the server contains "undefined", it will be turned into its JSON equivalent null automatically.

[rashidkpc]

@ppisljar pretty sure this will fix it.

--- a/src/core_plugins/timelion/public/panels/timechart/schema.js
+++ b/src/core_plugins/timelion/public/panels/timechart/schema.js
@@ -222,7 +222,7 @@ module.exports = function timechartFn(Private, config, $rootScope, timefilter, $
               _.merge(options, series._global, function (objVal, srcVal) {
                 // This is kind of gross, it means that you can't replace a global value with a null
                 // best you can do is an empty string. Deal with it.
-                if (objVal == null) return srcVal;
+                if (srcVal == null) return objVal;
               });
             }

[manuelbernhardt]

@rashidkpc @ppisljar I can confirm that this fixes it, thanks! I backported it to a 5.0 deployment, will this make it into one of the next 5.x releases?

[rashidkpc]

@manuelbernhardt yeah, this will go into 5.x