search

victronenergy / venus-influx-loader

NodeJS server that takes from MQTT into Influx, and config UI and still more
MIT License
12 stars 4 forks source link

Add support for dot separated measurements #69

Closed kvesterling closed 1 month ago

kvesterling commented 3 months ago

Add topic as a tag enabling further functionality in grafana. Use a dot in the measurement instead of / such that we can use $1,$2,$3, etc for aliases in Grafana.

This will break a lot of dashboards currently in use, but the benefits outweigh the little bit of work to straighten it out.

mman commented 3 months ago

Thanks for the PR @kvesterling, let me please dig into this a bit deeper. Since it's gonna break everything, I am not going to merge this as is, but I think it would be a great addition while preserving backwards compatibility in a reasonable way. Let me please investigate how to move this forward...

kvesterling commented 3 months ago

It does break the grafana dashboards, BUT it's worth the pain. I've already rebuilt the majority of them. Easy fix if you just do a replace in the JSON, then it's just a few things after that. BUT much gets easier.

Let me know what you need fixed.

mman commented 2 months ago

Adding a note for myself how this is supposed to work. If you structure your InfluxDB measurements in a way that the measurement field contains multiple values separated by dots. then the parts of the measurement delimited by the dots are available in the query builder as variables $0, $1, ...

image

I am not convinced this is a way to go, as (AFAIK) general InfluxDB recommendation is to keep the number of measurements as low as possible and add as many tags as you wish to add necessary dimensions.

mman commented 2 months ago

@kvesterling Sorry for my late reply.

Could you please post perhaps couple more screenshots where and how you use the measurements with dots to actually compose a query and where having $0, $1 is benefiting you?

Also what do you actually get by copying the measurement name itself into another tag? I'm not seeing the benefit immediately.

As a side note, one of the main goals of Venus Grafana integration is to allow people to easily explore all the measurements available and make panels out of them easily - without necessarily knowing what is out there. InfluxDB data source query builder is helping this with measurement autocomplete, but we have been bitten by https://github.com/victronenergy/venus-docker-grafana/issues/8 since day zero until today, and I think if we affect how measurements are named, it would be great to help address this issue as well.

Unfortunately given how measurement auto complete is implemented (https://github.com/grafana/grafana/blob/8f38ef70ceaea12ca27bc0d856951e6e036da115/public/app/plugins/datasource/influxdb/influxql_query_builder.ts#L62) I do not see an easy way to do it.

For now please add couple more examples how separating measurement components with dots actually helps you.

I stand my initial comment though, we are not gonna break anyones dashboards, if we merge this, it will be a configurable option - perhaps default for new releases but with backwards compatibility. Thanks for your understanding.

kvesterling commented 2 months ago

I think I miscommunicated. If you store THE measurement with dots as opposed to slashes, then you can use $1, $2, $3 in grafana... Using slashes defeats that capability. If you would do so, I'd be more inclined to contribute to your branch. Even provide the updated dashboards, etc...

I was able to update the dashboards quite easily, and I agree that it does break what's there, but being able to say...

image

Is this what you mean buy auto-complete?

image
mman commented 2 months ago

@kvesterling I understand the benefit of using dots in the measurement, was just curious to see more examples like your SELECT over all AC inputs and aliasing using $3.

Could I also ask what is the reason of adding the measurement as additional tag?

And yes, for the autocomplete, the current implementation is to return first 100 rows that match what you type in and because we have currently a lot of measurements, the autocomplete only displays the beginning. My thinking is around the lines of displaying top level categories, and perhaps first subcategory (up to a second delimiter) so that you can quickly skim though the available tree of all measurements. But as I said, that's just an unrelated side note.

mman commented 2 months ago

I have created issue https://github.com/victronenergy/venus-influx-loader/issues/82 to discuss this in further detail. Please move the discussion there.

mman commented 1 month ago

Superseded by https://github.com/victronenergy/venus-influx-loader/pull/98