Skip to content

ConsoleMsg is not set by open source or AIStor, we should remove this an… #5186

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

Merged
merged 1 commit into from
Apr 8, 2025

Conversation

marktheunissen
Copy link
Contributor

@marktheunissen marktheunissen commented Apr 8, 2025

…d correctly show Message only

Community Contribution License

All community contributions in this pull request are licensed to the project maintainers
under the terms of the Apache 2 license.
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.

Description

mc admin logs is missing messages, which are being sent in the json payload but not being printed

Motivation and Context

This results in the following:

~/src ❯ mc admin logs local
 API: SYSTEM.healthcheck()
 Time: 14:03:19 UTC 04/08/2025
 DeploymentID: f382210e-331f-4082-9d99-abe81a9d32a4

~/src ❯ mc admin logs local --json
{
 "status": "success",
 "deploymentid": "f382210e-331f-4082-9d99-abe81a9d32a4",
 "level": "EVENT",
 "errKind": "",
 "time": "2025-04-08T14:03:19.688893Z",
 "api": {
  "name": "SYSTEM.healthcheck",
  "args": {}
 },
 "message": "This is my message",
 "ConsoleMsg": "",
 "node": ""
}

Notice that the "message" field in the JSON contains the message, and the "ConsoleMsg" field is empty. MinIO and AIStor do not set this field. However, mc is still looking for it. The first command mc admin logs local showed lines for API, Time and Deployment but not the actual message.

After this PR, it shows this:

❯ ./mc admin logs local
 API: SYSTEM.healthcheck()
 Time: 14:03:19 UTC 04/08/2025
 DeploymentID: f382210e-331f-4082-9d99-abe81a9d32a4
 Message: This is my message

How to test this PR?

mc admin logs

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

cmd/admin-logs.go:107

  • Since the 'ConsoleMsg' field is no longer used, consider removing its associated data or struct field to simplify the codebase.
log := l.LogInfo

cmd/admin-logs.go:136

  • Consider adding unit tests to verify that the 'Message' field is printed correctly in various log scenarios.
if l.Message != "" {

@harshavardhana harshavardhana merged commit e929f89 into minio:master Apr 8, 2025
5 checks passed
@marktheunissen marktheunissen deleted the noconsolemsg branch April 9, 2025 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants