Improve InfluxDB

[titilambert]

Description:
This PR revert #4791 to store data in InfluxDB using best practices (low cardinality).
See comment here: #4791 (comment)

It also fixes #4696.

Related issue: fixes #4696 , Revert #4791

[mention-bot]

@titilambert, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bah2830, @bestlibre and @fabaff to be potential reviewers.

[balloob]

I don't have too much insight on this. @bah2830, @MartinHjelmare please respond :-)

[brentahughes]

I agree with @titilambert on the mix of data that can live in these fields is a problem. But by reverting we go back to the issue that nearly every state change goes into it's own series. Which means trying to sum up all motion sensors or averaging all temp sensors is impossible as they don't all use the same or even set the unit_of_measurement.

This also has the issue that "unit_of_measurement" is wildly inconsistent. Climate platforms all set the unit_of_measurement in degrees while the state is actually a string (cool, heat, etc...). Also if the unit_of_measurement is in the attributes every attribute now goes into that same unit.

The problem is each state object can have lots of different types of data in it, including a mix within the same state. So assuming "unit_of_measurement" covers everything means you now have to go searching through your series to figure out that your humidity level in the house is under the ℉ series because it was an attribute of a climate entity that used a string as the state but ℉ as the unit_of_measurement.

[brentahughes]

Not sure if this was the original or not. But this means you can only override the default if a unit_of_measurement is not set.

[titilambert]

Yes this was the original. And you're right we can only override if there is no unit_of_measurement...

[titilambert]

Which means trying to sum up all motion sensors or averaging all temp sensors is impossible as they don't all use the same or even set the unit_of_measurement.

@bah2830 could you give me an example of influxQL query you want to do ?

[brentahughes]

@titilambert Most common one for me would be counting every time a state changes and then grouping by domain. Gets an overall idea of the activity of things.

Or my humidity sensors that use % as the unit_of_measurement. But the humidity for the nest is an attribute so it ends up in the degrees series. I can no longer group them by time(1m) and average it.

You can't group by across series.

[titilambert]

@bah2830 I understand your point. In fact you want join series/measurements.
see here: influxdata/influxdb#3552 (comment)
But this is not ready :(
And I don't think group all metrics in the same measurement is the good way for this using InfluxDB...
Maybe you should look at Kapacitor. I use it, I know this is really complex, but Kapacitor can merge and do some processing across measurements and push results to influxDB ...

[brentahughes]

So joining series would be nice. My problem is assuming the unit_of_measurement is actually a representation of the data when it's not currently.

Ideally Home Assistant would be more strictly typed but that's not the case and we can't make it just for this.

If we update the default_measurement to allow overriding the unit_of_measurement then I think this change could do everything people may need. Including allowing me to group up series that would normally be split.

[titilambert]

@bah2830 based on your last comment I updated the PR. I just add a new param: override_measurement

[brentahughes]

Thanks. I think adding that and the update to send '_str' for non numerics should work out just fine for most users.

[titilambert]

@bah2830 could you confirm that is working for you ?

[brentahughes]

@titilambert I will merge your branch into mine tonight and let it run for a while to check for any errors and verify it will work as expected.

[balloob]

@bah2830 run for a while -> we should merge this before Friday when it is the release day.

[brentahughes]

I started getting the data type error immediately after startup.

17-01-10 07:36:00 ERROR (Thread-4) [homeassistant.components.influxdb] Error saving event "[{'fields': {'node_id': 9.0, 'battery_level': 100.0, 'friendly_name_str': 'Ecolink PIR Motion Sensor SourceNodeId', 'value': 0.0}, 'measurement': 'test_series', 'tags': {'source': 'hass', 'entity_id': 'ecolink_pir_motion_sensor_sourcenodeid_9_2', 'instance': 'production', 'domain': 'sensor'}, 'time': datetime.datetime(2017, 1, 10, 13, 36, 0, 734430, tzinfo=<UTC>)}]" to InfluxDB
Traceback (most recent call last):
  File "/usr/src/app/homeassistant/components/influxdb.py", line 151, in influx_event_listener
    influx.write_points(json_body)
  File "/usr/local/lib/python3.5/site-packages/influxdb/client.py", line 402, in write_points
    tags=tags)
  File "/usr/local/lib/python3.5/site-packages/influxdb/client.py", line 447, in _write_points
    expected_response_code=204
  File "/usr/local/lib/python3.5/site-packages/influxdb/client.py", line 289, in write
    headers=headers
  File "/usr/local/lib/python3.5/site-packages/influxdb/client.py", line 259, in request
    raise InfluxDBClientError(response.content, response.status_code)
influxdb.exceptions.InfluxDBClientError: 400: {"error":"field type conflict: input field \"value\" on measurement \"test_series\" is type float, already exists as type integer"}

Looks like the states that made it into influxdb are all setting the value to 0 causing the column to be an int.

[brentahughes]

This needs to be cast to a float to prevent inconsistent values that cause data type errors

[titilambert]

The cast is done line 107:

            _state = float(state_helper.state_as_number(state))

[MartinHjelmare]

It would be interesting to know what the previous events for the PIR were where the values were converted to int. I'm going to go through the reported issues for Influxdb to see how wide range of type conversion problems we have seen.

[brentahughes]

This was using the the override config so all were in the same measurement. But I've seen it a lot with just the nest that the nest API will return 71 and then later return 71.2

Many people in the gitter channels complain about this data type error. Which is where the original PR came from to convert them to float and fix the most common error I've seen with the influxdb component.

[MartinHjelmare]

Numbers will already have been converted to float via the state helper state_as_number, so your Nest example shouldn't give us a problem.

[brentahughes]

I think the problem is that state_as_number will return a float only if the state is not one of the const listed. If not you get a 1 or 0.

So possibly because the nest and all other climates set the state as a string based on the current mode of the thermostat instead of the actual temp.

But I can guarantee without anything custom on influxdb config and just letting it use the unit_of_measurement or the entity as the series name I get an insane number of date type errors. Even if I completely delete the database and start fresh I get the same result. By forcing everything to float I get zero errors but then its just down to getting the series in a more usable structure for looking at overall stats.

[titilambert]

@balloob Where can we document the migration ?

[titilambert]

@ArnoutVerbeken I just retested and this work for me and I don't have any error in HA logs anymore (about influxDB)
I got only one issue during the migration: The waqi component export a data named time. This causes an issue at the migration beginning.
The only workaround I found, for now, is to delete the measurement AQI... (we can not delete a field ...)
BTW with this patch, there will not tag/fields named time anymore.

You have to note:

• all numeric fields (int/float/bool) will stored as float inside influxDB
• all string fields will be renamed as FIELDNAME_str to avoid type conflit
• all value field typed as string will be renamed as state
• field named value will always be a float
• field named state will always be a string

@ArnoutVerbeken so maybe you will have to change your graph queries when you grab some string fields (to use FIELDNAME_str) otherwise all should be working...

[titilambert]

@bah2830 BTW I fixed the progress bar issue...

[MartinHjelmare]

For states the key should be "state" for strings according to the component code. State attributes have the correct key now.

[titilambert]

@MartinHjelmare Thanks for your review ! I just updated the migration script !

[MartinHjelmare]

@titilambert this looks good! Tests for the script would be icing on the cake, but I'm not sure if it's strictly needed.

[titilambert]

I found an issue in the migration script. I need more invstigation

[titilambert]

The cloning query is now fixed

[balloob]

What is the state of this, is this ok to merge for 0.36?

[MartinHjelmare]

@balloob do we need tests for the migration script? We should document the breaking change in the release notes and explain how to run the migration script (somewhere).

@titilambert I've tested the migration script on a very limited db and there it works. We should mention in the docs for the script that users should only run it on an old db, ie before starting hass after the upgrade. Otherwise, if running the script after already having started an upgraded hass, the keys in the db will be modified again by the script. Ie "FIELDNAME_str" will be turned into "FIELDNAME_str_str".

Besides these points I think we can merge.

[titilambert]

THe only issue is the migration of waqi data (AQI measurement) which have a time field (which is not recommended)

[balloob]

@titilambert the waqi time field has to get removed anyway, only JSON serializable fields should be sent over.

[ArnoutVerbeken]

So.... We got these instructions to run the migration script.
hass --script influxdb_migrator
-H IP_INFLUXDB_HOST -u INFLUXDB_USERNAME -p INFLUXDB_PASSWORD
-o test_series -d INFLUXDB_DB_NAME

• Can I omit -u -p if I don't have an influxdb user?
• what is -o test_series? Is this the new DB name I have to specify here?

Should we run the script after upgrade to 0.36 but before first start of 0.36? This has been said here somewhere but it is not in the documentation page....

I just don't want to loose my DB data...

Thanks

[MartinHjelmare]

I'll write up a PR with some docs now. Happy if @titilambert can review that when it's up.

@ArnoutVerbeken

• You should be able to omit username and password, if authentication is disabled. It is by default.
  See:
  https://docs.influxdata.com/influxdb/v1.1/query_language/authentication_and_authorization/#authorization
• The -o option is to set an override measurement where all data points will be stored.
• Yes, run the script after upgrade to 0.36 but before first regular start of hass.
• If you run the script with only the -h option, you will get a help printout with a short explanation of the different options.

Sorry that all docs wasn't ready at release. We'll try to make it right asap.

[ArnoutVerbeken]

Thank you. Do we need to override measurement or is there a default if we omit it? It seems we can leave it out, but it is in the example.

[MartinHjelmare]

You do not need to set the override_measurement. If that is not specified, the old measurement names will be retained. It's only if you want to change how the data is stored and instead store everything under a single measurement.

[ArnoutVerbeken]

OK, understood. What is the best practice to do with all the changes in this PR: Single measurement or leave like it is?
Or does it really depend on the needs of the user? (Once more, I am an "Influx consumer" and don't really know the inner works or best practices of this).

Thanks.

[MartinHjelmare]

I'd recommend you to leave it like it is. If you don't know that you want to change this, I suggest you don't change it.

The default way in 0.35 was to use the unit_of_measurement as measurement if there is one for the data to be stored, otherwise the component will look for a user specified default_measurement, otherwise use the entity_id. With 0.36 the user has the option of storing all data in a single measurement, specified via override_measurement in the config and override the default behavior.