-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps #9978
Merged
sspaink
merged 2 commits into
influxdata:master
from
slai:kube_inventory_stops_skipping_pods
Oct 25, 2021
Merged
fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps #9978
sspaink
merged 2 commits into
influxdata:master
from
slai:kube_inventory_stops_skipping_pods
Oct 25, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous k8s client library returned times as a Time struct that specified time in Seconds and Nanos (https://github.com/ericchiang/k8s/blob/master/apis/meta/v1/generated.pb.go#L1718). Presumably sometimes the API returns pods that were created at the beginning of time or were missing creation timestamps, which should be ignored (https://github.com/influxdata/telegraf/pull/5110/files#diff-45d12681bfc2ede2a3349288a555ba38819744b3cb829cd131ad8d2b230ce8dbR26). During the switch over to the k8s/client-go library, this logic was retained (influxdata@79f5803#diff-45d12681bfc2ede2a3349288a555ba38819744b3cb829cd131ad8d2b230ce8dbR23). *However*, in k8s/client-go, times are returned as standard go time.Time structs (with a minor wrapper). Therefore checking whether `t.Second() == 0 and t.Nanosecond() == 0` actually _only checks whether the second and nanosecond components of time.Time are 0_, and not whether the entire timestamp is zero (i.e. not checking day, month, year, hour, minute). As a result, any pods that were created right at the start of a minute, e.g. `2021-10-22T09:05:00.000Z`, are currently being dropped. This PR fixes that by using the time.IsZero() method instead, and also tweaks the test to check for this scenario.
slai
changed the title
fix(inputs/kube_inventory): don't skip pods with zero s/ns timestamps
fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps
Oct 22, 2021
📦 Looks like new artifacts were built from this PR. Expand this list to get them here! 🐯Artifact URLs |
powersj
approved these changes
Oct 25, 2021
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.
Thanks for caching this and detailing the issue!
powersj
added
the
ready for final review
This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
label
Oct 25, 2021
sspaink
approved these changes
Oct 25, 2021
phemmer
added a commit
to phemmer/telegraf
that referenced
this pull request
Oct 26, 2021
* origin/master: (176 commits) fix: Linter fixes for plugins/inputs/[h-j]* (influxdata#9986) fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps (influxdata#9978) fix: update gjson to v1.10.2 (influxdata#9998) fix: procstat tags were not getting generated correctly (influxdata#9973) chore: create bug report form (influxdata#9976) fix: Allow for non x86 macs in Go install script (influxdata#9982) test: add sqlserver plugin integration tests (influxdata#9943) feat: plugins/common/tls/config.go: Filter client certificates by DNS names (influxdata#9910) feat: add option to skip table creation in azure data explorer output (influxdata#9942) docs: update nightlies links (influxdata#9989) fix: add s390x to nightlies (influxdata#9990) feat: Add more details to processors.ifname logmessages (influxdata#9984) docs: Create SECURITY.md (influxdata#9951) fix: set NIGHTLY=1 for correctly named nightly artifacts (influxdata#9987) feat: Kafka Add metadata full to config (influxdata#9833) chore: Update to AWS SDK v2 (influxdata#9647) chore: lint ignore fmt.Printf unhandled error (influxdata#9967) fix: starlark pop operation for non-existing keys (influxdata#9954) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#9876) fix: Check return code of zfs command for FreeBSD. (influxdata#9956) chore: update go to 1.17.2 (influxdata#9873) fix: Graylog plugin TLS support and message format (influxdata#9862) docs: update README with info on package repos (influxdata#9964) feat: Modbus connection settings (serial) (influxdata#9256) fix: segfault in ingress, persistentvolumeclaim, statefulset in kube_inventory (influxdata#9585) fix: add normalization of tags for ethtool input plugin (influxdata#9901) chore: remove empty build.py (influxdata#9958) fix: internet_speed input plugin not collecting/reporting latency (influxdata#9957) chore: reference db2 external plugin (influxdata#9952) chore: update readme go version from 1.14 to 1.17 (influxdata#9944) fix: decode Prometheus scrape path from Kuberentes labels (influxdata#9662) docs: fix broken link (influxdata#9812) fix: Correct conversion of int with specific bit size (influxdata#9933) fix: update golanci-lint to v1.42.1 (influxdata#9932) feat: Azure Event Hubs output plugin (influxdata#9346) feat: more fields for papertrail event webhook (influxdata#9940) fix: solve compatibility issue for mongodb inputs when using 5.x relicaset (influxdata#9892) docs: Add symlink to command documentation (influxdata#9926) docs: update contributing.md (influxdata#9914) chore: reference oracle external plugin (influxdata#9934) ...
phemmer
added a commit
to phemmer/telegraf
that referenced
this pull request
Oct 26, 2021
* origin/master: (176 commits) fix: Linter fixes for plugins/inputs/[h-j]* (influxdata#9986) fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps (influxdata#9978) fix: update gjson to v1.10.2 (influxdata#9998) fix: procstat tags were not getting generated correctly (influxdata#9973) chore: create bug report form (influxdata#9976) fix: Allow for non x86 macs in Go install script (influxdata#9982) test: add sqlserver plugin integration tests (influxdata#9943) feat: plugins/common/tls/config.go: Filter client certificates by DNS names (influxdata#9910) feat: add option to skip table creation in azure data explorer output (influxdata#9942) docs: update nightlies links (influxdata#9989) fix: add s390x to nightlies (influxdata#9990) feat: Add more details to processors.ifname logmessages (influxdata#9984) docs: Create SECURITY.md (influxdata#9951) fix: set NIGHTLY=1 for correctly named nightly artifacts (influxdata#9987) feat: Kafka Add metadata full to config (influxdata#9833) chore: Update to AWS SDK v2 (influxdata#9647) chore: lint ignore fmt.Printf unhandled error (influxdata#9967) fix: starlark pop operation for non-existing keys (influxdata#9954) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#9876) fix: Check return code of zfs command for FreeBSD. (influxdata#9956) chore: update go to 1.17.2 (influxdata#9873) fix: Graylog plugin TLS support and message format (influxdata#9862) docs: update README with info on package repos (influxdata#9964) feat: Modbus connection settings (serial) (influxdata#9256) fix: segfault in ingress, persistentvolumeclaim, statefulset in kube_inventory (influxdata#9585) fix: add normalization of tags for ethtool input plugin (influxdata#9901) chore: remove empty build.py (influxdata#9958) fix: internet_speed input plugin not collecting/reporting latency (influxdata#9957) chore: reference db2 external plugin (influxdata#9952) chore: update readme go version from 1.14 to 1.17 (influxdata#9944) fix: decode Prometheus scrape path from Kuberentes labels (influxdata#9662) docs: fix broken link (influxdata#9812) fix: Correct conversion of int with specific bit size (influxdata#9933) fix: update golanci-lint to v1.42.1 (influxdata#9932) feat: Azure Event Hubs output plugin (influxdata#9346) feat: more fields for papertrail event webhook (influxdata#9940) fix: solve compatibility issue for mongodb inputs when using 5.x relicaset (influxdata#9892) docs: Add symlink to command documentation (influxdata#9926) docs: update contributing.md (influxdata#9914) chore: reference oracle external plugin (influxdata#9934) ...
VladislavSenkevich
pushed a commit
to gwos/telegraf
that referenced
this pull request
Nov 23, 2021
phemmer
added a commit
to phemmer/telegraf
that referenced
this pull request
Nov 30, 2021
* origin/master: (176 commits) fix: Linter fixes for plugins/inputs/[h-j]* (influxdata#9986) fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps (influxdata#9978) fix: update gjson to v1.10.2 (influxdata#9998) fix: procstat tags were not getting generated correctly (influxdata#9973) chore: create bug report form (influxdata#9976) fix: Allow for non x86 macs in Go install script (influxdata#9982) test: add sqlserver plugin integration tests (influxdata#9943) feat: plugins/common/tls/config.go: Filter client certificates by DNS names (influxdata#9910) feat: add option to skip table creation in azure data explorer output (influxdata#9942) docs: update nightlies links (influxdata#9989) fix: add s390x to nightlies (influxdata#9990) feat: Add more details to processors.ifname logmessages (influxdata#9984) docs: Create SECURITY.md (influxdata#9951) fix: set NIGHTLY=1 for correctly named nightly artifacts (influxdata#9987) feat: Kafka Add metadata full to config (influxdata#9833) chore: Update to AWS SDK v2 (influxdata#9647) chore: lint ignore fmt.Printf unhandled error (influxdata#9967) fix: starlark pop operation for non-existing keys (influxdata#9954) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#9876) fix: Check return code of zfs command for FreeBSD. (influxdata#9956) chore: update go to 1.17.2 (influxdata#9873) fix: Graylog plugin TLS support and message format (influxdata#9862) docs: update README with info on package repos (influxdata#9964) feat: Modbus connection settings (serial) (influxdata#9256) fix: segfault in ingress, persistentvolumeclaim, statefulset in kube_inventory (influxdata#9585) fix: add normalization of tags for ethtool input plugin (influxdata#9901) chore: remove empty build.py (influxdata#9958) fix: internet_speed input plugin not collecting/reporting latency (influxdata#9957) chore: reference db2 external plugin (influxdata#9952) chore: update readme go version from 1.14 to 1.17 (influxdata#9944) fix: decode Prometheus scrape path from Kuberentes labels (influxdata#9662) docs: fix broken link (influxdata#9812) fix: Correct conversion of int with specific bit size (influxdata#9933) fix: update golanci-lint to v1.42.1 (influxdata#9932) feat: Azure Event Hubs output plugin (influxdata#9346) feat: more fields for papertrail event webhook (influxdata#9940) fix: solve compatibility issue for mongodb inputs when using 5.x relicaset (influxdata#9892) docs: Add symlink to command documentation (influxdata#9926) docs: update contributing.md (influxdata#9914) chore: reference oracle external plugin (influxdata#9934) ...
phemmer
added a commit
to phemmer/telegraf
that referenced
this pull request
Jan 3, 2022
* origin/master: (176 commits) fix: Linter fixes for plugins/inputs/[h-j]* (influxdata#9986) fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps (influxdata#9978) fix: update gjson to v1.10.2 (influxdata#9998) fix: procstat tags were not getting generated correctly (influxdata#9973) chore: create bug report form (influxdata#9976) fix: Allow for non x86 macs in Go install script (influxdata#9982) test: add sqlserver plugin integration tests (influxdata#9943) feat: plugins/common/tls/config.go: Filter client certificates by DNS names (influxdata#9910) feat: add option to skip table creation in azure data explorer output (influxdata#9942) docs: update nightlies links (influxdata#9989) fix: add s390x to nightlies (influxdata#9990) feat: Add more details to processors.ifname logmessages (influxdata#9984) docs: Create SECURITY.md (influxdata#9951) fix: set NIGHTLY=1 for correctly named nightly artifacts (influxdata#9987) feat: Kafka Add metadata full to config (influxdata#9833) chore: Update to AWS SDK v2 (influxdata#9647) chore: lint ignore fmt.Printf unhandled error (influxdata#9967) fix: starlark pop operation for non-existing keys (influxdata#9954) feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#9876) fix: Check return code of zfs command for FreeBSD. (influxdata#9956) chore: update go to 1.17.2 (influxdata#9873) fix: Graylog plugin TLS support and message format (influxdata#9862) docs: update README with info on package repos (influxdata#9964) feat: Modbus connection settings (serial) (influxdata#9256) fix: segfault in ingress, persistentvolumeclaim, statefulset in kube_inventory (influxdata#9585) fix: add normalization of tags for ethtool input plugin (influxdata#9901) chore: remove empty build.py (influxdata#9958) fix: internet_speed input plugin not collecting/reporting latency (influxdata#9957) chore: reference db2 external plugin (influxdata#9952) chore: update readme go version from 1.14 to 1.17 (influxdata#9944) fix: decode Prometheus scrape path from Kuberentes labels (influxdata#9662) docs: fix broken link (influxdata#9812) fix: Correct conversion of int with specific bit size (influxdata#9933) fix: update golanci-lint to v1.42.1 (influxdata#9932) feat: Azure Event Hubs output plugin (influxdata#9346) feat: more fields for papertrail event webhook (influxdata#9940) fix: solve compatibility issue for mongodb inputs when using 5.x relicaset (influxdata#9892) docs: Add symlink to command documentation (influxdata#9926) docs: update contributing.md (influxdata#9914) chore: reference oracle external plugin (influxdata#9934) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
fix
pr to fix corresponding bug
ready for final review
This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous k8s client library returned times as a generated Time struct that specified time in Seconds and Nanos (https://github.com/ericchiang/k8s/blob/master/apis/meta/v1/generated.pb.go#L1718). Presumably sometimes the API returns pods that were created at the beginning of time or were missing creation timestamps, which should be ignored -
https://github.com/influxdata/telegraf/pull/5110/files#diff-45d12681bfc2ede2a3349288a555ba38819744b3cb829cd131ad8d2b230ce8dbR26
During the switch over to the k8s/client-go library, this logic was retained and rewritten to -
79f5803#diff-45d12681bfc2ede2a3349288a555ba38819744b3cb829cd131ad8d2b230ce8dbR23
However, in k8s/client-go, times are returned as standard go time.Time structs (with a minor wrapper). Therefore checking whether
t.Second() == 0 and t.Nanosecond() == 0
actually only checks whether the second and nanosecond components of time.Time are 0, and not whether the entire timestamp is zero (i.e. not checking day, month, year, hour, minute).As a result, any pods that were created right at the start of a minute, e.g.
2021-10-22T09:05:00.000Z
, are currently being dropped.This PR fixes that by using the time.IsZero() method instead, and also tweaks the test to check for this scenario.
UPDATE: also applied this fix to the other affected k8s resources.