Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse more value types #9

Merged
merged 3 commits into from
May 3, 2020
Merged

Parse more value types #9

merged 3 commits into from
May 3, 2020

Conversation

oxplot
Copy link
Contributor

@oxplot oxplot commented May 1, 2020

Various sensors have string attributes such as "state" which are still
useful to track as metrics. For example, state may be "on" or "off".
These can be showen as 1 and 0 and be used to trigger alerts.

@hikhvar
Copy link
Owner

hikhvar commented May 1, 2020

Hello,
thank you for your PR. In general I like the idea to map string values to certain numbers. However, I think it would be better if we don't hardcode that mapping into the code. I think the best place is the MetricConfig https://github.com/hikhvar/mqtt2prometheus/blob/master/pkg/config/config.go#L42 . There can the user define a mapping of strings to numeric values.

@oxplot
Copy link
Contributor Author

oxplot commented May 2, 2020

we don't hardcode that mapping into the code

Ye, this was my quick attempt to make it work for myself, and I opened this PR to get the ball rolling here.

I think we can safely map JSON true to 1 and false to 0. As for JSON null, I've never seen it show up in practice so we can ignore that (as we do for now anyway). That only leaves strings.

For strings, here's one example config to map string values to floats:

  - prom_name: zb_state
    mqtt_name: state
    help: Discrete level state of the sensor
    type: gauge
    string_value_mapping:
      default: 1
      map:
        off: 0
        low: 0
        false: 0

@hikhvar
Copy link
Owner

hikhvar commented May 2, 2020

Hey,
that sounds good. I think we should still use the current "try to parse string numbers" implementation if the user did not specify an eplixit string_value_mapping. What is the purpose of the default value? Is this value set if the parsing failed? If that is the case I would prefer the name error_value.

@oxplot
Copy link
Contributor Author

oxplot commented May 3, 2020

I think we should still use the current "try to parse string numbers" implementation if the user did not specify an eplixit string_value_mapping

Yep

What is the purpose of the default value? Is this value set if the parsing failed? If that is the case I would prefer the name error_value.

Sounds good.

There is another thing to consider: as there is no standard on what values each sensor attribute can have, there should be a way in the config to specify filters that limits which mqtt topics the rules apply to, either based on the topic name via a regex, or based on attributes. Here is an example:

  - prom_name: zb_state
    mqtt_name: state
    sensor_name: ".*brand_a.*"
    ...

We can look at this in another PR though.

* Parse bools as 0 for false and 1 for true
* Allow mapping between string and float values
@oxplot
Copy link
Contributor Author

oxplot commented May 3, 2020

I pushed a fresh change with config changes.

@hikhvar
Copy link
Owner

hikhvar commented May 3, 2020

LGTM. Thank you for your contribution!

@hikhvar hikhvar merged commit 443f437 into hikhvar:master May 3, 2020
@hikhvar
Copy link
Owner

hikhvar commented May 3, 2020

Released in v0.0.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants