I've created this issue to be able to patch a D8 release of JSONlog into the 8.x branch of this module.

Comments

lorenzs created an issue. See original summary.

lorenzs’s picture

StatusFileSize
new72.91 KB

Patch attached for an initial dev release of JSONlog 8.x - to be pushed into 8.x branch.
This dev version does not include the jsonlog_test_filing() option via drush anymore. However you can do a simplified test via the admin screen.

The D8 version has been tested on our development server (PHP 7).

lorenzs’s picture

Assigned: lorenzs » Unassigned
Status: Active » Patch (to be ported)
lorenzs’s picture

Status: Patch (to be ported) » Needs work
jacobfriis’s picture

Assigned: Unassigned » jacobfriis

  • jacobfriis committed 6c02e4e on 8.x-1.x
    Issue #2871691 by lorenzs: Drupal 8 version, initial and defective, much...

  • jacobfriis committed 3742cec on 8.x-1.x
    Revert "Issue #2871691 by lorenzs: Drupal 8 version, initial and...

  • jacobfriis committed 10c1f1f on 8.x-1.x authored by lorenzs
    Issue #2871691 by lorenzs: Drupal 8 version, initial and defective, much...
jacobfriis’s picture

Assigned: jacobfriis » Unassigned

@lorenzs Truly great job you've done here :-)
However, there are several things to be done prior to a release. This is what I stumbled upon - there will turn up more on the way ;-)

Todos - major

  1. The module doesn't log at all, except when tested directly via it's admin form - apparantly it doesn't succeed in implementing the D8 equivalent of hook_watchdog.
  2. Establishing default site ID doesn't work; the function jsonlog_default_site_id() should fetch the name of the default database via D8 means (not via D7'ish $GLOBALS['databases'], which btw is void in the context).

Todos - minor

  1. OOP - the .module file should only contain stuff that absolutely cannot be placed in PHP classes (for instance the settings form should probably extend or negotiate with D8 ConfigFormBase).
  2. Coding style - lots of (allright minor) flaws, which an appropriate IDE can fix in whim.
  3. module_load_include has been replaced by something else ... loadInclude().
lorenzs’s picture

StatusFileSize
new4.19 KB

Hi Jacob;

Todos - major

  1. I did add a default timestamp format now but except for that, when I enable the module and perform cron or a 404 (without even saving the config form) it immediately starts logging to the default dir/jsonlog file. The only issue I encountered was a file permission issue in case the dir/file is not writable. Can you explain a bit more on this (we do use the module already for testing purposes). The implemented LoggerInterface should pick up all messages logged via @logger.factory.
  2. Overlooked the default_site_id being broken idd - fixed now fetching the database config.

Todos - minor

  1. concerning the form alter & submit, for now I followed core's syslog module. As long as the form_alter is not deprecated I don't think it is a priority for now. Is there already an 'official' way for overriding core forms?
  2. code styling is now my PHPStorm 's formatting (set as Drupal code style), might have missed some options... If really a lot of stuff; feel free to reformat or share you're (source of) settings.
  3. I replaced the module_load_include in the Logger itself via DI; left it for now in the module file as it might be refactored, see 1.

See patch attached with above changes.

lorenzs’s picture

StatusFileSize
new4.19 KB

Typo in services.yml - updated patch to #11

jacobfriis’s picture

Assigned: Unassigned » jacobfriis
jacobfriis’s picture

Oops. No wonder it didn't log at my end.

Forgot about severity threshold (default: warning) - I was attempting to log debug messages.

And (more interesting) wasn't aware that the equivalent of D7 watchdog(...)
D8 \Drupal::logger($type)->log(...)
only logs to a single channel - Log API:

D8 - injecting a specific channel
Drupal core registers only one channel in core.services.yml for "system" channel.

dblog was enabled too, at my end.

  • jacobfriis committed 44a6438 on 8.x-1.x authored by lorenzs
    Issue #2871691 by lorenzs: - Set default timestamp.
    - Fixed getting...
jacobfriis’s picture

Assigned: jacobfriis » Unassigned

@lorenzs Great job.

Todos - major (revisited)

  1. Yep - it does log.
  2. Swell.

Todos - minor (revisited)

  1. Yes, I can see that some core modules actually use old-school functional PHP for admin forms. So lets not spend time on that - at least until it gets deprecated or unsupported.
  2. Surely a minor issue, didn't stumble upon anything today, except some undocumented return types. I use the same IDE ;-)
  3. Right.

Added CHANGELOG.txt

Great as easily accessible tracking. And useful when creating release(note)s.

lorenzs’s picture

Assigned: Unassigned » lorenzs
lorenzs’s picture

Version: 7.x-2.1 » 8.x-1.0-beta1
Assigned: lorenzs » Unassigned
Status: Needs work » Needs review

Beta release for D8 is now available for testing.
https://www.drupal.org/project/jsonlog/releases/8.x-1.0-beta1

  • lorenzs committed a6447d6 on 8.x-1.x
    Issue #2871691 by lorenzs: update release_notes for release.
    
lorenzs’s picture

Version: 8.x-1.0-beta1 » 8.x-1.0
Status: Needs review » Fixed

8.x-1.0 released.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.