Stackdriver / collectd

Stackdriver's monitoring agent based on collectd (http://collectd.org).
https://cloud.google.com/monitoring/agent/
Other
51 stars 15 forks source link

Fix mongoc header and update write_mongodb.c from upstream. #148

Closed luna391 closed 5 years ago

googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

luna391 commented 5 years ago

I signed it!

googlebot commented 5 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

luna391 commented 5 years ago

@nina-marie @gbprz can any of you review this PR. I'm working on packaging stackdriver for nixos and this is a blokcer for us. Thanks for your time and help.

igorpeshansky commented 5 years ago

@luna391, can you please elaborate your use case? Stackdriver currently does not support nixos. What configure options are you using to build? Did you test this change on the existing platforms to make sure it doesn't break?

luna391 commented 5 years ago

@igorpeshansky thanks for the quick follow-up. We're working on packaging Stackdriver to be working on NixOS. We have GCP instances running on NixOS and we wanted to start collecting disk usage, memory usage ... which can't be accomplished without Stackdriver agent being installed. The Stackdriver packaging is inspired from collectd packaging, for further details: https://github.com/NixOS/nixpkgs/tree/master/pkgs/tools/system/collectd https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/monitoring/collectd.nix

Now regarding the issue we faced while using the "stackdriver-agent-5.5.2" code, while we're compiling an error is thrown: src/write_mongodb.c file is including a header that does not exist which is mongo.h

`write_mongodb.c:42:10: fatal error: mongo.h: No such file or directory

include

      ^~~~~~~~~

compilation terminated. make[3]: *** [Makefile:4970: write_mongodb_la-write_mongodb.lo] Error 1`

Checking the code, we found that there's no mongo.h file and there's only the "mongoc.h" header and that's why we tried to fix it. As you can see in the code that exists in "5.8.1" branch the correct header name is being used, line 39: #include <mongoc.h> https://github.com/Stackdriver/collectd/blob/collectd-5.8.1/src/write_mongodb.c

Please let me know if you need further details. Thanks,

igorpeshansky commented 5 years ago

Ah, I see now. We don't bundle the write_mongodb plugin into our packages, so never noticed that it's broken. Do you actually need write_mongodb, or are you simply after a NixOS equivalent of the official Stackdriver package? Note that our packages are built using the recipes in Stackdriver/agent-packaging — you might want to emulate those (especially the configure options) on NixOS. Also note that on newer OS versions (e.g., Ubuntu 18.04) we are using --with-libmongoc=yes instead of --with-libmongoc=own, to avoid the (also broken) vendored version of libmongoc.

luna391 commented 5 years ago

@igorpeshansky, we're working to get a NixOS equivalent of the official Stackdriver package. I'll check the recipes in Stackdriver/agent-packaging .That being said, can we merge this PR to fix the write_mongodb plugin ? thanks.

luna391 commented 5 years ago

@igorpeshansky we're currently testing the NixOS packaging, for now build succeeded but we're not able to see that metrics are being sent from GCP instances. I found other issues in write_mongodb.c file so I've copied the one used in collectd-5.8.1 branch, that made our build succeed. So you might check the changes pushed as part of your PR and if you have issues please raise them. Thanks !

igorpeshansky commented 5 years ago

As I said before, I wouldn't expect write_mongodb to work or build at all, so rather than trying to fix it, I'd suggest abandoning this PR and simply not enabling it in your packaging. I'm wary of pulling in changes from the collectd-5.8.1 branch into stackdriver-agent-5.5.2 — we're working on a separate rebase against 5.8.1 that will eventually supersede this branch.

luna391 commented 5 years ago

@igorpeshansky we finally were able to package Stackdriver on NixOS, the missing part was related to the configuration once we've fixed it we were able to install the agent and start sending the needed metrics. We've used the code in this PR: src/write_mongodb.c , that was the only needed update we need to push to stackdriver-agent-5.5.2. Thus if it's possible to merge this PR to fix the broken src/write_mongodb.c since you're not using any ways in your packaging but it's currently being used in ours and the updated version is working fine. Please let me know what do you think. Thanks,