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

fix(app): #1536 fix logging format to work with pino-pretty #1593

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

fix(app): #1536 fix logging format to work with pino-pretty #1593

wants to merge 4 commits into from
+36 −1

Conversation

mohit-nagaraj
Copy link

@mohit-nagaraj mohit-nagaraj commented Oct 17, 2024 •
edited
Loading

Summary

This PR introduces a new --output-json flag for the RunAppsGetLogs function in the DigitalOcean CLI (doctl). When this flag is used, the logs are outputted in JSON format without the service name and timestamp prefix.

Changes Made

  • Added --output-json flag to the RunAppsGetLogs function.
  • Modified the log processing logic to remove the service name and timestamp prefix when the --output-json flag is used.

Closes #1536

andrewsomething
andrewsomething requested changes Oct 17, 2024
View reviewed changes
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Thanks for this great contribution @mohit-nagaraj! I do have some feedback:

For your new flag to work as expected, it needs to be registered with the command. You can see how that was done with other flags for the logs command here:

doctl/commands/apps.go

Lines 167 to 188 in 7f7a3c0

logs := CmdBuilder(
cmd,
RunAppsGetLogs,
"logs <app id> <component name (defaults to all components)>",
"Retrieves logs",
`Retrieves component logs for a deployment of an app.
Three types of logs are supported and can be specified with the --`+doctl.ArgAppLogType+` flag:
- build
- deploy
- run
- run_restarted
For more information about logs, see [How to View Logs](https://www.digitalocean.com/docs/app-platform/how-to/view-logs/).
`,
Writer,
aliasOpt("l"),
)
AddStringFlag(logs, doctl.ArgAppDeployment, "", "", "Retrieves logs for a specific deployment ID. Defaults to current deployment.")
AddStringFlag(logs, doctl.ArgAppLogType, "", strings.ToLower(string(godo.AppLogTypeRun)), "Retrieves logs for a specific log type. Defaults to run logs.")
AddBoolFlag(logs, doctl.ArgAppLogFollow, "f", false, "Returns logs as they are emitted by the app.")
AddIntFlag(logs, doctl.ArgAppLogTail, "", -1, "Specifies the number of lines to show from the end of the log.")

That is also where you add the information that shows in the help output.

I also think output-json might not be the best name for this flag. The output is only in JSON if your application logs in JSON. If you log in plain text, using the output-json would still produce plain text. Maybe something like no-prefix as it removes the prefixes from the logs? We could provide a hint in the help text that it useful for JSON structured logs.

@mohit-nagaraj
Copy link
Author

Sure got it fixing them

@mohit-nagaraj
Copy link
Author

Hey I updated code as specified, Could you re-review them will implement any of the changes you say

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewsomething andrewsomething Awaiting requested review from andrewsomething

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

doctl apps logs doesn't output JSON
3 participants
@mohit-nagaraj @andrewsomething @loosla