blakelead / couchbase_exporter

Export metrics from Couchbase Server for Prometheus consumption
Other
35 stars 18 forks source link

Switch to using https for connection to Couchbase cluster #34

Closed bitdba88 closed 5 years ago

bitdba88 commented 5 years ago

Change to default to https://localhost:18091 instead of the non secure port since the code is pushing a password in the connection. Need to also add the ability to change TLS setting to ignore self signed certificates.

You can check my code on the fork https://github.com/bitdba88/couchbase_exporter if that makes it easier.

blakelead commented 5 years ago

Hey @bitdba88 ,

That's a big contribution thanks a lot! If you don't mind I'll add your additions (with minor modifications) and acknowledge your contribution in the README.

After checking your code I did not understand the purpose of the second dbURI. Could you tell me more about that ?

bitdba88 commented 5 years ago

That was so in a multi node environment if the first node did not respond it will move to another node. I need to double check because when I have run Coucbbase in a cluster I have always only gone to one node to see everything going on for that cluster. But I read one of questions that said you need to run one connection per node. I don't think that is true but I will need to test to see what is being return. When I tested you're 0.7.0 code I could see all nodes in the cluster metrics return but will need to double check tomorrow.

bitdba88 commented 5 years ago

It was from this answer that I questioned if the changes for the second URI was required.

https://github.com/leansys-team/couchbase_exporter/issues/29

bitdba88 commented 5 years ago

I think I was looking at a different exporter before I forked yours. In another exporter it shows all the nodes are healthy from a single scrape. https://github.com/totvslabs/couchbase-exporter.git. but this code is expecting to run on every node like node exporter. I can live with that. Do you have the prometheus setup your using? I don't see that in the examples. I'm going to remove the second URI from my fork as it has no benefit.

blakelead commented 5 years ago

I'm using Ansible to install Prometheus and that is the configuration template associated with it (I removed the parts unrelated to Couchbase):

global:
  scrape_interval: 10s
  scrape_timeout: 5s # must be lower than scrape_interval
  evaluation_interval: 5s

rule_files:
- ./couchbase-rules.yml

scrape_configs:
- job_name: Couchbase
  static_configs:
{% for host in groups['couchbase'] %}
  - targets: ['{{ hostvars[host].ansible_host }}:{{ couchbase_exporter_port }}']
    labels:
      instance: '{{ hostvars[host].inventory_hostname }}'
      host: '{{ hostvars[host].ansible_host }}'
      environment: '{{ environment_name }}'
{% endfor %}

And the ./couchbase-rules.yml file is actually the one you can find in examples directory.

I proposed earlier to copy your code into the exporter and mention you in the readme, but if you prefer to submit a PR that's great too!

bitdba88 commented 5 years ago

I did the pull request. Hopefully it's correct.

blakelead commented 5 years ago

Hi @bitdba88 ,

I modified a bit what you proposed and pushed it to develop branch. But since I don't have access to an Enterprise cluster with TLS enabled, I can't fully test the code.

Do you have the possibility (and the time) to test it? It would be really helpful.

bitdba88 commented 5 years ago

I will try to get it tested this week.

Don

On Fri, Aug 2, 2019 at 5:13 PM Adel Abdelhak notifications@github.com wrote:

Hi @bitdba88 https://github.com/bitdba88 ,

I modified a bit what you proposed and pushed it to develop https://github.com/leansys-team/couchbase_exporter/tree/develop branch. But since I don't have access to an Enterprise cluster with TLS enabled, I can't fully test the code.

Do you have the possibility (and the time) to test it? It would be really helpful.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/leansys-team/couchbase_exporter/issues/34?email_source=notifications&email_token=AL7KDBCVNUWKI2Z6JHDTSKTQCSWR7A5CNFSM4HNGSB3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3O6N3Y#issuecomment-517859055, or mute the thread https://github.com/notifications/unsubscribe-auth/AL7KDBGVPHXUFK6OVSZDXSTQCSWR7ANCNFSM4HNGSB3A .

bitdba88 commented 5 years ago

Looks good in my testing. It threw the certificate error on self signed with default. Worked fine when setting changed to true. Tested on both darwin and Redhat.

blakelead commented 5 years ago

Great! Thanks for your help and your contribution.