SUSE / sap_host_exporter

Prometheus exporter for SAP systems (SAP S/4 HANA and SAP Netweaver hosts)
Apache License 2.0
34 stars 13 forks source link

RFC: Zero config Exporter-> automatic discover the target. #54

Open MalloZup opened 4 years ago

MalloZup commented 4 years ago

I do have though a bit and I found out that configuring the sap_host exporter is by user not trivial.

we don't offer the same experience we offer like the ha_cluster_exporter without any configuration needed by default.

I think we can configure this all dynamically and retrieve the Instances automatically and serve them.

This is also a pattern in prometheus exporters

https://github.com/wrouesnel/postgres_exporter#automatically-discover-databases

https://github.com/wrouesnel/postgres_exporter/pull/215

We might need to research also how we could offer such experience.

I think that using systemd It is still an elegant solution but we can always improve :grin:

I'm adding as RFC because we might think.

The same would be valuable for hanadb exporter where we don't need all this config files. if we can implement it here.

stefanotorresi commented 4 years ago

Hmm, I'm not sure I understand. What did you find non-trivial?

At the end of the day, sap-control-uds is the only thing most users will be required to set. We could provide a default value for that, i.e. /tmp/.sapstream50013, which would work OOTB in most netweaver installations.

MalloZup commented 4 years ago

yes setting a default value would be good!

@stefanotorresi https://github.com/SUSE/sapnwbootstrap-formula/blob/master/netweaver/monitoring.sls

My intent would be if we run like the ha_cluster_exporter:

./ha_cluster_exporter and everything works. the users doesn't need to search by default the config.

the same aspects we should work on the hanadb exporter.

putting config values should be imho something advanced which is error prone for users

stefanotorresi commented 4 years ago

Ok then, I guess we can make the UDS the default. This was not done initially because we didn't have UDS support in the first place, the default was TCP/IP on localhost. Note this will require to change how the precedence of the connection parameters works: right now, UDS has precedence on TCP/IP so, if we provide a default value for the UDS path, a user would never be able to use TCP/IP instead. We either need to reverse the precedence (so, if a user gives TCP/IP parameters, those are used, otherwise UDS is used by default), or we need to add a new parameter that explicitly indicates which of the transport a user might wish to use, and provide default values for both types (e.g. /tmp/.sapstream50013 and localhost:50013 respectively). In the way it's coded right now, we cannot have default values for both, because we establish the precedence by checking if a value is provided.

MalloZup commented 4 years ago

@stefanotorresi fair enough. Yes no worries, I think initially we introduced the config file because of authentication and at that time we didn't know UDS was possible..

I like personally your 1st suggestion, to change the precedence order.

The rationale behind this is to make easy the deployment of our monitoring solution. (also in regard with SUMA integration) or other any possible deployment automation.

E.g right now, we are advising the user to install our monitoring solution with formulas.

We both know that this is not an elegant solution. Especially this might be not the best one for already deployed systems where we cannot know the state they are.

Ideally if we can for all exporter we ship provide them with "0" configuration, we can then integrate our exporter here:

https://github.com/SUSE/salt-formulas/tree/master/prometheus-exporters-formula

I initially also wrote a PR for that, but then I realized that the sap_host_exporter and more the hanadb are doing to much magic with sid and so on via salt. (see https://github.com/SUSE/saphanabootstrap-formula/blob/master/hana/monitoring.sls) Partially because we need to extract the pydbapi close sourced driver and other things.

I believe technically speaking, it is an anti-pattern the whole monitoring.sls state, which we historically did but I believe an exporter should be really self-sufficient.

I think sap_host_exporter can be a 1st step on providing this "gold" user experience. The 2nd step would be the hanadb_exporter.

I'm tracking this in a meta issue/story.

stefanotorresi commented 4 years ago

Consider also that changing this configuration precedence constitutes a backwards incompatible change. E.g. at the moment, a user might have had a configuration file with values for TCP/IP, but then switched to UDS later by adding the path to the socket, and left the TCP/IP config values there, because they get ignored anyways. Changing the precedence would break this kind of configuration, as those old configuration values suddenly have precedence. I'm not sure we want that.

MalloZup commented 4 years ago

Ok true. I guess we know the problem now. I would think the implementation lets see later on.