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

Fix logging data context to file #32242

Merged
merged 1 commit into from
Jun 21, 2022
Merged

Fix logging data context to file #32242

merged 1 commit into from
Jun 21, 2022

Conversation

tcitworld
Copy link
Member

@tcitworld tcitworld commented May 2, 2022

It was only logged when an exception was provided or when using logData (which is not being much used btw).

We make sure only the parameters that weren't interpolated are logged.

Only tested with file write logger, but other writers shouldn't work differently.

Crash reporters always had the context.

@tcitworld tcitworld added bug 3. to review Waiting for reviews labels May 2, 2022
@tcitworld tcitworld requested review from CarlSchwan and removed request for a team May 2, 2022 15:54
@tcitworld tcitworld force-pushed the fix-logging-data-array branch from adef8fb to 67ad2cf Compare May 2, 2022 15:55
@tcitworld tcitworld added this to the Nextcloud 25 milestone May 2, 2022
@tcitworld tcitworld added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 2, 2022
@tcitworld tcitworld force-pushed the fix-logging-data-array branch from 67ad2cf to 613f608 Compare May 2, 2022 16:30
It was only logged when an exception was provided or when using
logData (which is not being much used).

We make sure the interpolated parameters are not logged.

Only tested with file write logger, but shouldn't work differently.

Crash reporters always had the context.

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@tcitworld tcitworld force-pushed the fix-logging-data-array branch from 613f608 to 1e7d924 Compare May 2, 2022 17:30
@tcitworld tcitworld added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 2, 2022
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

🙏

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 16, 2022
@blizzz blizzz merged commit cf5452e into master Jun 21, 2022
@blizzz blizzz deleted the fix-logging-data-array branch June 21, 2022 16:49
@tcitworld
Copy link
Member Author

Backports ?

@blizzz
Copy link
Member

blizzz commented Jun 22, 2022

imo it's rather an enhancement than a fix?

@tcitworld
Copy link
Member Author

It was the behaviour before the switch to PSR's LoggerInterface IIRC, far from being important anyway indeed.

@blizzz
Copy link
Member

blizzz commented Jun 22, 2022

It was the behaviour before the switch to PSR's LoggerInterface IIRC, far from being important anyway indeed.

didn't recall. Nothing older than yesterday's release 🤷 Well, then it is a regression indeed, and backporting would be legit.

@blizzz
Copy link
Member

blizzz commented Jun 22, 2022

/backport to stable24

@blizzz
Copy link
Member

blizzz commented Jun 22, 2022

/backport to stable23

@blizzz
Copy link
Member

blizzz commented Jun 22, 2022

/backport to stable22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants