Including email conversation regarding OSA host instrumentation issue:
The AbstractMonitor already has a "_processed" attribute that should be set to true once processed, short-circuiting any further calls to monitor.process(). The issue is that the MonitoringEngine.compositeMonitorCompleted(...) method does not initiate processing of the unfinished monitors via the monitor itself. However, simply marking the monitor as processed the first time would not have the desired result for TransactionMonitors anyhow because attributes set in the done() method (latency for e.g.) will not be recorded on the monitor.
IMHO the best fix here is to stop attempting to automatically process the unprocessed child monitors and simply log a warning for these. So if you have:
(Out of order calls to done().) m2 will be processed only when done() is called on it, with its full set of attributes. If m2.done() does not exist or is not reached a warn msg would be logged and the monitor would not be processed.
I like this approach because:
1. It does what you would expect (least surprise)
2. Double processing of monitors may be worse than no processing (especially when the set of attributes differ between the two processing attempts)
From: Korabelnikov, Jack
Sent: Friday, September 12, 2008 11:11 AM
To: Opaczewski, Greg; Mullins, Stephen
Cc: Spevak, Michael
Subject: TransactionMonitors suggestion
You probably remember the problem we had with double data where we were calling done() on monitors twice - once automatically by parent monitor and once manually. My suggestion is to prevent that, make done executable only once and if it was already called then do nothing. Or you can make it like java Thread where once it's done, any call on it results in exception. This way it will fail fast instead people figuring out weird consequences.
P.S. Do you also submit all changes to the open source erma project?
I vote for the pluggable interface strategy, let the users of the library decide if they want the monitors to be processed or not. For Orbitz, I would want them to be processed so as matt said in our MonitoringEngineManager we use that strategy by default.
+1 for breaking up the MonitoringEngine
I agree with the plugin strategy but would prefer #3 by default.
I can see a strategy here. I would also prefer that #3 be the default. Where's Ray for the tie?
Are you guys saying the child monitor shouldn't be allowed to complete?
Closing idle issues.