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

Step level log #535

Open
xinnjie opened this issue Jul 20, 2023 · 9 comments
Open

Step level log #535

xinnjie opened this issue Jul 20, 2023 · 9 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@xinnjie
Copy link
Contributor

xinnjie commented Jul 20, 2023

Feature request

Currently logs of all steps in one Taskrun is mixed up, we should provide step level log, so that querying the log of one specific step becomes possible.

Use case

  • [Tekton Dashboard integrations] dashboard demand the function of querying log of one specific step.
  • [Performance enhancement] logs sometimes are quite large, if one queries log of one specific step, it's no need to query log of other steps.
@xinnjie xinnjie added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 20, 2023
@xinnjie
Copy link
Contributor Author

xinnjie commented Jul 20, 2023

One possible implementation:

one log record for one Taskrun(just same as current implementation), and change LogStatus like below, every step log store in different physical files

Model changes:

LogStatus
before:

// LogStatus defines the current status of the log resource.
type LogStatus struct {
	Path string `json:"path,omitempty"`
	Size int64  `json:"size"`
}

after:

// LogStatus defines the current status of the log resource.
type LogStatus struct {
	Path string `json:"path,omitempty"` //  Deprecated
	Size int64  `json:"size"`           //  Deprecated

	Steps []StepLogStatus `json:"steps"`
}

// StepLogStatus defines the current status of step log
type StepLogStatus struct {
	Name string `json:"name"` // Step name
	Path string `json:"path"`
	Size int64  `json:"size"`
}

API changes

before:

message GetLogRequest {
  // Name of the log resource to stream
  string name = 1 [
    (google.api.field_behavior) = REQUIRED,
    (google.api.resource_reference) = {
      type: "tekton.results.v1alpha2/Log"
    }];
}

after:

message GetLogRequest {
  // Name of the log resource to stream
  string name = 1 [
    (google.api.field_behavior) = REQUIRED,
    (google.api.resource_reference) = {
      type: "tekton.results.v1alpha2/Log"
    }];
  // Specify which step of log should be returned,  if not specified, return logs of all steps.
  string step = 2 [
    (google.api.field_behavior) = OPTIONAL
  ];
}

before:

// Log is a chunk of a log.
message Log {
  option (google.api.resource) = {
    type: "tekton.results.v1alpha2/Log"
  };

  // Resource name fo the log
  string name = 1 ;

  // The log data
  bytes data = 2;
}

after:

// Log is a chunk of a log.
message Log {
  option (google.api.resource) = {
    type: "tekton.results.v1alpha2/Log"
  };

  // Resource name fo the log
  string name = 1 ;

  // The log data
  bytes data = 2;
  
  // The step name which the log belongs to
  string step = 3; 
}

@xinnjie
Copy link
Contributor Author

xinnjie commented Jul 20, 2023

cc @adambkaplan @sayan-biswas

@sayan-biswas
Copy link
Contributor

sayan-biswas commented Jul 20, 2023

Wouldn't this create a metadata entry for each step in the database? This would be too granular and would require multiple database trip to build the complete log of the taskrun. IMO, if there's a approach which can create one metadata log for a result but the physical files will be stored in the steps level. Then we can provide feature in the API to request only a particular step.

@xinnjie
Copy link
Contributor Author

xinnjie commented Jul 20, 2023

Wouldn't this create a metadata entry for each step in the database?

No, one log record for one Taskrun. Current implementation is intuitive and good enough.

create one metadata log for a result but the physical files will be stored in the steps level

Yes , I think so too.

One possible implementation would be: one log record for one Taskrun(just same as current implementation), and change LogStatus like below, every step log store in different physical files.

// LogStatus defines the current status of the log resource.
type LogStatus struct {
	Path string `json:"path,omitempty"` //  Deprecated
	Size int64  `json:"size"`           //  Deprecated

	Steps []StepLogStatus `json:"steps"`
}

// StepLogStatus defines the current status of step log
type StepLogStatus struct {
	Name string `json:"name"` // Step name
	Path string `json:"path"`
	Size int64  `json:"size"`
}

@xinnjie
Copy link
Contributor Author

xinnjie commented Jul 20, 2023

Looks like we all agree on providing feature in the API to request only a particular step?
What left is API/Model changes and implementation detail.

@adambkaplan
Copy link
Contributor

Something to consider is that we will want to support delegated log forwarding (say, via fluentd). This was called out as a potential long-term feature in TEP-0117. I've filed #619 to make this more concrete.

How steps are represented may depend on the log forwarder deployed, and how logs are represented in the storage backend.

@cmorinupgrade
Copy link
Contributor

@adambkaplan was there any progress with regards to this issue? Thank you.

@sayan-biswas
Copy link
Contributor

@adambkaplan was there any progress with regards to this issue? Thank you.

Not yet. :-(
But we are still considering this feature and working on a design and maybe a POC soon.

@sayan-biswas
Copy link
Contributor

Before this we are trying to get tekton result use the v1 tekton APIs and an overall stability to the existing tekton results APIs, and specially the watcher component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants