Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Eronarn’s picture

1. I would prefer to keep the _early and _late module names as these modules do not strictly segregate layer entries and exits - they each register both.

Yeah, good point - I changed this because there had been confusion, but I suppose it will cause other kinds of confusion. Is there any alternative here? Would rather figure out what it should be rather than backing it out only to change it again later.

2. Proposed changes to default configuration need to be submitted as a separate issue rather than as part of the D8 port so that they can be readily back-ported to the 7.x branch where applicable.

Easy enough to do this for the configuration values - patch attached. However, I suggest keeping the changes to the key names - config YAMLs should be semantic/grouped in Drupal 8, and some of the keys really do mean different things too (for example, you will get layers without enabling the hook tracking, so 'enable layers' doesn't make sense now).

3. Branding changes need to be submitted as a separate issue, same reason as above.

I assume that (apart from the early/late vs. entry/exit) you mean just the help message/field description changes? That's fine - looks like a straightforward revert of 60d6054, and then part of the changes in e21b76, b37711, and possibly 9a7bba. Do you want me to rebase those commits or just make a patch with the equivalent changes for Drupal 7?

pdrake’s picture

The early and late modules could be renamed to something else entirely, I suppose. Where has the confusion been? I haven't seen anything in the issue queue regarding the names.

I have no problem changing some config names for D8 where it make sense. I just want to have general recommendations regarding changes to the default values for settings to be a separate issue so we can track it through the various versions.

If you could remove any branding changes from the D8 port patch and make them a separate issue with a branding changes only patch, that would be great.

Eronarn’s picture

Issue summary: View changes
FileSize
28.27 KB

Attached a zip with a reworked branch (23 commits!) that works on D8 master as of a few days ago, but is based on changes made up to 7.x-1.7 of the module, and doesn't include any branding/copy/help updates. The internal variable and form names are still different, again because of the semantic nature of D8 config, though I haven't adjusted how the form looks apart from removing drupal_http_request (which is deprecated).

I feel pretty comfortable having people start trying this out! It would be great to get this up as an official alpha.

Works:
Modifying configuration
Real user monitoring
Role partitioning
Kernel event tracking
Twig template tracking
Controller/action (though D8 gives bad results here because it is overly complicated)

Doesn't work:
Early/late modules (most hooks have been removed anyways - might be worth cutting these!)
Drush (same issue that affects 7.x-1.7, should fix it there first)
Watchdog 403/404 (these are exceptions now, so we should remove this option and improve the exception handling instead)

Untested:
Anything to do with the API
Remote service call tracking

Eronarn’s picture

Status: Needs work » Needs review
FileSize
13.4 KB

Okay, so that's pretty embarrassing... I somehow lost the commit that fixed traceview.install! Crashing on install probably made that last patch set pretty hard to review, huh?

I've attached a zip with an additional zip of 12 patches that fixes that problem, as well as brings the module in line with 8.0-alpha12:

Fix drush support
Remove early/late modules because they no longer provided value
Remove watchdog code because it's no longer needed and watchdog is deprecated
Remove hook, profile, view, and watchdog configuration options

Eronarn’s picture

Two more patches: had a broken conditional that also happened to be illegal in PHP<5.5, and needed to rename the module's service provider (Traceview rather than TraceView) because camelization reasons.

Strayer’s picture

D8 is out, look forward to first D8 beta - willing to help test, let me know if help needed.

dkuebric’s picture

FileSize
16.68 KB

Here's the latest release of the D8 instrumentation--apologies for the latency on the thread!

We're actively developing automated support for D8 monitoring. Consequently, feedback about the data collected (and anything not collected you'd like to see) would be great.