Refactor OpAMP bridge logging and configuration #2196
Conversation
|
@jaronoff97 What is your view on #1559 (comment) ? Because, that would require change in the OpAMP bridge code as well as in the Operator. |
|
@avadhut123pisal @srikanthccv i like the recommendation, but rather than using a struct, i'm going to use an enum and then a map from enum -> bool. This provides the same YAML interface, but is much cleaner to work with in code because I don't need to explicitly check the presence of each field. |
…or into refactor-bridge-config
|
My reasoning behind using typed struct is as the bridge implementation improves, we will find ourselves checking if a capability exists before performing certain actions (for instance, AcceptConnectionSettings). I find it better to do some like |
|
We are able to do that with a map check with an enum as well. I'm trying to avoid needing to do an if on every struct field for validation or defaulting. |
|
I am more biased towards the end user's POV/experience than what needs to be done in our codebase. The enums/strings are prone to typos. The struct makes it clear about the fixed set of capabilities that the user can enable or disable and works nicely with the IDE yaml schema generation and validation. I don't have a strong opinion. I am fine with either of the approaches. |
|
Using a map is the same user experience while also being more ergonomic for use in go by creating constants for each capability. We also only use these types as an intermediary for the actual capabilities struct which has what you are asking for. |
…lemetry-operator into refactor-bridge-config
| # Use pipe (|) for multiline entries. | ||
| subtext: | | ||
| `components_allowed` => `componentsAllowed` | ||
| :x: `protocol` which is now inferred from endpoint |
There was a problem hiding this comment.
what does :x: mean? Does it mean that is has been removed?
As a side quest from #1368, this PR changes how config is loaded in to the bridge as well as the logging structure.
This has some breaking changes around the config. This is okay because currently no one is running the bridge (AFAIK!)
components_allowed=>componentsAllowed❌
protocolwhich is now inferred from endpointcapabilities
[]string=>map[Capability]boolfor enhanced configuration validation