Allow to choose hostname and FQDN to be lowercased or not #236
Conversation
AndersonQ
added
the
Team:Elastic-Agent-Data-Plane
AndersonQ
force-pushed
the
set-lowerHostname
branch
3 times, most recently
from
0b70157
to
5167c16
Compare
providers.LowercaseHostname and providers.SetLowerHostname allow to control hostname case sensitivity This addresses the behavior change introduced in v1.11.0 and reverted in v1.14.1. By default, hostnames are not lowercased.
AndersonQ
force-pushed
the
set-lowerHostname
branch
from
5167c16
to
6ed87ca
Compare
There was a problem hiding this comment.
I think this library should not handle this normalization for users. It's easy enough that any caller that wants normalization can do strings.ToLower on their own. I do however think types.HostInfo could more explicitly document that the value is passed-through without any normalization.
go-sysinfo has avoided any global state thus far because this was a problem we observed with similar libraries. If two usages in the same process have different needs then they end up stepping on each other when configuring the global SetLowerHostname. We should avoid this.
I have mentioned this in other places, but haven't opened an issue (will do). The providers/ packages should be treated as internal, and I would like to make this official by moving it to internal/providers in a "v2" at some point. This way the only API surface is the top-level API. This will give us a little more flexibility in refactoring and force users to go through the front door as it was designed. This would mean that the lowercase behavior must be configurable as a ProviderOption, but I don't think this is a necessary configuration option.
|
@andrewkroh fair point, that was a quick fix, I changed it now using |
| @@ -45,6 +45,12 @@ func WithHostFS(hostfs string) ProviderOption { | |||
| } | |||
| } | |||
|
|
|||
| func WithLowerHostname() ProviderOption { | |||
There was a problem hiding this comment.
This needs to have some godoc explaining what it does (and what fields are affected) and what the default behavior is if you don't set the option.
|
I'll close it for now as we found another solution to upgrade |
Adds a provider option to choose to lowercase the hostname/FQDN
This addresses the behavior change introduced in v1.11.0 and reverted in v1.14.1. By default, hostnames are not lowercased.
Related issues