Skip to content

json logs#105

Open
krezreb wants to merge 5 commits into5.x-devfrom
jsonlogs
Open

json logs#105
krezreb wants to merge 5 commits into5.x-devfrom
jsonlogs

Conversation

@krezreb
Copy link
Copy Markdown
Contributor

@krezreb krezreb commented Mar 25, 2025

Description

Complements matomo-org/matomo#23167
Updates the LogViewer view to correctly display logs when set to json format

Issue No

Complements matomo-org/matomo#23167

Steps to Replicate the Issue

  1. See Make it easier to use different log formats matomo#23167 (comment)

Checklist

  • [✔/✖] Tested locally or on demo2/demo3?
  • [✔/✖/NA] New test case added/updated?
  • [✔/✖/NA] Are all newly added texts included via translation?
  • [✔/✖/NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [✔/✖/NA] Version bumped?

Copy link
Copy Markdown
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

@krezreb I set log_format as log_format = "json" in my config.ini.php and I don't see the message as JSON, do I need to do anything else ? I am already on jsonlogs branch for core and plugin.

Screenshot from 2025-03-26 05-49-46

@krezreb
Copy link
Copy Markdown
Contributor Author

krezreb commented Mar 26, 2025

No that's how it's supposed to work, in the LogViewer it should display as it did before in non json format

@AltamashShaikh AltamashShaikh requested a review from snake14 March 27, 2025 00:31
Copy link
Copy Markdown
Contributor

@snake14 snake14 left a comment

Choose a reason for hiding this comment

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

Looks good. I just had one question.

Copy link
Copy Markdown
Contributor

@snake14 snake14 left a comment

Choose a reason for hiding this comment

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

Apologies @krezreb . I didn't realise that this hadn't been merged yet. @AltamashShaikh are you aware of any reason not to merge this PR?

Edit: I guess that the PR which it's dependent on hasn't been merged yet, so there isn't a rush.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants