-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[regression] v1.27 shell-like expansion replaces non-environment variables #13432
Comments
It might be due to #13229 |
@powersj the docker-compose library doesn't handle the case of
Both of these solutions are somewhat breaking the existing configurations unfortunately. 😞 I checked the codebase and I believe only regex processor has this issue. If none of them are acceptable, we may need to revert the PR. |
When I tested this I was more concerned about not breaking the TOML parser and less about how we maintain backwards compatibility. I clearly missed the various regex like usage throughout the TOML config with Telegraf. We do not want to break configurations between versions so we want to ensure the previous behavior is maintained. In order to add your new feature, which I really do want to keep, @srebhan and I have two possible ideas:
What do you think? |
@powersj The second option seems feasible and basically enforces users to check their existing configurations if they are using the feature flag. Let me know your thoughts. |
@powersj and @neelayu I think it is not so difficult you just have to do some preprocessing... i.e.
Alternatively, you can even use the |
Is there a case where this is a valid regex or usage in the existing processors? We are letting this feature take over the usage of the double dollar sign and I want to make sure we are not still stepping on someone's current and valid usage. |
Yes I have considered the possibility of using SubstituteWith func. Just to clarify- |
@neelayu OF COURSE I took care of this. ;-) Now patterns |
I just found the right package in the PR build artifacts and installed it :-) And: yes, this would fix the issue. Both is working then: |
@peerfiet can you please test the binary in #13457 once CI finished the tests successfully? We needed to change the behavior again as we found other cases where the implementation broke existing configs... So now your original config )(the one with |
Sure. I will let you know tomorrow asap.
|
@srebhan I get a lengthy error message:
|
@peerfiet can you please show me the [[processors.regex]]
order = 50
namepass = ["wallbox_status"]
[[processors.regex.fields]]
key = "0_chargingRate"
pattern = "Charging rate: (.*)kw.*\\[(.*)A (.*)pf \\]"
replacement = "${1}"
result_key = "charge_kW" that should work again now without further changes? |
@srebhan Here is my conf, that is giving above error (error loading config file /etc/telegraf/telegraf.d/inputs.conf: error parsing data: Invalid template:)
|
@peerfiet can you please try again once the updated version is built by our CI!? |
@srebhan Using the binary from #13457 (comment) I can confirm above config is working as expected. Furthermore, using |
Relevant telegraf.conf
Logs from Telegraf
System info
Telegraf 1.27.0
Docker
No response
Steps to reproduce
This is the string used as input to regex plugin: "Charging rate: 0kw 3 phase [0A pf ] "
Expected behavior
field "charge_kW" should contain "0"
Actual behavior
field "charge_kW" contains "1}"
Additional info
changing
replacement = "${1}"
to
replacement = "$1"
makes it work as expected again in Telegraf 1.27. However, the documentation of the regex plugin is suggesting to use "${1}", which was working up to 1.26.x
The text was updated successfully, but these errors were encountered: