Sapcontrol gatherer #270
Sapcontrol gatherer #270
Conversation
| } | ||
|
|
||
| // Trick to keep the SIDs as capital letter | ||
| result := &entities.FactValueMap{Value: make(map[string]entities.FactValue)} |
There was a problem hiding this comment.
I need to apply this "trick" to keep the SIDs as capital letter. All the other gatherers have them with this format.
Other solution (with more code) would be to have internal data structs that are almost identical with the ones coming from sapcontrolapi but with snake_case json output
|
@arbulu89 the For me, what really matters, is whether is running or not . An I make a note to discuss this with them in our next call. |
| cachedFact, cacheHit := cachedFacts[factReq.Argument] | ||
|
|
||
| switch { | ||
| case len(factReq.Argument) == 0: |
There was a problem hiding this comment.
We can move this case to an if statement on line 119 before initializing the webmethod etc..
There was a problem hiding this comment.
I have moved the conditions to use if/continues instead of the switch, so we don't need to initialize variables needlesly.
| webmethod, ok := whitelistedSapControlArguments[factReq.Argument] | ||
| cachedFact, cacheHit := cachedFacts[factReq.Argument] | ||
|
|
||
| switch { |
There was a problem hiding this comment.
Could you please add {} around the cases? So we don't use only the indent but we have a clear barrier of the case structure, so it's easier to follow and read
There was a problem hiding this comment.
I have removed the switch at the end, so no need for this
|
|
||
| if factValue, err := outputToFactValue(sapControlMap); err != nil { | ||
| gatheringError := SapcontrolDecodingError.Wrap(err.Error()) | ||
| log.Error(gatheringError) |
There was a problem hiding this comment.
Could we add more info here, like the argument
| sapControlMap := make(SapControlMap) | ||
| for sid, instances := range foundSystems { | ||
| sapControlInstance := []SapControlInstance{} | ||
| for _, instanceData := range instances { |
There was a problem hiding this comment.
could we extract instanceData[0] and [1] to variables, it's used in more places, it's easier to read
sapcontrolgatherer.It supports the
GetProcessList,GetSystemInstanceList,GetVersionInfo,HACheckConfigandHAGetFailoverConfigwebmethods.It caches the outputs of the commands so they are executed only once.
I have taken the autogenerated code from sap_host_exporter
Only the strictly required fields. More info about the auto generated code here
Find here the
sapcontrolapi: https://www.sap.com/documents/2016/09/0a40e60d-8b7c-0010-82c7-eda71af511fa.htmlBy now, I have not included the
CheckHostAgent, as it is not available in the public API. In fact, I think we could leverage thesaphostctrlgathererPingfunction. I know it is not 100% the same, but I think it would validate what we need. Wdyt @abravosuse ?Anyway, if it is 100% needed, i would include it in a future PR.