hydrophone icon indicating copy to clipboard operation
hydrophone copied to clipboard

Fix different timezones in logs

Open letmerecall opened this issue 1 year ago • 20 comments

What's the issue?

The logs of hydrophone show time in different time zones. One seems to be local time and other in UTC.

Screenshot 2024-01-20 at 5 04 02 PM

How to reproduce?

bin/hydrophone --conformance

letmerecall avatar Jan 20 '24 12:01 letmerecall

We should be able to fix this and I don't think it will be a difficult resolution. By default the conformance image is going to use the tzinfo from the host it's running on. Typically that's going to be UTC for most cloud providers (including KIND).

We can override that by setting the TZ environment variable on the conformance pod when it's created. The slightly more tricky part will be setting the value of that variable. I don't really want timezone to be a flag folks can set. Instead we can use the time package to get the local tzinfo like:

tz, offset := time.Now().Zone()

We'll need to plumb the return value of tz into the environment variable TZ on the conformance image. But that should fix it. @letmerecall are you interested in trying to code this up?

rjsadow avatar Jan 20 '24 14:01 rjsadow

/assign

letmerecall avatar Jan 20 '24 15:01 letmerecall

Hey @letmerecall how are you doing on this? Are you still interested in trying to impliment a fix?

rjsadow avatar Feb 09 '24 14:02 rjsadow

Hey @rjsadow sorry, and yes I'll find some time and try it this weekend.

letmerecall avatar Feb 09 '24 14:02 letmerecall

We should be able to fix this and I don't think it will be a difficult resolution. By default the conformance image is going to use the tzinfo from the host it's running on. Typically that's going to be UTC for most cloud providers (including KIND).

We can override that by setting the TZ environment variable on the conformance pod when it's created. The slightly more tricky part will be setting the value of that variable. I don't really want timezone to be a flag folks can set. Instead we can use the time package to get the local tzinfo like:

tz, offset := time.Now().Zone()

We'll need to plumb the return value of tz into the environment variable TZ on the conformance image. But that should fix it. @letmerecall are you interested in trying to code this up?

@rjsadow Well, it's a bit more complicated than just tz, offset := time.Now().Zone(). time.Now().Zone() returns an abbreviation (like IST), meanwhile TZ environment variable in conformance image expect a full string (like Asia/Kolkata).

Also, we might need different logic for difference OSes 🤦🏻‍♂️ (something like this https://stackoverflow.com/questions/68938751/how-to-get-the-full-name-of-the-local-timezone)

letmerecall avatar Feb 13 '24 10:02 letmerecall

Hey @rjsadow If no one is working on it then I can work on this one just I some more details

Bharadwajshivam28 avatar Mar 02 '24 22:03 Bharadwajshivam28

@Bharadwajshivam28 feel free to give it a shot. Please assign it to yourself when you start working on it.

letmerecall avatar Mar 06 '24 18:03 letmerecall

@Bharadwajshivam28 feel free to give it a shot. Please assign it to yourself when you start working on it.

Thanks....

Bharadwajshivam28 avatar Mar 06 '24 19:03 Bharadwajshivam28

/assign

Bharadwajshivam28 avatar Mar 06 '24 19:03 Bharadwajshivam28

meanwhile TZ environment variable in conformance image expect a full string (like Asia/Kolkata).

Are you sure? The image is based on Debian and https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html mentions that format like "utc+HH" would be valid.

However I think there's no tzdata in the image at all:

$ docker run -e "TZ=UTC" --rm --entrypoint=sh registry.k8s.io/conformance:v1.29.1 -c date
Sun Apr  7 18:31:35 UTC 2024
$ docker run -e "TZ=Asia/Kolkata" --rm --entrypoint=sh registry.k8s.io/conformance:v1.29.1 -c date
Sun Apr  7 18:31:29 Asia 2024

The name changes, but the actual times do not. Which would be super confusing.

Setting the offset does work however, but it's equally confusing (my timezone is Europe/Berlin, which is UTC+2, but TZ would require the opposite sign (the correct output would be 20:31 right now):

$ docker run -e "TZ=UTC+2" --rm --entrypoint=sh registry.k8s.io/conformance:v1.29.1 -c date
Sun Apr  7 16:31:35 UTC 2024

Setting this does not affect the go-runner (times should be 20:34), neither does setting TZ=Europe/Berlin:

$ docker run -e "TZ=UTC-02" --rm registry.k8s.io/conformance:v1.29.1
2024/04/07 18:34:37 The resultsDir /tmp/results does not exist, will create it
2024/04/07 18:34:37 Running command:
Command env: []

xrstf avatar Apr 07 '24 18:04 xrstf

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Jul 06 '24 19:07 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot avatar Aug 05 '24 19:08 k8s-triage-robot

/remove-lifecycle rotten

rjsadow avatar Aug 05 '24 20:08 rjsadow

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Nov 03 '24 20:11 k8s-triage-robot

Are you sure? The image is based on Debian and https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html mentions that format like "utc+HH" would be valid.

However I think there's no tzdata in the image at all:

Thanks for looking into it, and you are correct!

Conformance image seems to use registry.k8s.io/build-image/debian-base-arm64:bookworm-v1.0.4 base which does not include tzdata.

However, even with tzdata some abbreviations are not supported, for example IST is not supported but CET is. This is what I get from latest debian image:

$ docker run -e TZ="UTC" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 11:59:22 UTC 2024

$ docker run -e TZ="Asia/Kolkata" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 12:01:09 IST 2024

$ docker run -e TZ="IST" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 06:31:18 IST 2024

$ docker run -e TZ="CET" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 07:31:29 CET 2024

$ docker run -e TZ="Europe/Berlin" --rm --entrypoint=sh debian:latest -c date
Fri Nov 29 07:31:50 CET 2024

Also, should we think about adding tzdata to conformance image here? 🤔 https://github.com/kubernetes/kubernetes/blob/master/test/conformance/image/Dockerfile

letmerecall avatar Nov 29 '24 08:11 letmerecall

How about we just make the hydrophone CLI log in UTC?

the conformance results should always be in UTC currently

/remove-lifecycle stale

BenTheElder avatar Dec 17 '24 17:12 BenTheElder

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Mar 17 '25 17:03 k8s-triage-robot

/remove-lifecycle stale

Given conformance is the original use case for SIG testing to sponsor this project, I really think we should just use UTC locally

It's also less fragile than depending on the conformance image having updated tzdata (it currently has no tzdata)

By default the conformance image is going to use the tzinfo from the host it's running on. Typically that's going to be UTC for most cloud providers (including KIND).

Actually tzdata isn't something mounted into a container typically, it's from the container image files, and we don't install tzdata into the e2e image (I also think we shouldn't, it will be out of date anyhow and conformance results are in UTC)

BenTheElder avatar Apr 02 '25 10:04 BenTheElder

@rjsadow thoughts on above?

farazkhawaja avatar Apr 02 '25 12:04 farazkhawaja

I agree, if we can have all the logs be UTC then that answers the ask.

rjsadow avatar May 13 '25 16:05 rjsadow

@BenTheElder can be closed.

farazkhawaja avatar May 21 '25 06:05 farazkhawaja

/close Thanks! 🙌

BenTheElder avatar May 21 '25 13:05 BenTheElder

@BenTheElder: Closing this issue.

In response to this:

/close Thanks! 🙌

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot avatar May 21 '25 13:05 k8s-ci-robot