Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

Change the way the gardenctl displays logs from loki #509

Merged
merged 1 commit into from
Apr 6, 2021

Conversation

Kristian-ZH
Copy link
Contributor

@Kristian-ZH Kristian-ZH commented Mar 24, 2021

What this PR does / why we need it:

  1. Change the default value of tail flag from 200 to 1000, because if someone does not know about this flag, he will think that there is an issue with gardenctl logs/loki because very few logs are displayed

  2. Change the way the gardenctl displays logs from loki.
    Now the logs will be visualised in the following way:

=============================================================================================
Pod Name kube-controller-manager-fsdfsd-fsdfs,  Container Name: kube-controller-manager, DockerID: dbcb643a5dfbbd12e8dbf50e497dab8
=============================================================================================
// logs
// logs
// logs
=============================================================================================
Pod Name kube-controller-manager-fsdfsd-fsdfs, Container Name: kube-controller-manager, DockerID: fsdfsdgsfsgsfsfsfs
=============================================================================================
//logs
//logs
//logs
=============================================================================================
Pod Name side-car-fsdfsd-fsdfs, Container Name: kube-controller-side-car-container, DockerID: 31241321413132
=============================================================================================
// logs
// logs
// logs

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

New format of the `gardenctl logs --loki` command

@Kristian-ZH Kristian-ZH requested a review from a team as a code owner March 24, 2021 15:39
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Mar 24, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 24, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 24, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 24, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 24, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 25, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 25, 2021
Copy link
Contributor

@tedteng tedteng left a comment

Choose a reason for hiding this comment

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

It looks good.
just one comment, I am not clear the docker id in there. Is that represent history Pod? eg. api-server has been restarted, the DockerID to distinguish previous Pod and current live Pod? if Yes, is it possible to display the Pod name to identify it as well ?

============================================================================================
Container Name: kube-controller-manager, DockerID: dbcb643a5dfbbd12e8dbf50e497dab8
=============================================================================================
// logs
// logs
// logs
=============================================================================================
Container Name: kube-controller-manager, DockerID: fsdfsdgsfsgsfsfsfs
=============================================================================================```

@Kristian-ZH
Copy link
Contributor Author

Kristian-ZH commented Mar 26, 2021

Is that represent history Pod? eg. api-server has been restarted, the DockerID to distinguish previous Pod and current live Pod?

Yes correct. In each container there is a running docker process. DockerID represents this process. If the DockerID is changed, it means that the the container has been restarted.

is it possible to display the Pod name to identify it as well ?

Yes, it is possible will do this today

@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 26, 2021
@Kristian-ZH
Copy link
Contributor Author

did the changes

@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 26, 2021
@Kristian-ZH
Copy link
Contributor Author

Kristian-ZH commented Mar 26, 2021

Аlso to emphasise that the current gardenctl logs --loki command does not work correctly. It sorts logs by stream, not by container as this PR does.
Each stream includes Severity label and this means that all logs with severity=Error will be displayed on one place, with severity=Info on another and so on.

Copy link
Contributor

@tedteng tedteng left a comment

Choose a reason for hiding this comment

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

lgtm

@neo-liang-sap
Copy link
Contributor

neo-liang-sap commented Apr 6, 2021

Per discussion with @Kristian-ZH i will merge this PR and trigger a minor release
BTW personally i didn't perform any test on this

@neo-liang-sap neo-liang-sap merged commit 7b10ff9 into gardener-attic:master Apr 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants