You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This question is related to #11734 which is working towards #10260 and #10266. The PR I have open enables the decoding of nil values in mapstructure so that we can use an Optional type with a default value (see #10260 (comment) for more context). While enabling the decoding of nil values, I came across an issue where we were unable to differentiate between nil and empty slices. The reason for that is because we always initialize a nil slice and do not populate it if it is empty (see https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/confmap.go#L135). In order to get around this, I added this patch in the PR above.
ifm==nil {
returnnewSlice
}
newSlice= []any{}
This allows us to differentiate between a nil slice and an empty slice which we need in the zeroSliceHookFunc. However, that patch causes this unit test to fail because there's an expectation here that an empty slice gets rendered as nil when calling ToStringMap. I wanted to ask if it was valid to change this expectation so that it matches the decoding workflow we have in which an empty slice is preserved. cc @yurishkuro
The text was updated successfully, but these errors were encountered:
I would be okay changing this (I consider this an implementation detail). I believe this test was added by @songy23 because a change broke some tests on the Datadog Agent, but so long as it is clearly listed in the changelog and it does not break existing use cases (Datadog or otherwise) I am okay with it.
This question is related to #11734 which is working towards #10260 and #10266. The PR I have open enables the decoding of
nil
values inmapstructure
so that we can use an Optional type with a default value (see #10260 (comment) for more context). While enabling the decoding of nil values, I came across an issue where we were unable to differentiate between nil and empty slices. The reason for that is because we always initialize a nil slice and do not populate it if it is empty (see https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/confmap.go#L135). In order to get around this, I added this patch in the PR above.This allows us to differentiate between a nil slice and an empty slice which we need in the
zeroSliceHookFunc
. However, that patch causes this unit test to fail because there's an expectation here that an empty slice gets rendered as nil when callingToStringMap
. I wanted to ask if it was valid to change this expectation so that it matches the decoding workflow we have in which an empty slice is preserved. cc @yurishkuroThe text was updated successfully, but these errors were encountered: