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.

Sign up for GitHub

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

Add a filter to only include *.conf files from collectd.d. #120

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add a filter to only include *.conf files from collectd.d. #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95b77bd
Add a filter to only include *.conf files from collectd.d.
igorpeshansky Mar 4, 2021
5486199
Bump minor version because a config include filter is technically a n…
igorpeshansky Mar 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VERSION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# The release version and build number of the stackdriver-agent package.
PKG_VERSION=6.1.3
PKG_VERSION=6.2.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I have very contradicting feelings about this. I don't consider this to be a change in the API because the agent still behaves exactly the same for its purpose of collecting/sending metrics. The only change is to a config file that isn't fixed -- and that configurability is a "part of the API" (eh, sort of, maybe, not really, but you get it). That part of me wants to say that this doesn't even deserve a patch bump, but I do think there's merit in being able to differentiate between versions of the agent with the default changed vs not -- thus I'd vote patch bump.
(side question: if a user upgraded, their config wouldn't be overwritten with this right? (and yes yes, templates and new VMs that don't have one to begin with, inconsistencies etc etc))

If we /do/ consider a config file (that's shipped with the agent by default but configurable by the user by feature) to be a core part of the API, then I'd say this should be a major version bump.

PKG_BUILD=1
9 changes: 6 additions & 3 deletions collectd.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ PostCacheChain "PostCache"
</Rule>
</Chain>

# if you have other config, especially for plugins, you can drop them
# into this directory
# Obsolete config directory for backward compatibility.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users could have put files into the other dir without the .conf extension, right? Since that's already not totally backwards-compatible (changing that one to take .conf only), I'd say for consistency we should either:

  1. get rid of 2 dir options and just leave the new block including .conf (131-133), or
  2. have both dir options, and they both only accept .conf files

(i'm partial to the former)

Include "/opt/stackdriver/collectd/etc/collectd.d"
Include "/etc/stackdriver/collectd.d"
# If you have other config, especially for plugins, you can drop them
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/config/configs

# into this directory.
<Include "/etc/stackdriver/collectd.d">
Filter "*.conf"
</Include>