Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
MalloZup opened this issue Oct 7, 2020 · 6 comments
Open

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

MalloZup opened this issue Oct 7, 2020 · 6 comments

Comments

@MalloZup
Copy link
Contributor

MalloZup commented Oct 7, 2020

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

prometheus-community/postgres_exporter#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 😁

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.

@MalloZup MalloZup changed the title RFC: Zero config Exporter RFC: Zero config Exporter-> automatic discover the target. Oct 7, 2020
@stefanotorresi
Copy link
Collaborator

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
Copy link
Contributor Author

MalloZup commented Oct 8, 2020

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
Copy link
Collaborator

stefanotorresi commented Oct 8, 2020

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
Copy link
Contributor Author

MalloZup commented Oct 8, 2020

@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
Copy link
Collaborator

stefanotorresi commented Oct 8, 2020

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
Copy link
Contributor Author

MalloZup commented Oct 8, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants