opentelemetry-collector
opentelemetry-collector copied to clipboard
[confmap] Strings containing numbers in nonstandard notation are normalized to decimal notation
Describe the bug When using the $env:ENVIRONMENT_VARIABLE_NAME syntax in the collector configuration a string with numbers only starting with zero is interpreted as a number.
Steps to reproduce https://github.com/rassmate/opentelemetry-collector/commit/855679d1b20f46b214f2389d590ee79573617337
What did you expect to see? When working with environment variables I expected to get the exact value that the environment variable holds.
What did you see instead? A string got interpreted into a number very different from the one in the environment variable when the value had a leading zero.
What version did you use? v0.84
What config did you use?
env:
name: TEST
value: "0123"
...
processors:
transform:
error_mode: ignore
trace_statements:
- context: span
statements:
- set(resource.attributes["TEST"], "${env:TEST}")
...
Environment OS: (e.g., "Ubuntu 20.04") Compiler(if manually compiled): (e.g., "go 14.2") Macbook air m2. Same error in openshift cluster
Additional context Add any other context about the problem here.
Thanks for reporting this.
This comes from the YAML library we use, a minimal example can be seen here: https://go.dev/play/p/eO1O_q2ITAU. It is documented on the README:
Octals encode and decode as 0777 per YAML 1.1, rather than 0o777 as specified in YAML 1.2, because most parsers still use the old format
You don't need to be using the ${env:..}
feature for this: directly setting an integer-type option to 0123
will cause it to be interpreted as the equivalent octal number (83).
In your particular case, setting TEST to "!!str 0123"
(using YAML explicit tags) or with explicit quotes as in "\"0123\""
would be a valid workaround (this workaround is valid so long as the variable is not being used to set a configuration value of int
type).
I asked upstream for a toggle on the library to avoid this quirk on go-yaml/yaml/issues/996.
Ok, thanks for the clarification.
I think it is a bit strange though that in the envprovider.go even use the yaml-parser to get environment variable values. I would have guessed on just fetching the clean value from the environment without any parsing.
Right, the ${env:...}
feature actually supports YAML syntax, e.g. you can do
processors:
transform: ${env:TRANSFORM_CONFIG}
and pull in your full transform processor configuration from the TRANSFORM_CONFIG
environment variable.
Your issue though points to a general problem where our configuration loading system does not know it is mapping to a string and it loses the original representation of string values that can be interpreted as integers/floats.
I wrote a small example here: https://go.dev/play/p/3gXGDllytzd, in summary with a config struct like:
type Config struct {
Octal string
Hex string
Scientific string
Positive string
Infinite string
}
a configuration like:
Octal: 0123
Hex: 0xdeadbeef
Scientific: 1.23e5
Positive: +123
Infinite: .inf
when loaded through ${env:...}
or any other provider, will get the following loadedConfig
struct:
loadedConfig := Config{
Octal: "83",
Hex: "3735928559",
Scientific: "123000",
Positive: "123",
Infinite: "+Inf",
}
where even if the fields are string
, their original values are not kept and instead a normalized representation of the strings as integers/floats is applied.
Thanks for a great explanation!
Since env is such a well established term I as a new user does not expect the support for yaml-parsing on my environment variables although i like the feature.
Perhaps a solution could be suggested where we explicitly can call for the raw value of the environment variable?
${env:YAML_VALUE_OR_OTHER}
${env_raw:RAW_ENV_VALUE_NOT_RUN_THROUGH_YAML_PARSER}
I've managed to work around it by adding the double quotes in the value so it is not critical for me. Just trying to share a newbies thoughts.
@rassmate If you use ${ENV_VAR}
instead of ${env:ENV_VAR}
, then you get the verbatim string value, don't you?
I think we can use something like this https://go.dev/play/p/g8hciqbI921 to fix this, but I want to first agree on how ${env:..}
, ${..}
and how confmap resolution more generally should work, for which I will open a separate document/issue/something else.
As part of the RFC we decided that ${FOO}
and ${env:FOO}
would be resolved the same way (at least for our distros) which means supporting YAML syntax for both styles.
@mx-psi what needs worked on for this issue? If there are reasonable work arounds, such as export TEST=\"0123\"
, I propose we remove this as a 1.0 requirement.
Sorry for ghosting you on this issue for a while but I like what I read and some thought is put in to this matter.
Keep up the good work!
@TylerHelmuth This will be solved by setting WeaklyTypedInput
to false
, there is nothing else needed here