Skip to content

make error_value global to metric and add error_expression #157

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

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

fatpat
Copy link
Contributor

@fatpat fatpat commented Dec 27, 2024

in order to better handle errors in config metric:

  • move string_mapping_value.error_value to upper level
  • mark string_mapping_value.error_value as deprecated

if there's an error during type conversion or during execution of expression then error_value is returned (if set, otherwise an error is thrown and no metric is sent)

Readme.md Outdated
error_value: 1
# When specified, if the value cannot be parsed or expression fails, runs this expression as fallback
# if it fails, error_value will be returned
error_expression: "float(join(filter(split(a, ''), { # matches '^[0-9]$' }), ''))"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we need an error_expression? Can't we just use a normal expression to implement parsing of strings like H20241227104456? The only reason I understand are message producers writing messages in inconsistent formats so that we have to apply two different parsing strategies. Is that the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using expression is not possible because it runs after type conversion and it fails as it's not a bool, a float nor a static map string

I can see several options:

  • make expression to run before type conversion. but this would break all current expression implementation because the value sent is the value after type conversion
  • create another expression that runs before type conversion. Like expression_raw and if set, return a float from whatever value is read from MQTT bypassing the current type conversion/expression
  • add error_expression which is kind of the same of the previous one but seems more generic.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually would like to prever the second solution:

create another expression that runs before type conversion. Like expression_raw and if set, return a float from whatever value is read from MQTT bypassing the current type conversion/expression

This is more logical having a parsing expression yourself instead of doing it after the fact. In that case, if the expression returns a string, we could enable the user to parse the string first, and then apply the mapping the user specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree,

i'll push another PR with raw_expression and only keep error_value in this one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #161 and #162

* fail if both metric.error_value and
  metric.string_value_mapping.error_value are set
* mark metric.string_value_mapping.error_value as deprecated
@hikhvar hikhvar merged commit 3a9ad76 into hikhvar:master Dec 31, 2024
15 of 16 checks passed
@fatpat fatpat deleted the error_value branch January 1, 2025 06:08
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