-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Set default metricsets in Docker module #6718
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
- module: docker | ||
metricsets: | ||
- "container" | ||
- "cpu" | ||
- "diskio" | ||
- "healthcheck" | ||
- "info" | ||
#- "image" | ||
- "memory" | ||
- "network" | ||
hosts: ["unix:///var/run/docker.sock"] | ||
period: 10s | ||
enabled: true | ||
|
||
# Replace dots in labels with `_`. Set to false to keep dots | ||
labels.dedot: true | ||
|
||
# To connect to Docker over TLS you must specify a client and CA certificate. | ||
#ssl: | ||
#certificate_authority: "/etc/pki/root/ca.pem" | ||
#certificate: "/etc/pki/client/cert.pem" | ||
#key: "/etc/pki/client/cert.key" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,5 @@ | ||
- module: docker | ||
metricsets: ["container", "cpu", "diskio", "healthcheck", "info", "memory", "network"] | ||
hosts: ["unix:///var/run/docker.sock"] | ||
period: 10s | ||
|
||
# Replace dots in labels with `_`. Set to false to keep dots | ||
labels.dedot: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the default, so I guess it is not needed in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason I left this in is because we plan to change the default here in 7.0. So if it is in the config most people use, 7.0 will be less breaking for existing users. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok, right then. |
||
|
||
# To connect to Docker over TLS you must specify a client and CA certificate. | ||
#ssl: | ||
#certificate_authority: "/etc/pki/root/ca.pem" | ||
#certificate: "/etc/pki/client/cert.pem" | ||
#key: "/etc/pki/client/cert.key" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,9 +14,9 @@ import ( | |
// init registers the MetricSet with the central registry. | ||
// The New method will be called after the setup of the module and before starting to fetch data | ||
func init() { | ||
if err := mb.Registry.AddMetricSet("docker", "image", New); err != nil { | ||
panic(err) | ||
} | ||
mb.Registry.MustAddMetricSet("docker", "image", New, | ||
mb.WithHostParser(docker.HostParser), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It had no host parser before 🤔 but I guess it should have had it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, good point. Yes it should have one. |
||
) | ||
} | ||
|
||
// MetricSet type defines all fields of the MetricSet | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,5 @@ | ||
- module: docker | ||
metricsets: ["container", "cpu", "diskio", "healthcheck", "info", "memory", "network"] | ||
hosts: ["unix:///var/run/docker.sock"] | ||
period: 10s | ||
|
||
# Replace dots in labels with `_`. Set to false to keep dots | ||
labels.dedot: true | ||
|
||
# To connect to Docker over TLS you must specify a client and CA certificate. | ||
#ssl: | ||
#certificate_authority: "/etc/pki/root/ca.pem" | ||
#certificate: "/etc/pki/client/cert.pem" | ||
#key: "/etc/pki/client/cert.key" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of the scope, but I think this should be the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on that.