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

Added a flag to ignore replications updated_on #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nevans
Copy link
Contributor

@nevans nevans commented May 10, 2018

I'm running servers which have tens of thousands of unique replication documents, hundreds of which are running at any one point in time (using 2.1's scheduler). It might occasionally be useful to know the last updated_on, but I can get that data just as easily via CouchDB's /_scheduler/docs API: http://docs.couchdb.org/en/2.1.0/api/server/common.html#scheduler-docs, and don't need it to be tracked (and taking disk space) by prometheus. So I added a -couchdb.replsupdate=true boolean flag, which can be used to opt-out.

@nevans
Copy link
Contributor Author

nevans commented May 10, 2018

I didn't add any tests (yet). Let me know what you think.

@gesellix
Copy link
Owner

Tests would be nice, for both cases with enabled and disabled updated_on reporting. The default case should already be covered, so I assume you'd only have to add a test case for disabled replsupdate.
Can you also add the check (and test) for replsupdate in the v1 collector? I'm always trying to keep v1 and v2 behaviour in sync.

Travis CI already complains about something, but I haven't looked into the build log, yet.

... and thanks for the hint about the CouchDB 2.1.x new /_scheduler/docs api! :)

@nevans
Copy link
Contributor Author

nevans commented May 11, 2018

The test failure looks like an obvious failure to even build the tests (and update the test code to match the updated NewExporter func signature) on my part. I'll fix the tests, add new test cases, and update v1 collector later today.

Does the replsupdate name (especially for the command line flag) sound good to you, or do you have a better suggestion?

@gesellix
Copy link
Owner

Does the replsupdate name (especially for the command line flag) sound good to you, or do you have a better suggestion?

Good question - and I actually have been thinking about it without having a much better idea.

I cannot predict whether that flag will be kept as is, because I have no idea whether similar flags will be necessary in the future. When there would be more flags like the replsupdate, I would try to have an option like tracked-details=replication.updated_on,replication...,foo.bar, .... In other words: a config option with a whitelist of tracked details might make more sense. For now I would start with the dedicated flag instead of the list. What do you think?

For the time being I would suggest something like track-replication-update - with the more important aspect of expanding repls to replication.

@nevans
Copy link
Contributor Author

nevans commented May 14, 2018

Sorry, got pulled off into other things. Will get to this tomorrow.

I like -track-replication-update. Works for the time being at least.

-tracked-details=csv could make long term sense, but I'd want either a +/- system (to modify defaults) or a blacklist. Whitelist seems like it would be more config work than necessary for the common case, which I'd assume is to track everything except for unbounded (like replications) and experimental options. For that matter, I think the command line -help flags are probably better self-documenting if we just make a separate command line flag for each option. In which case, keeping a common prefix (e.g. -tracked-foo) is helpful. What do you think?

@gesellix
Copy link
Owner

I like -track-replication-update. Works for the time being at least.

let's got for it, then 👍

... track everything except for unbounded (like replications) and experimental options

a personal note about that aspect: I always keep in mind that the exporter needs to make several http requests to the CouchDB and I'm unsure where to make a cut. So I tend to keep the amount of requests minimal, but maybe I'm not always making the best decision. Feedback like yours from another real life user is really helpful in that regard.

@gesellix
Copy link
Owner

@nevans did you already have a chance to rename the flag?

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

Successfully merging this pull request may close these issues.

None yet

2 participants