Preserve tags to join/window

[yosiat]

Hi,

I am creating tick script with measurement with tags (server_group, dc, etc), my
tick script is something like this:

var windows = stream.from('some_measurement')
                                      .where(lambda: 'dc' = 'europe')
                                        .window()
                                            .every(10s)
                                            .period(40s)

var first = windows.first('value')
var last = windows.last('value')


first.join(last)
         .eval(lambda: 'last.last' - 'first.first').as('cvalue')
         .alert()
            // some levels..
            .post('http://some-service')

In the json I am getting on the service I don't have all tags I have in "some_measurement" whom I need.
Is there a way to preserve the tags?

[nathanielc]

@yosiat You need to group by the tags you want preserved.

For example:

var windows = stream.from('some_measurement')                                      
                                      .where(lambda: 'dc' = 'europe')
                                      .groupBy('dc')
                                        .window()
                                            .every(10s)
                                            .period(40s)
...

[nathanielc]

@yosiat But you are using selector functions first,last so that should preserve them. I'll look into this.

[yosiat]

@nathanielc I can check this out as well thanks to the log function, I will see this tomorrow and let you know.

[nathanielc]

@yosiat OK, FYI I was discovered that without changes to the InfluxDB influxql package and the way it handles Aux fields this will not be possible. The tag information is lost when it goes through the first last methods.

[yosiat]

@nathanielc can you explain? are you talking about influxql.first or first ?

[nathanielc]

@yosiat

These lines:

var first = windows.first('value')
var last = windows.last('value')

end up calling functions here https://github.com/influxdata/influxdb/blob/master/influxql/call_iterator.go#L266 and https://github.com/influxdata/influxdb/blob/master/influxql/call_iterator.go#L330

Those function do not preserve the tags on the selected points, only the Aux information. The Aux information is a list of values but the keys are stored elsewhere. For InfluxDB it knows ahead of time which tags it wants to preserve when performing a selector function like first so it can toss the data in the Aux fields. (i.e. SELECT first(value), host, dc FROM ..., InfluxDB knows it needs the host and dc tags). Kapacitor on the other hand does not know since any node later in a task could reference a tag or it could be expected to be there in the JSON output of an AlertNode. As a result Kapacitor would either need to do a complete look ahead and find all unique tag keys in a window and then repopulate the Aux fields(really slow) , or we need to refactor the Influxdb.influxql package in order to preserve tags directly. The first option is just a hack so I prefer the second option.

[yosiat]

@nathanielc Ok, and this dosen't neither on windows.mapReuce(influxql.first('value')) ..

Is there some hack/patch solution that I can do in my tick script to solve this?

[nathanielc]

@yosiat adding a groupBy(*) will cause it to preserve all tags but it will also window by all tags too which may break what you are trying to do.

[yosiat]

@nathanielc I think as well it will be the behaviour of my alert.
I will wait to a fix, Thanks!

[yosiat]

@nathanielc I tried the gropuBy(*) and now it looks that is works.. we will do extensive testing and then we will know finally if it works.

[nathanielc]

I just had an idea of how this could work without requiring any changes to InfluxQL. Basically we just set the tags and fields maps as the first two values of the Aux slice. The Aux fields are completely opaque interface{} values to the InfluxQL code, so we can use them however we need.

[yosiat]

@nathanielc this sound really cool, If this is an easy (It sounds easy) I can implement and submit a PR

[nathanielc]

It should be pretty easy but it deals with some complex features of go. I'll do my best to explain how this could work

First there are different types of InfluxQL functions, aggregators, transformations, and selectors See https://docs.influxdata.com/influxdb/v0.12/query_language/functions/

This change only applies to the Selector functions.

The pieces that need to change:

1. Mark which functions are selectors and not on the ReduceCreater struct https://github.com/influxdata/kapacitor/blob/master/pipeline/influxql.gen.go#L25
2. If a function is a select add the fields and tags maps as the first two entries in the Aux slice of the Point in the Aggregate methods. Similar to https://github.com/influxdata/kapacitor/blob/master/influxql.gen.go#L43
3. In the Emit methods replace the fields and tags on the Point from the Aux fields if it was a selector function. https://github.com/influxdata/kapacitor/blob/master/influxql.gen.go#L123

Also notice that the code linked is generated code because of the typed handling of the data. So you will need to modify the original templates and the regen the code. How to do so is explained in the CONTRIBUTING.md doc.

Finally I started writing a test for this use case in this branch https://github.com/influxdata/kapacitor/tree/nc-issue%23371

The test is a bit old but should get you started.

[yosiat]

@nathanielc I will need to add to ReduceCreator add selectors for first and last like:

FirstCallInfo
LastCallInfo

using the generator and then do the second and last step?

[nathanielc]

@yosiat There isn't any info to convey about the last or first functions so a simple IsSimleSelector bool should work. There are a total of 7 selectors but since Top and Bottom need special info don't count them as selectors which leaves 5 left.

Here for example https://github.com/influxdata/kapacitor/blob/master/pipeline/influxql.go#L180 add IsSimpleSelector: true,

and the rest that follow, in the Selector Functions section but not for Top and Bottom.

[yosiat]

@nathanielc OK, I understand :)

by the way, I think I will call it SelectorInfo instead of IsSmileSelector ;)

[nathanielc]

@yosiat that was a typo ;). I meant IsSimpleSelector

[yosiat]

@nathanielc We only to copy the tags, am I right?
for example the fields in last() will be "last" and we don't to keep the field "value"

If we need to copy only tags, we are copying them to Aux and not to FloatPoint.Tags ?

[nathanielc]

@yosiat We need to copy both fields and tags.

Fields because while we may be finding the last point in time we want to preserve all fields on the point. These are selection functions so the selected point should be unmodified.

If we need to copy only tags, we are copying them to Aux and not to FloatPoint.Tags ?

Correct the Aux fields for various reasons are the preserved values during selection not the .Tags field.

So the Aux fields should contains something like []interface{}{p.Fields, p.Tags}

Then later p.Fields = aux[0] and p.Tags = aux[1]. If you want to create constants for the fields and tags index so its clear what is going on that would be great.

[yosiat]

@nathanielc
When I did on the emit point p.Fields = aux[0], the results was "time" and "value" and it removed the "last", do we want to fields to be merged so they include "time", "value" and "last" ?

And do we want to change the topBottom aux to be []interface{fields, tags} as well?

[yosiat]

by the way, can you merge your test please or do you want me to copy them? and I found bug in your test: you need to add another tag "type" with value of "idle"

[nathanielc]

And do we want to change the topBottom aux to be []interface{fields, tags} as well?

No, topBottom need to use the Aux fields as they are, their implementation uses the Aux fields differently.

by the way, can you merge your test please or do you want me to copy them? and I found bug in your test: you need to add another tag "type" with value of "idle"

Go ahead and add the test yourself. Since it won't pass until your changes are in anyways.

When I did on the emit point p.Fields = aux[0], the results was "time" and "value" and it removed the "last", do we want to fields to be merged so they include "time", "value" and "last" ?

I am not sure here, seems like a select function should just return the point unmodified but currently it renames the field to last and drops all other fields. I guess let's not preserve fields for now and keep the existing fields behavior. We can change it later. So this change will only preserve tags. We can start a new discussion on whether fields should also be preserved. Once the work is done to preserve tags, fields will be easy if we decided its the right thing to do.

[yosiat]

@nathanielc so to sum it up: the Aux for simple selectors will contain only tags?

[nathanielc]

@yosiat Correct

[yosiat]

@nathanielc ok, this sounds easy :) I will do this tomorrow

[yosiat]

Closing due to pull request - #425