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

Segregate System Logs From Datapath Logs #1497

Merged
merged 10 commits into from
Jul 21, 2022
Merged

Segregate System Logs From Datapath Logs #1497

merged 10 commits into from
Jul 21, 2022

Conversation

ihcsim
Copy link
Contributor

@ihcsim ihcsim commented Jun 21, 2022

Change Overview

This PR adds a new LogKind field to log lines generated by the KubeExec and KubeTask functions. This field will be assigned the value datapath to distinguish datapath logs from the controller's system logs.

This PR also update the documentation to include a reference architecture with instructions on setting up Loki and Grafana to visualize and segregate Kanister logs.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

Test Plan

Deploy this blueprint that uses flog to generate Apache log lines:

apiVersion: cr.kanister.io/v1alpha1
kind: Blueprint
metadata:
  name: stream-apache-logs
  namespace: kanister
actions:
  flogTask:
    phases:
    - func: KubeTask
      name: taskApacheLogs
      args:
        namespace: "{{ .Namespace.Name }}"
        image: mingrammer/flog:0.4.3
        command:
        - flog
        - -f
        - apache_combined
        - -n
        - "120"
        - -s
        - 0.5s
  flogExec:
    phases:
    - func: KubeExec
      name: execApacheLogs
      args:
        namespace: default
        pod: flog
        container: flog
        command:
        - flog
        - -f
        - apache_combined
        - -n
        - "120"
        - -s
        - 0.5s

Deploy this actionset to invoke the log streaming actions:

apiVersion: cr.kanister.io/v1alpha1
kind: ActionSet
metadata:
  generateName: stream-apache-logs-task-
  namespace: kanister
spec:
  actions:
  - name: flogTask
    blueprint: stream-apache-logs
    object:
      kind: Namespace
      name: default

The Kanister logs should show log lines with the LogKind fields:

{"Container":"flog","File":"pkg/log/log.go","Function":"github.com/kanisterio/kanister/pkg/log.PrintTo","Line":174,"LogKind":"datapath","Out":"201.239.177.66 - miller5735 [21/Jun/2022:04:42:34 +0000] \"PATCH /optimize/efficient HTTP/1.1\" 200 50635 \"https://www.chiefexpedite.org/cultivate/leverage\" \"Mozilla/5.0 (X11; Linux i686) AppleWebKit/5311 (KHTML, like Gecko) Chrome/40.0.859.0 Mobile Safari/5311\"","Pod":"flog","cluster_name":"49dabaf0-1d27-4684-9527-f871ca789ffc","hostname":"kanister-kanister-operator-85c747bfb8-blp4w","level":"info","msg":"action update","time":"2022-06-21T04:41:36.58252109Z"}
{"Container":"flog","File":"pkg/log/log.go","Function":"github.com/kanisterio/kanister/pkg/log.PrintTo","Line":174,"LogKind":"datapath","Out":"241.51.55.81 - trantow6825 [21/Jun/2022:04:42:35 +0000] \"GET /deliver/e-enable HTTP/1.1\" 404 74803 \"http://www.internationalsexy.info/iterate/e-commerce/applications/interfaces\" \"Mozilla/5.0 (Windows 95) AppleWebKit/5361 (KHTML, like Gecko) Chrome/40.0.861.0 Mobile Safari/5361\"","Pod":"flog","cluster_name":"49dabaf0-1d27-4684-9527-f871ca789ffc","hostname":"kanister-kanister-operator-85c747bfb8-blp4w","level":"info","msg":"action update","time":"2022-06-21T04:41:36.582540669Z"}
{"Container":"flog","File":"pkg/log/log.go","Function":"github.com/kanisterio/kanister/pkg/log.PrintTo","Line":174,"LogKind":"datapath","Out":"93.104.102.21 - - [21/Jun/2022:04:42:35 +0000] \"POST /systems/sticky HTTP/2.0\" 403 14528 \"http://www.corporatee-markets.info/empower/architectures\" \"Opera/10.63 (Macintosh; PPC Mac OS X 10_6_1; en-US) Presto/2.8.255 Version/13.00\"","Pod":"flog","cluster_name":"49dabaf0-1d27-4684-9527-f871ca789ffc","hostname":"kanister-kanister-operator-85c747bfb8-blp4w","level":"info","msg":"action update","time":"2022-06-21T04:41:36.582559472Z"}
{"Container":"flog","File":"pkg/log/log.go","Function":"github.com/kanisterio/kanister/pkg/log.PrintTo","Line":174,"LogKind":"datapath","Out":"234.40.107.229 - raynor5704 [21/Jun/2022:04:42:36 +0000] \"DELETE /visionary/holistic HTTP/2.0\" 301 24584 \"https://www.dynamicnext-generation.com/deploy/bleeding-edge\" \"Mozilla/5.0 (X11; Linux i686) AppleWebKit/5360 (KHTML, like Gecko) Chrome/38.0.847.0 Mobile Safari/5360\"","Pod":"flog","cluster_name":"49dabaf0-1d27-4684-9527-f871ca789ffc","hostname":"kanister-kanister-operator-85c747bfb8-blp4w","level":"info","msg":"action update","time":"2022-06-21T04:41:36.582578359Z"}
  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@github-actions
Copy link
Contributor

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

events like interactions with the Kubernetes APIs, CRUD operations on
blueprints and actionsets etc.

Datapath logs, on the other hand, are logs emitted by data mover task pods
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if we can call them data mover task pods. In a lot of scenarios, the pods actually doesn't move data but just runs some command. Example is KubeExec function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do we have some other options than data path logs? for example functionPodLogs or kanFunPodLogs etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to leak implementation concepts like function, kanFunc etc. to the logs. Fundamentally, I want to convey high-level concepts like "system vs. user", "control plane vs data plane" etc. Other than operations like scale up and down, even most KubeExec and KubeOps calls made by the users involve some kind of data operations, right?

But yes, maybe we can replace "data mover task pods" with just "task pods"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am more inclined towards changing the value for const LogKindDatapath and the reason is, when users see the logs with this extra field they should be able to figure out what are these logs.
I think, dataPath is not intuitive enough to tell that these logs are from the commands that are there in the kanister functions. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What kind of information would you like to convey to the user in the logs? Will using the ActionSet and LogKind fields together provide the level of granularity you are referring to? Adding more fields to the logs may not necessary make things more intuitive as they tend to clutter the log outputs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, I was not requesting to add more fields. I was asking to change the value datapath to something else, if possible. Because, according to me, datapath is not able to convey which logs are these exactly.
If possible, we should be able to convey that, these logs are output of the commands that you (users) have mentioned in the blueprint.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we not want to do anything about it? @pavannd1 @ihcsim
If not, PR looks good to me.

docs/tasks/logs.rst Outdated Show resolved Hide resolved
docs/tasks/logs.rst Outdated Show resolved Hide resolved
docs/tasks/logs.rst Outdated Show resolved Hide resolved
pkg/consts/consts.go Outdated Show resolved Hide resolved
docs/tasks/logs.rst Show resolved Hide resolved
docs/tasks/logs.rst Outdated Show resolved Hide resolved
docs/tasks/logs.rst Outdated Show resolved Hide resolved
docs/tasks/logs.rst Outdated Show resolved Hide resolved
ihcsim added 10 commits July 21, 2022 09:51
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Kanister automation moved this from In Progress to Reviewer approved Jul 21, 2022
@ihcsim ihcsim added the kueue label Jul 21, 2022
@mergify mergify bot merged commit f815219 into master Jul 21, 2022
Kanister automation moved this from Reviewer approved to Done Jul 21, 2022
@mergify mergify bot deleted the loki-intg branch July 21, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants