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

fix(inputs/kube_inventory): don't skip resources with zero s/ns timestamps #9978

Merged
merged 2 commits into from
Oct 25, 2021

Conversation

slai
Copy link
Contributor

@slai slai commented Oct 22, 2021

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 -

	if p.Metadata.CreationTimestamp.GetSeconds() == 0 && p.Metadata.CreationTimestamp.GetNanos() == 0 {
		return nil
	}

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 -

	if p.GetCreationTimestamp().Second() == 0 && p.GetCreationTimestamp().Nanosecond() == 0 {
		return
	}

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.

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.
@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Oct 22, 2021
@slai 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
@telegraf-tiger
Copy link
Contributor

⚠️ This pull request increases the Telegraf binary size by 3.83 % for linux amd64 (new size: 132.1 MB, nightly size 127.2 MB)

📦 Looks like new artifacts were built from this PR.

Expand this list to get them here! 🐯

Artifact URLs

DEB RPM TAR GZ ZIP
amd64.deb aarch64.rpm darwin_amd64.tar.gz windows_amd64.zip
arm64.deb armel.rpm freebsd_amd64.tar.gz windows_i386.zip
armel.deb armv6hl.rpm freebsd_armv7.tar.gz
armhf.deb i386.rpm freebsd_i386.tar.gz
i386.deb ppc64le.rpm linux_amd64.tar.gz
mips.deb s390x.rpm linux_arm64.tar.gz
mipsel.deb x86_64.rpm linux_armel.tar.gz
ppc64el.deb linux_armhf.tar.gz
s390x.deb linux_i386.tar.gz
linux_mips.tar.gz
linux_mipsel.tar.gz
linux_ppc64le.tar.gz
linux_s390x.tar.gz
static_linux_amd64.tar.gz

Copy link
Contributor

@powersj powersj left a 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 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 sspaink merged commit ecd4d37 into influxdata:master 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)
  ...
powersj pushed a commit that referenced this pull request Oct 27, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants