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.

Sign up for GitHub

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

Jump to bottom

docs: Improve ORAS diagnose experience #1483

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

docs: Improve ORAS diagnose experience #1483

Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ba3dfc2
add spec: Improve ORAS diagnose experience
FeynmanZhou Aug 19, 2024
55e1d7f
refine spec
FeynmanZhou Aug 19, 2024
30e3e28
refine spec
FeynmanZhou Aug 19, 2024
7b823a5
update spec: Improve ORAS diagnose experience
FeynmanZhou Sep 3, 2024
f71aef8
update spec: Improve ORAS diagnose experience
FeynmanZhou Sep 4, 2024
65cc161
update spec based on comments
FeynmanZhou Sep 5, 2024
b6193f8
update spec per comments
FeynmanZhou Sep 9, 2024
d30b974
resolve comments and refactor the spec content
FeynmanZhou Sep 13, 2024
b1d7207
resolve comments
FeynmanZhou Sep 13, 2024
734dbbd
resolve new comments
FeynmanZhou Sep 16, 2024
4783369
resolve new comments
FeynmanZhou Sep 16, 2024
12bfc72
resolve recent comments
FeynmanZhou Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 13 additions & 20 deletions docs/proposals/diagnose-experience.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ At first, the output of ORAS flag `--verbose` and `--debug` should be clarified

There are three types of output in ORAS CLI:
Copy link

Choose a reason for hiding this comment

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

I think log is also one type of output. An "output” of a command refers to the information that is displayed or returned after the command is executed, it can be logs, formatted data, error messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Logs are logs, and logs may not be displayed or returned. They may be collected in systemd, or sent to fluentd.

FeynmanZhou marked this conversation as resolved.
Show resolved Hide resolved

- **Status output**: such as progress information, progress bar in pulling or pushing files
- **Metadata output**: showing what has been pulled (e.g. filename, digest, etc.) in specified format, such as JSON, text
- **Content output**: it is to output the raw data obtained from the remote registry server or file system, such as the pulled artifact content save as a file
- **Status output**: such as progress information, progress bar in pulling or pushing files.
- **Metadata output**: showing what has been pulled (e.g. filename, digest, etc.) in specified format, such as JSON, text.
- **Content output**: it is to output the raw data obtained from the remote registry server or file system, such as the pulled artifact content save as a file.
- **Error output**: error message are expected to be helpful to troubleshoot where the user has done something wrong and the program is guiding them in the right direction.


The target users of these types of output are normal users. Currently, the output of ORAS `--verbose` flag belongs to status output since it contains detailed progress status. The output of `--verbose` only exists in oras `pull/push/attach` commands.
The target users of these types of output are standard users. Currently, the output of ORAS `--verbose` flag only exists in oras `pull/push/attach/discover` commands, which prints out detailed status output and metadata output.
FeynmanZhou marked this conversation as resolved.
Show resolved Hide resolved

It is intended for end-users who want to observe the detailed file operation when using ORAS. It gives users a comprehensive view of what the tool is doing at every step and how long does it take when push or pull a file.

Expand All @@ -52,7 +52,7 @@ Logs focus on providing technical details for in-depth diagnosing and troublesho
## Proposals for ORAS CLI

- Deprecate the global flag `--verbose` and only remain `--debug` to avoid ambiguity. Based on the concept above, it is reasonable to continue using `--debug` to enable the output of `DEBUG` level logs in ORAS as it is in ORAS. Meanwhile, This change will make the diagnose experience much more straightforward and less breaking since only ORAS `pull/push/attach/discover` commands have verbose output.
- The existing output of `--verbose` in several ORAS commands `pull/push/attach/discover` can still be reserved. Introducing an additional flag to these commands.
- Introduce a new flag `--detail` to replace the existing global flag `--verbose` of commands like `pull`, `push`, `attach`, and `discover` for detailed output.
Copy link
Member

@Wwwsylvia Wwwsylvia Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just to discuss, how about --long?

Copy link

Choose a reason for hiding this comment

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

I don't think --detail is a good option, since it will cause similar confusion as --verbose. Questions like is it about detailed logs or about detailed other outputs?

One option is output the --verbose content by default, then we do not need any flags for verbose output that are not logs. This also solves the problem that some commands have --verbose flags, some don't.

Copy link

Choose a reason for hiding this comment

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

Will --debug be a global flag, or just available for some commands?

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of --detail, I think we can have --quiet for pull, push, and attach.

discover is different since the --verbose takes effect on the metadata output instead of the status output. In that case, we can just remove --verbose and introduce new values for --format such as --format tree-full. It is also a good time to revisit the outputs of oras discover.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree with @shizhMSFT . Current verbose output in pull, push, attach is progress information but in discover it is metadata output. They are different cases.

- Add separator lines between each request and response for readability.
- Add timestamp of each request and response to the beginning of each request and response.
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably just need to adjust the logrus implementation. /cc @Wwwsylvia for feasibility assessment.

Copy link
Member

Choose a reason for hiding this comment

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

Yes that is doable via the TextFormatter.FullTimestamp option.

- Add the response body including [error code](https://github.com/opencontainers/distribution-spec/blob/main/spec.md#error-codes) and the metadata of processed OCI object (e.g. image manifest) to the debug logs
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it mean? Is it applicable to OCI image layout?

Expand Down Expand Up @@ -96,13 +96,10 @@ Here are the guiding principles to write debug logs.
- Example: `DEBUG: Current registry URL: https://myregistry.io, Timeout setting:
FeynmanZhou marked this conversation as resolved.
Show resolved Hide resolved

### 6. **Avoid Logging Sensitive Information**
- **Privacy:** Abstain from logging sensitive information such as passwords, personal data, or security tokens.
- Example: `DEBUG: Attempting to authenticate user [UserID: usr123]` (without password details)

- **Compliance:** Ensure that logs adhere to relevant data protection and privacy regulations.
- Example: Anonymize or obfuscate sensitive customer data in logs.
- **Privacy and Security:** Abstain from logging sensitive information such as passwords, personal data, or security tokens.
- Example: `DEBUG: Attempting to authenticate user [UserID: usr123]` (exclude authentication token and password information).

## Investigation
## Investigation on other CLI tools

To make sure the ORAS diagnose functions are natural and easier to use, it worth knowing how diagnose functions work in other popular client tools.

Expand All @@ -118,10 +115,6 @@ Docker provides two options `--debug` and `--log-level` to control debug logs o

Helm CLI tool provides a global flag `--debug` to enable verbose output.

#### Kubectl

Kubectl has a command `kubectl logs` to show logs of objects such as Pod and container. No separate flags for verbose output and debug logs.

## Examples in ORAS

This section lists the current behaviors of ORAS debug logs, proposes the suggested changes to ORAS CLI commands. More examples will be appended below.
Expand All @@ -134,7 +127,7 @@ Pick the first two requests and responses as examples:
oras copy ghcr.io/oras-project/oras:v1.2.0 --to-oci-layout oras-dev:v1.2.0 --debug
```

**Current debug log0-=s**
**Current debug log**

```
[DEBU0000] Request #0
Expand Down Expand Up @@ -243,8 +236,8 @@ For example, the operating system and architecture are supposed to be outputted
$ oras version

ORAS Version: 1.2.0+Homebrew
Go version: go1.22.3
OS/Arch: Linux AMD64
Go version: go1.22.3
OS/Arch: linux/amd64
```

## Q & A
Expand All @@ -253,6 +246,6 @@ OS/Arch: Linux AMD64

**A:** Per our discussion in the ORAS community meeting, ORAS maintainers agreed to not introduce an additional environment variable as a global switch to enable debug logs since --debug is intuitive enough.

**Q2**: For the diagnose flag options, why deprecate `--verbose` and remain `--debug` as it is?
**Q2:**: For the diagnose flag options, why deprecate `--verbose` and remain `--debug` as it is?

**A**: The major reason is that this change avoids overloading the flag `--verbose` and reduce ambiguity in ORAS diagnose experience. Moreover, the `--debug` is consistent with other popular container client tools, such as Helm and Docker. Deprecation of `--verbose` is less breaking than changing behaviors of `--debug`.
Loading