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

Use monolog in tracker for logging #8598

Merged
merged 5 commits into from Aug 20, 2015

Conversation

Projects
None yet
3 participants
@tsteur
Member

tsteur commented Aug 19, 2015

refs #8593

configuration didn't change. It works as before: [Tracker]debug=1

@tsteur tsteur added this to the 2.15.0 milestone Aug 19, 2015

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 19, 2015

Contributor

StreamHandler could be used instead of writing a custom Monolog adapter (new StreamHandler('php://stdout'))?

Contributor

mnapoli commented Aug 19, 2015

StreamHandler could be used instead of writing a custom Monolog adapter (new StreamHandler('php://stdout'))?

tsteur added some commits Aug 19, 2015

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Aug 19, 2015

Member

I thought about using StreamHandler initially but then it's more work configuring it etc and the code of streamhandler looks complicated so rather went with the simple two liner

Member

tsteur commented Aug 19, 2015

I thought about using StreamHandler initially but then it's more work configuring it etc and the code of streamhandler looks complicated so rather went with the simple two liner

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 19, 2015

Contributor

That shouldn't be necessary if isTrackerDebugEnabled() is false then the logger is NullLogger so the log level doesn't really matter?

Contributor

mnapoli commented on plugins/Monolog/config/tracker.php in baec745 Aug 19, 2015

That shouldn't be necessary if isTrackerDebugEnabled() is false then the logger is NullLogger so the log level doesn't really matter?

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Aug 19, 2015

Member

Another plugin could enable it (set Monolog Logger) so want to make sure to not modify the log level unless specifically enabled

Member

tsteur replied Aug 19, 2015

Another plugin could enable it (set Monolog Logger) so want to make sure to not modify the log level unless specifically enabled

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 19, 2015

Contributor

OK, that's a big if but why not if you feel it's an issue.

Contributor

mnapoli replied Aug 19, 2015

OK, that's a big if but why not if you feel it's an issue.

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Aug 19, 2015

Member

We currently send header text/plain... which results in HTML not being parsed. Is it ok? I'd kinda prefer it like this to prevent any possible injections in case someone has debugging enabled. Can do a strip_tags or so

trackerlog

Member

tsteur commented Aug 19, 2015

We currently send header text/plain... which results in HTML not being parsed. Is it ok? I'd kinda prefer it like this to prevent any possible injections in case someone has debugging enabled. Can do a strip_tags or so

trackerlog

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Aug 19, 2015

Member

I'd kinda prefer it like this to prevent any possible injections in case someone has debugging enabled.

Security is the main reason the text/plain header was set 👍

Member

mattab commented Aug 19, 2015

I'd kinda prefer it like this to prevent any possible injections in case someone has debugging enabled.

Security is the main reason the text/plain header was set 👍

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Aug 19, 2015

Member

strip_tags then? I kinda do not really mind the html markup there. One can still copy/paste it and make it readable

Member

tsteur commented Aug 19, 2015

strip_tags then? I kinda do not really mind the html markup there. One can still copy/paste it and make it readable

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Aug 19, 2015

Member

I kinda do not really mind the html markup there. One can still copy/paste it and make it readable

👍

Member

mattab commented Aug 19, 2015

I kinda do not really mind the html markup there. One can still copy/paste it and make it readable

👍

mnapoli added a commit that referenced this pull request Aug 20, 2015

Merge pull request #8598 from piwik/8593
Use monolog in tracker for logging

@mnapoli mnapoli merged commit b2a6b20 into master Aug 20, 2015

2 of 3 checks passed

continuous-integration/travis-ci/push The Travis CI build failed
Details
Scrutinizer 2 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mnapoli mnapoli deleted the 8593 branch Aug 20, 2015

@mattab mattab added c: Platform and removed not-in-changelog labels Oct 13, 2015

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