[RFC] LiipMonitor integration

[Thinkscape]

This is a place for discussion which started in #30.

The idea is to provide some level of integration with LiipMonitor

[Thinkscape]

@lsmith77, what kind of integration would you like to see?

• ZFTool consuming LiipMonitor checks?
• LiipMonitor consuming ZFTool\Diagnostics checks ?
• Some form of cross-configuration ?
• Something else?

[Thinkscape]

Also please take a moment to skim through the tutorial of adding checks - it shows how they are defined (in several ways), interpreted and then invoked.

The only interface for tests instances I'm using is Diagnostics\TestInterface. Some generic tests that implement it live in the same namespace. If the user defines tests as closures (or provides some simple boolean-returning function name) a Diagnostics\Test\Callback instance is automatically created.

[lsmith77]

The TestInterface is very similar to the CheckInterface https://github.com/liip/LiipMonitor/blob/master/Check/CheckInterface.php

I like how you are also supporting closures.

See also the comparison of https://github.com/zendframework/ZFTool/blob/master/src/ZFTool/Diagnostics/Test/ExtensionLoaded.php https://github.com/liip/LiipMonitor/blob/master/Check/PhpExtensionsCheck.php

It seems like you also drew inspiration from nagios with the result reporting: https://github.com/zendframework/ZFTool/tree/master/src/ZFTool/Diagnostics/Result https://github.com/liip/LiipMonitor/blob/master/Result/CheckResult.php

So in terms of integration I would love to see us share the interface so that checks do not need to be duplicated, as we have right now.

[lsmith77]

BTW .. while LiipMonitor has been around for over a year, I do acknowledge that ZF has a bit more weight and from what I gather you guys are a bit hesitant with 3rd party dependencies. Obviously I would prefer to not have to move around that existing library but I am willing to consider it. However for that to be feasible I guess I would prefer for the interfaces and actual checks to become their own package, leaving the integration with ZF Tool where it currently is.

CCing @videlalvaro since he originally created this code.

[Thinkscape]

When designing Diag, I've been following our internal ZF2 guidelines on arch. style. I also tried to make it as simplistic as possible.

ZFTool nor ZF2 will not load external deps and I don't feel that's even a good idea.

However, what we could do is add some level of integration between both libs in terms of cross-consuming checks. That'd mean people could port their existing checks and run them in ZF2 with ZFTool or take ZF2 checks and use them with symfony bundle. How does that sound to you?

[weierophinney]

Artur, please don't take such a hardline stance.

Lukas is suggesting extracting interfaces to a common repository, which would make it possible to have alternate implementations and/or re-use the same implementation across multiple projects. This is not really an "external dependency," particularly if the same authors are involved; considering that this is happening in a module, and not the framework, I do not see it as a conflict at all.

If you're not willing to do this, I or somebody else on my team is; considering that you did the original work, however, you're a more natural fit. Please reconsider.

On Wed, May 8, 2013 at 5:40 AM, Artur Bodera notifications@github.comwrote:

When designing Diag, I've been following our internal ZF2 guidelines on arch. style. I also tried to make it as simplistic as possible.

ZFTool nor ZF2 will not load external deps and I don't feel that's even a good idea.

However, what we could do is add some level of integration between both libs in terms of cross-consuming checks. That'd mean people could port their existing checks and run them in ZF2 with ZFTool or take ZF2 checks and use them with symfony bundle. How does that sound to you?

— Reply to this email directly or view it on GitHubhttps://github.com/zendframework/ZFTool/issues/39#issuecomment-17597726 .

Matthew Weier O'Phinney matthew@weierophinney.net http://mwop.net/

[lsmith77]

Right .. like I said I am willing to consider to change LiipMonitor to use ZF2 interfaces. However for that to be feasible I would like those interfaces to be in a smaller package. With such an approach it wouldn't even require ZF2 to have a 3rd party dependency.

Now I was also suggesting to move the non ZF2 related checks (like extension etc) over there too at which point I would be willing to abandon LiipMonitor entirely, moving any checks you guys might not yet have over to ZF2.

With such a decoupled package I could update LiipMonitorBundle accordingly and ZFTool would be the equivalent module to integrate into ZF2 MVC.

[Thinkscape]

@weierophinney I'm not taking any stance... Why are you threatening on selecting "someone else to pick up the job"? We're in the middle of discussion here....

I'm open to suggestions on how we can make it work. There are several possible ways to do it and we're trying to discuss all of them. We might extract common interfaces but that might not be needed. I see the following ways of integration:

1. We create a 3rd party repo with common interfaces, both components can then $test instanceof CommonIntf and invoke it accordingly.
2. Don't create any new interfaces, make both components aware of each other - if($test instanceof LiipIntf) $test->check() and such.
3. Don't modify the "general-approach" LiipMonitor (which is really equivalent of ZFTool\Diagnostics\Runner, decoupled) - instead augment that S2 bundle and ZFTool module so they can instantiate and invoke each others' tests. Because test results format and interpretation differs they might be "decorated" with a wrapper class (for example Diagnostics\Test\LiipCheck::__construct(LiipCheck $check).

[Thinkscape]

@lsmith77 How big is the adoption for Liip? Would it upset a lot of people if it was deprecated?

[Thinkscape]

@lsmith77 How about a minimalistic approach to the interface?

namespace AppDiagnostics;

interface DiagnosticCheck
{
    /**
     * @return DiagnosticResultInterface
     */
    public function check();
}

My current interface contains an additional set/getLabel(), while yours contains getGroup() for grouping purposes. I've moved grouping logic to test runner, while you don't use the setter. We could remove all this from the basic altogether - then create GroupableCheck and LabellableCheck interfaces. Whatcha think?

[videlalvaro]

I think the bundle is used a lot. It was pretty popular on the Knpbundles website

On May 8, 2013, at 1:12 PM, Artur Bodera notifications@github.com wrote:

@lsmith77 How big is the adoption for Liip? Would it upset a lot of people if it was deprecated?

— Reply to this email directly or view it on GitHub.

[Thinkscape]

@videlalvaro The bundle is ok, because we can change it and wrap whatever new architecture we devise. It'd be transparent to current users. How about the basic component LiipMonitor (without bundle) ?

[lsmith77]

You can see some stats on the lib here https://packagist.org/packages/liip/monitor I assume that most of these come from users of Symfony2 bundles. Note however that a fair number of them will have created custom checks.

As such I would want to keep the changes to the interfaces to a minimum if possible. Given that ZFTool hasn't been released yet you might be willing to modify the ZFTool interfaces a bit? Like I said I would in turn be open to killing of LiipMonitor in favor of a decoupled package inside ZF2.

I would prefer to get the above done but if we cannot for some reason, then yes making the runners from both sides be able to run checks from the other lib would be great. That being said .. here some interface alignment might still be useful to make the effort smaller.

[Thinkscape]

@lsmith77 ok, here's an idea on how we can approach this to minimise confusion to end-users:

By test I mean a diagnostic check.

1. Create a new universal repo
   • Basic interface will be minimal (see above).
   • All non-MVC tests from ZFTool will be ported.
   • All non-MVC tests from LiipMonitor will be ported.
   • TestAggregate / TestGroup for grouping purposes
2. Update LiipMonitor
   • Remove all tests.
   • Set universal repo as dependency in composer.json.
   • Create a compatibility wrappers which consume universal tests.
3. Update LiipMonitorBundle
   • Set universal repo as dependency in composer.json.
   • By default consume universal tests instead of LiipChecks
   • In case of LiipCheck - wrap it.
   • Grouping logic - use the universal TestAggregate / TestGroup from universal repo.
   • Naming/labeling logic - make it internal to the component.
   • S2 MVC-specific tests live here.
4. Update ZFTool\Diagnostics
   • Set universal repo as dependency in composer.json.
   • Remove all tests.
   • Create a compatibility wrapper (same as above)
   • Consume universal tests by default.
   • Naming/labeling logic - make it internal to Test\Runner.
   • ZF2 MVC-specific tests live here.

Advantages:

1. Tests are decouple and can be used in ANY framework, component or even runner.
2. LiipMonitorBundle and ZFTool\Diagnostics changes will be transparent to users (BC compatible).
3. Users can choose test runner while consuming the same checks (thanks to universal interface)
4. People can add-on more tests to the universal repo (regardless of which framework they use).

[Thinkscape]

I've created a stub at https://github.com/Thinkscape/PHPDiagnostics and invited you all as contributors.

[lsmith77]

Sounds like a good plan to me .. as for the universal repo, given that ZF2 doesnt have a CLA I am more than ok if it lives inside ZF2. I will be away from this afternoon to sunday afternoon CET.

[Thinkscape]

You mean zendframework organisation on github? It is possible, though I believe it might be polarising for some people - on the grounds that "my framework is better than you framework" people might refuse to use it... and symfony2 people might not be very happy that a s2 bundle is depending on zf2-named component.

If we kept all those tests and universal interfaces outside of symfony2 and zf2 (or any particular framework/community) we might get greater adoption (i.e. modules for cake, codeigniter, yii, drupal, etc. all consuming/depending on this universal lib).

@lsmith77 @videlalvaro @weierophinney Thoughts?

[lsmith77]

Don't worry about egos. We are past this. This stuff can live inside ZF2. The key is just to not pull in unused code via the resulting dependency, which is why I was asking to move this out of ZFTool. Thanks to composer I am hopeful that other communities will then also adopt this. Potentially we can one day submit this as a PSR too.

[Thinkscape]

If it was a Zend Framework component, it'd amount to something like: composer require zendframework/zend-diagnostics

I really think it might hurt the adoption if it lived "inside zend framework". On the other hand, I see a lot of benefits of keeping tests outside, on neutral ground, and providing bundles/modules that consume it. @weierophinney has got much more experience in these matters though; I'd wait for his opinion on that.

@lsmith77 What is your reasoning against my proposal? Why do you think it is better to put interfaces and tests inside zf2 ?

[lsmith77]

I think we shouldn't let us be guided by peoples opinions of the past .. the code counts and the maintainers only in so far as if you can trust them to stick around. so imho ZF2 should be a fine place for this code. But we can first see if we can agree on common interfaces (which I am very confident we can) and wait some more before making a decision where the code will live in the end ..

[Thinkscape]

Ok. I disagree zf2 is a good place to put all those checks and intfs., but let's collect opinions from other people before we decide.

For now, I've created a basic set of interfaces: https://github.com/Thinkscape/PHPDiagnostics/tree/master/src/PHPDiagnostics

1. I agree the word Check is more commonly used, i.e. by Nagios and Cacti. ("monitors perform checks").
2. I've created interfaces for check and result.
3. OOP result interfaces IMO are much better than relying on constants. It's contract-based design, much cleaner and logical to perform $result instanceof Success as opposed to extending some class and comparing integers.
4. The basic ResultInterface can be always implemented to provide user-land results, i.e. NoticeResult, SuperBrokenResult, SomewhatBrokenResult etc.
5. I followed Exception logic on the result - it now hints getMessage() and getData() which are the basic pieces of information a result should hold in my opionion.

[weierophinney]

I suggest we have it as an independent component under the zendframework organization; we can add it to packagist and packages.zendframework.com easily that way as well. I'll happily provide the infrastructure for that.

I like the idea of calling it either "Diagnostics" or "PHPDiagnostics" as well.

Let's work on ironing out the interfaces, and then we can worry about the specific repo/project name from there. Sound good, @lsmith77, @videlalvaro, and @Thinkscape ?

[Thinkscape]

Aaaaand it's live! https://github.com/zendframework/Diagnostics thx @weierophinney. Let's switch the discussion to the new repo at: https://github.com/zendframework/Diagnostics/issues/1