Dump Arrays, Objects and Exceptions to the log #12682

oparoz · 2014-12-08T03:18:24Z

By using a 3rd party normalizer, it's possible to dump objects and arrays to the log, without having to pre-encode them.

The library used in this PR is : https://github.com/interfasys/lognormalizer

You add the variables to the context array like this:

$this->logger->debug("Here is what's inside this FileInfo object: {fileinfo}", array(
    'app' => 'myApp',
    'fileinfo' => $fileInfo,
));

or

try {
    throw new \Exception("I want to be logged as an Exception!");
} catch (\Exception $e) {
    $this->logger->debug("I'm logging the full Exception object here: {exception}", array(
        'app' => 'myApp',
        'exception' => $e,
    ));
}

Note: It has only been tested with the ownlcloud logger (log_type).

DeepDiver1975 · 2014-12-08T06:54:13Z

Php 5.4 is now minimum requirement for oc8. You can remove this special handling.

DeepDiver1975 · 2014-12-08T06:54:52Z

Unit tests are welcome

oparoz · 2014-12-08T13:51:41Z

@DeepDiver1975 - I'll add the unit tests if this extension of the logger is wanted by most and if it works with other backends

MorrisJobke · 2014-12-09T09:05:56Z

I like this :)

DeepDiver1975 · 2014-12-09T09:53:19Z

lib/private/log.php

+		return $this->toJson($data, true);
+	}
+
+	private function toJson($data, $ignoreErrors = false) {


please kill $ignoreErrors - it is always called with true and I see no reason to do this different - THX

@oparoz please kill that unused argument- THX

I've moved everything into a separate class. I "just" need to add it to this PR
https://github.com/interfasys/galleryplus/blob/master/utility/normalizer.php

Looks good - I'm happy to accept this class (unit tests for sure required 🙊 ) - many thanks!

DeepDiver1975 · 2014-12-09T09:54:04Z

@DeepDiver1975 - I'll add the unit tests if this extension of the logger is wanted by most and if it works with other backends

yes - I'd like to see this getting part of ownCloud core - THX a lot! Please add unit tests 🙈

MorrisJobke · 2014-12-19T10:05:25Z

@oparoz DId you need any help?

oparoz · 2014-12-19T10:09:12Z

I just need to find the time to add the unit tests, to this PR and to many others... But if someone else wants to write them for this one, then no problem.

MorrisJobke · 2014-12-19T10:22:47Z

@PVince81 @LukasReschke Maybe one of you have time today to do this, to review this today ;)

DeepDiver1975 · 2014-12-22T13:37:10Z

moving to OC8.1

nickvergessen · 2015-03-18T11:17:05Z

lib/private/log.php

+			$count = 1;
+			foreach ($data as $key => $value) {
+				if ($count++ >= 1000) {
+					$normalized['...'] = 'Over 1000 items, aborting normalization';


Should do that in depth aswell? Also I'd stop at 100 or seomthing lower... no one looks at more entries anyway

Check the Normalizer class https://github.com/interfasys/galleryplus/blob/master/utility/normalizer.php#L99
I've added a variable for that. Ideally it should come from the config, if there really is a need to play with it.
I did change it at times when looking at some objects.

Well the normalizer is not in this PR
Config: 🙅 👎

You can make it a class member of this class, that's fine. But nothing more really

I was thinking of adding a separate class to keep this one at a decent size, but it probably won't work because it needs to be instantiated, which means changing the ctor's sig

th3fallen · 2015-04-09T19:06:06Z

question instead of making this ourselves (great job btw) would it be easier/better in the long run to just add the option to use monolog instead of the oc logger. I have a working POC of that i did on one of my exploratory days. To enable it you just set 'log_type' => 'monolog' in your config.

Let me know and i'll create a PR if its wanted. @DeepDiver1975 @oparoz

oparoz · 2015-04-11T17:31:42Z

@th3fallen - There have been several requests to add Monolog (#12671, #10405 which is actually yours :)), but they all have been turned down, so I decided to import and "improve" the normalizer.
I don't think it would require much effort to maintain that class since I don't expect the number of variable types in PHP to change frantically over the years.

nickvergessen · 2015-04-13T08:51:50Z

needs to be rebased

oparoz · 2015-04-13T12:22:49Z

OK, rebased, using the new Normalizer class.
Let me know what needs to be adjusted or if it breaks anything.

nickvergessen · 2015-04-13T13:12:15Z

lib/private/normalizer.php

+					'Over ' . $maxArrayRecursion . ' items, aborting normalization';
+				break;
+			}
+			$normalized[$key] = $this->normalize($value, $depth);


depth is not increased?

$depth is only used by normalizeObject to prevent ending up with a huge log entry with details about every "sub-objects".
Arrays have their own limits.

I've tried to have sensible limits, based on my needs when investigating issues, but there might be better ways to do this.
I've run into the array's limit before for things like iConfig objects, but at the same time, it's annoying to get 50 long strings into one log entry when feeding the log with results per example.

im not talking about:

[ 1, 2, ..., ];

I'm talking about:

[ [ [ [ ..., ], ], ], ];

You're correct, array depth is not limited.

oparoz · 2015-04-27T18:19:10Z

This won't work without adding interfasys/lognormalizer to the list of 3rd party dependencies, so all tests will fail until that happens.

ghost · 2015-04-30T21:26:10Z

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/12067/
💣 Test FAILed. 💣

PVince81 · 2015-07-03T11:56:06Z

Send a PR to https://github.com/owncloud/3rdparty to include the required dependency ?

oparoz · 2015-07-03T14:20:30Z

I didn't do it for 2 reasons:

There is an ongoing discussion about moving away from using that 3rdparty folder
Adding the dependency makes it permanent, until removed if this PR is rejected and so I was wondering what the process was to sync PRs

So, should I just add it to 3rdparty so that people can play with the lib?

MorrisJobke · 2015-09-01T09:11:19Z

lib/private/log.php

@@ -27,6 +28,8 @@

 namespace OC;

+use InterfaSys\LogNormalizer\Normalizer;


This is not defined. What does it do?

It's available here and needs to be added to the list of external dependencies in 3rdparty
https://github.com/interfasys/lognormalizer

MorrisJobke · 2015-09-01T09:12:00Z

@oparoz I don't think this will make it into 8.2 -> defer to 9.0

oparoz · 2015-09-01T12:22:19Z

Feature freeze is in 3 weeks, so I'll try to create a PR in the 3rdparty repo. I didn't have much luck getting it to work locally last time I tried. No problems in apps. I can load the lib without any issues using composer.

MorrisJobke · 2015-09-01T15:27:44Z

Feature freeze is in 3 weeks,

2 weeks ;) https://github.com/owncloud/core/wiki/Maintenance-and-Release-Schedule#owncloud-82

oparoz · 2015-09-01T20:06:57Z

Ah, so that paragraph on the same page needs to be updated or is this an exception?

The feature freeze of each version will be one month before the release dates.

MorrisJobke · 2015-09-02T05:43:06Z

Ah, so that paragraph on the same page needs to be updated or is this an exception?

This is a rough description of when something will happens. The excact dates are then determined by @cmonteroluque and announced. This is what I have written to the wiki then. So thread the exact dates as the correct ones and the text as a long term guideline.

scrutinizer-notifier · 2015-09-14T12:47:52Z

A new inspection was created.

oparoz · 2015-09-14T13:17:02Z

You need to switch your 3rdparty branch to this PR: owncloud-archive/3rdparty#195

Then you can dump any array, object, exception to owncloud.log, as described in the OP.

@DeepDiver1975 @th3fallen @MorrisJobke @LukasReschke @PVince81 @rullzer @Xenopathic

MorrisJobke · 2015-09-24T16:27:49Z

Superseeded by core branch PR in #19030

MorrisJobke · 2015-09-28T06:04:35Z

@oparoz Can I ask you to document this in the developers manual too? Thanks https://doc.owncloud.org/server/8.2/developer_manual/app/logging.html

oparoz · 2015-09-28T13:21:43Z

@MorrisJobke Added an issue in order to not forget.

oparoz mentioned this pull request Dec 8, 2014

Use a smarter logger #12671

Closed

MorrisJobke added the 3 - To Review label Dec 8, 2014

DeepDiver1975 reviewed Dec 9, 2014
View reviewed changes

MorrisJobke added 2 - Developing and removed 3 - To Review labels Dec 9, 2014

MorrisJobke added this to the 8.0-current milestone Dec 9, 2014

DeepDiver1975 modified the milestones: 8.1-next, 8.0-current Dec 22, 2014

nickvergessen reviewed Mar 18, 2015
View reviewed changes

DeepDiver1975 modified the milestones: 8.2-next, 8.1-current Apr 2, 2015

oparoz self-assigned this Apr 13, 2015

nickvergessen reviewed Apr 13, 2015
View reviewed changes

MorrisJobke reviewed Sep 1, 2015
View reviewed changes

MorrisJobke modified the milestones: 9.0-next, 8.2-current Sep 1, 2015

Use a 3rd party normalizer for the logger

70ebdd3

oparoz mentioned this pull request Sep 14, 2015

Add a variable normalizer for logs owncloud-archive/3rdparty#195

Merged

oparoz modified the milestones: 8.2-current, 9.0-next Sep 14, 2015

oparoz added 3 - To Review and removed 2 - Developing labels Sep 14, 2015

DeepDiver1975 mentioned this pull request Sep 14, 2015

[jenkins] Dump Arrays, Objects and Exceptions to the log #19030

Merged

1 task

MorrisJobke closed this Sep 24, 2015

oparoz mentioned this pull request Sep 28, 2015

New way to log objects and arrays needs to be documented owncloud-archive/documentation#1706

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019

		@@ -27,6 +28,8 @@

		namespace OC;

		use InterfaSys\LogNormalizer\Normalizer;

Dump Arrays, Objects and Exceptions to the log #12682

Dump Arrays, Objects and Exceptions to the log #12682

Conversation

oparoz commented Dec 8, 2014

DeepDiver1975 commented Dec 8, 2014

DeepDiver1975 commented Dec 8, 2014

oparoz commented Dec 8, 2014

MorrisJobke commented Dec 9, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DeepDiver1975 commented Dec 9, 2014

MorrisJobke commented Dec 19, 2014

oparoz commented Dec 19, 2014

MorrisJobke commented Dec 19, 2014

DeepDiver1975 commented Dec 22, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

th3fallen commented Apr 9, 2015

oparoz commented Apr 11, 2015

nickvergessen commented Apr 13, 2015

oparoz commented Apr 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oparoz commented Apr 27, 2015

ghost commented Apr 30, 2015

PVince81 commented Jul 3, 2015

oparoz commented Jul 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MorrisJobke commented Sep 1, 2015

oparoz commented Sep 1, 2015

MorrisJobke commented Sep 1, 2015

oparoz commented Sep 1, 2015

MorrisJobke commented Sep 2, 2015

scrutinizer-notifier commented Sep 14, 2015

oparoz commented Sep 14, 2015

MorrisJobke commented Sep 24, 2015

MorrisJobke commented Sep 28, 2015

oparoz commented Sep 28, 2015