Fixed ICE-5620: metrics admin creation in IceBox, fixed bug where timer wasn't instrumented

20 files changed, 197 insertions, 182 deletions

diff --git a/cpp/include/Ice/MetricsAdminI.h b/cpp/include/Ice/MetricsAdminI.h
index 56612569b28..ad07c77e057 100644
--- a/cpp/include/Ice/MetricsAdminI.h
+++ b/cpp/include/Ice/MetricsAdminI.h
@@ -571,12 +571,13 @@ private:
 };
 typedef IceUtil::Handle<MetricsViewI> MetricsViewIPtr;
 
-class ICE_API MetricsAdminI : public IceMX::MetricsAdmin, public Ice::PropertiesAdminUpdateCallback, 
+class ICE_API MetricsAdminI : public IceMX::MetricsAdmin, public Ice::PropertiesAdminUpdateCallback,
                               private IceUtil::Mutex
 {
 public:
 
     MetricsAdminI(const ::Ice::PropertiesPtr&, const Ice::LoggerPtr&);
+    ~MetricsAdminI();
 
     void destroy();
 
@@ -638,8 +639,6 @@ public:
 
     const Ice::LoggerPtr& getLogger() const;
 
-    void setProperties(const Ice::PropertiesPtr&);
-
 private:
 
     MetricsViewIPtr getMetricsView(const std::string&);
diff --git a/cpp/include/IceUtil/Timer.h b/cpp/include/IceUtil/Timer.h
index c284187797e..d3d37b99d2e 100644
--- a/cpp/include/IceUtil/Timer.h
+++ b/cpp/include/IceUtil/Timer.h
@@ -43,7 +43,7 @@ typedef IceUtil::Handle<TimerTask> TimerTaskPtr;
 // repeated execution. Tasks are executed by the dedicated timer thread 
 // sequentially.
 //
-class ICE_UTIL_API Timer : public virtual IceUtil::Shared, private virtual IceUtil::Thread
+class ICE_UTIL_API Timer : virtual public IceUtil::Shared, private IceUtil::Thread
 {
 public:
 
@@ -83,7 +83,10 @@ public:
     //
     bool cancel(const TimerTaskPtr&);
 
-private:
+protected:
+
+    virtual void run();
+    virtual void runTimerTask(const TimerTaskPtr&);
 
     struct Token
     {
@@ -95,8 +98,6 @@ private:
         inline bool operator<(const Token& r) const;
     };
 
-    virtual void run();
-
     IceUtil::Monitor<IceUtil::Mutex> _monitor;
     bool _destroyed;
     std::set<Token> _tokens;
diff --git a/cpp/src/Glacier2/Instance.cpp b/cpp/src/Glacier2/Instance.cpp
index 9e1908a42dd..98b4181433a 100644
--- a/cpp/src/Glacier2/Instance.cpp
+++ b/cpp/src/Glacier2/Instance.cpp
@@ -74,7 +74,7 @@ Glacier2::Instance::Instance(const Ice::CommunicatorPtr& communicator, const Ice
     if(o)
     {
         const_cast<Glacier2::Instrumentation::RouterObserverPtr&>(_observer) = 
-            new RouterObserverI(o->getMetricsAdmin(), 
+            new RouterObserverI(o->getFacet(), 
                                 _properties->getPropertyWithDefault("Glacier2.InstanceName", "Glacier2"));
     }
 }
diff --git a/cpp/src/Ice/CommunicatorI.cpp b/cpp/src/Ice/CommunicatorI.cpp
index 498f7c126f3..25a08049c83 100644
--- a/cpp/src/Ice/CommunicatorI.cpp
+++ b/cpp/src/Ice/CommunicatorI.cpp
@@ -156,7 +156,7 @@ Ice::CommunicatorI::getLogger() const
 Ice::Instrumentation::CommunicatorObserverPtr
 Ice::CommunicatorI::getObserver() const
 {
-    return _instance->getObserver();
+    return _instance->initializationData().observer;
 }
 
 RouterPrx
diff --git a/cpp/src/Ice/ConnectionFactory.cpp b/cpp/src/Ice/ConnectionFactory.cpp
index 23ae8d63429..064dd10f0a6 100644
--- a/cpp/src/Ice/ConnectionFactory.cpp
+++ b/cpp/src/Ice/ConnectionFactory.cpp
@@ -227,7 +227,7 @@ IceInternal::OutgoingConnectionFactory::create(const vector<EndpointIPtr>& endpt
     // Try to establish the connection to the connectors.
     //
     DefaultsAndOverridesPtr defaultsAndOverrides = _instance->defaultsAndOverrides();
-    const CommunicatorObserverPtr& obsv = _instance->getObserver();
+    const CommunicatorObserverPtr& obsv = _instance->initializationData().observer;
     vector<ConnectorInfo>::const_iterator q;
     for(q = connectors.begin(); q != connectors.end(); ++q)
     {
@@ -1139,7 +1139,7 @@ IceInternal::OutgoingConnectionFactory::ConnectCallback::nextConnector()
     try
     {
 
-        const CommunicatorObserverPtr& obsv = _factory->_instance->getObserver();
+        const CommunicatorObserverPtr& obsv = _factory->_instance->initializationData().observer;
         if(obsv)
         {
             _observer = obsv->getConnectionEstablishmentObserver(_iter->endpoint, _iter->connector->toString());
diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp
index fad11c7cd0f..564e3ab88d7 100644
--- a/cpp/src/Ice/ConnectionI.cpp
+++ b/cpp/src/Ice/ConnectionI.cpp
@@ -530,11 +530,11 @@ Ice::ConnectionI::updateObserver()
         return;
     }
 
-    assert(_instance->getObserver());
-    _observer.attach(_instance->getObserver()->getConnectionObserver(initConnectionInfo(),
-                                                                     _endpoint,
-                                                                     toConnectionState(_state),
-                                                                     _observer.get()));
+    assert(_instance->initializationData().observer);
+    _observer.attach(_instance->initializationData().observer->getConnectionObserver(initConnectionInfo(),
+                                                                                     _endpoint,
+                                                                                     toConnectionState(_state),
+                                                                                     _observer.get()));
 }
 
 void
@@ -2564,16 +2564,16 @@ Ice::ConnectionI::setState(State state)
         }
     }
 
-    if(_instance->getObserver())
+    if(_instance->initializationData().observer)
     {
         ConnectionState oldState = toConnectionState(_state);
         ConnectionState newState = toConnectionState(state);
         if(oldState != newState)
         {
-            _observer.attach(_instance->getObserver()->getConnectionObserver(initConnectionInfo(),
-                                                                             _endpoint,
-                                                                             newState,
-                                                                             _observer.get()));
+            _observer.attach(_instance->initializationData().observer->getConnectionObserver(initConnectionInfo(),
+                                                                                             _endpoint,
+                                                                                             newState,
+                                                                                             _observer.get()));
         }
         if(_observer && state == StateClosed && _exception.get())
         {
diff --git a/cpp/src/Ice/IPEndpointI.cpp b/cpp/src/Ice/IPEndpointI.cpp
index acbfb356715..b8df1f4da07 100644
--- a/cpp/src/Ice/IPEndpointI.cpp
+++ b/cpp/src/Ice/IPEndpointI.cpp
@@ -550,7 +550,7 @@ IceInternal::EndpointHostResolver::resolve(const string& host, int port, Ice::En
     }
 
     ObserverHelperT<> observer;
-    const CommunicatorObserverPtr& obsv = _instance->getObserver();
+    const CommunicatorObserverPtr& obsv = _instance->initializationData().observer;
     if(obsv)
     {
         observer.attach(obsv->getEndpointLookupObserver(endpoint));
@@ -615,7 +615,7 @@ IceInternal::EndpointHostResolver::resolve(const string& host, int port, Ice::En
     entry.endpoint = endpoint;
     entry.callback = callback;
 
-    const CommunicatorObserverPtr& obsv = _instance->getObserver();
+    const CommunicatorObserverPtr& obsv = _instance->initializationData().observer;
     if(obsv)
     {
         entry.observer = obsv->getEndpointLookupObserver(endpoint);
@@ -662,13 +662,13 @@ IceInternal::EndpointHostResolver::run()
             threadObserver = _observer.get();
         }
 
-        try
+        if(threadObserver)
         {
-            if(threadObserver)
-            {
-                threadObserver->stateChanged(ThreadStateIdle, ThreadStateInUseForOther);
-            }
+            threadObserver->stateChanged(ThreadStateIdle, ThreadStateInUseForOther);
+        }
 
+        try
+        {
             NetworkProxyPtr networkProxy = _instance->networkProxy();
             ProtocolSupport protocol = _protocol;
             if(networkProxy)
@@ -735,7 +735,7 @@ void
 IceInternal::EndpointHostResolver::updateObserver()
 {
     Lock sync(*this);
-    const CommunicatorObserverPtr& obsv = _instance->getObserver();
+    const CommunicatorObserverPtr& obsv = _instance->initializationData().observer;
     if(obsv)
     {
         _observer.attach(obsv->getThreadObserver("Communicator", name(), ThreadStateIdle, _observer.get()));
diff --git a/cpp/src/Ice/Incoming.cpp b/cpp/src/Ice/Incoming.cpp
index 7a6f216b041..b2cc6cbaef4 100644
--- a/cpp/src/Ice/Incoming.cpp
+++ b/cpp/src/Ice/Incoming.cpp
@@ -601,7 +601,7 @@ IceInternal::Incoming::invoke(const ServantManagerPtr& servantManager, BasicStre
         _current.ctx.insert(_current.ctx.end(), pr);
     }
 
-    const CommunicatorObserverPtr& obsv = _is->instance()->getObserver();
+    const CommunicatorObserverPtr& obsv = _is->instance()->initializationData().observer;
     if(obsv)
     { 
         // Read the parameter encapsulation size.
diff --git a/cpp/src/Ice/Instance.cpp b/cpp/src/Ice/Instance.cpp
index 6e7734979a4..0247b858704 100644
--- a/cpp/src/Ice/Instance.cpp
+++ b/cpp/src/Ice/Instance.cpp
@@ -37,13 +37,13 @@
 #include <IceUtil/StringUtil.h>
 #include <Ice/PropertiesI.h>
 #include <Ice/Communicator.h>
-#include <Ice/MetricsAdminI.h>
 #include <Ice/InstrumentationI.h>
 #include <Ice/ProtocolInstance.h>
 #include <Ice/LoggerAdminI.h>
 #include <IceUtil/UUID.h>
 #include <IceUtil/Mutex.h>
 #include <IceUtil/MutexPtrLock.h>
+#include <Ice/ObserverHelper.h>
 
 #include <stdio.h>
 #include <list>
@@ -186,6 +186,84 @@ private:
     const InstancePtr _instance;
 };
 
+
+//
+// Timer specialization which supports the thread observer 
+//
+class Timer : public IceUtil::Timer
+{
+public:
+
+    Timer(int priority) : IceUtil::Timer(priority) 
+    {
+    }
+
+    Timer()
+    {
+    }
+
+    void updateObserver(const Ice::Instrumentation::CommunicatorObserverPtr&);
+
+private:
+
+    virtual void runTimerTask(const IceUtil::TimerTaskPtr&);
+
+    IceUtil::Mutex _mutex;
+    volatile bool _hasObserver;
+    ObserverHelperT<Ice::Instrumentation::ThreadObserver> _observer;
+};
+
+}
+
+void
+Timer::updateObserver(const Ice::Instrumentation::CommunicatorObserverPtr& obsv)
+{
+    IceUtil::Mutex::Lock sync(_mutex);
+    assert(obsv);
+    _observer.attach(obsv->getThreadObserver("Communicator", 
+                                             "Ice.Timer", 
+                                             Ice::Instrumentation::ThreadStateIdle, 
+                                             _observer.get()));
+    _hasObserver = _observer.get();
+}
+
+void
+Timer::runTimerTask(const IceUtil::TimerTaskPtr& task)
+{
+    if(_hasObserver)
+    {
+        Ice::Instrumentation::ThreadObserverPtr threadObserver;
+        {
+            IceUtil::Mutex::Lock sync(_mutex);
+            threadObserver = _observer.get();
+        }
+        if(threadObserver)
+        {
+            threadObserver->stateChanged(Ice::Instrumentation::ThreadStateIdle, 
+                                         Ice::Instrumentation::ThreadStateInUseForOther);
+        }
+        try
+        {
+            task->runTimerTask();
+        }
+        catch(...)
+        {
+            if(threadObserver)
+            {
+                threadObserver->stateChanged(Ice::Instrumentation::ThreadStateInUseForOther, 
+                                             Ice::Instrumentation::ThreadStateIdle);
+            }
+        }
+        if(threadObserver)
+        {
+            threadObserver->stateChanged(Ice::Instrumentation::ThreadStateInUseForOther, 
+                                         Ice::Instrumentation::ThreadStateIdle);
+        }
+    }
+    else
+    {
+        task->runTimerTask();
+    }
 }
 
 IceUtil::Shared* IceInternal::upCast(Instance* p) { return p; }
@@ -1394,44 +1472,42 @@ IceInternal::Instance::finishSetup(int& argc, char* argv[], const Ice::Communica
     }
 
     PropertiesAdminIPtr propsAdmin;
-
     if(_adminEnabled)
     {
         _adminFacets.insert(FacetMap::value_type("Process", new ProcessI(communicator)));
         
         propsAdmin = new PropertiesAdminI("Properties", _initData.properties, _initData.logger);
         _adminFacets.insert(FacetMap::value_type("Properties", propsAdmin));
-
-        _metricsAdmin = new MetricsAdminI(_initData.properties, _initData.logger);
-        _adminFacets.insert(FacetMap::value_type("Metrics", _metricsAdmin));
     }
-     
+
     //
-    // Setup the communicator observer only if the user didn't already set an
-    // Ice observer resolver and Admin is enabled
+    // Setup the communicator observer if Admin is enabled and the
+    // facet isn't filtered or if Ice.Admin.Metrics is enabled.
     //
-    if(_adminEnabled && (_adminFacetFilter.empty() || _adminFacetFilter.find("Metrics") != _adminFacetFilter.end()))
+    if((_adminEnabled && (_adminFacetFilter.empty() || _adminFacetFilter.find("Metrics") != _adminFacetFilter.end())) ||
+       _initData.properties->getPropertyAsInt("Ice.Admin.Metrics"))
     {
-        _observer = new CommunicatorObserverI(_metricsAdmin, _initData.observer);
+        CommunicatorObserverIPtr observer = new CommunicatorObserverI(_initData);
+        _initData.observer = observer;
+        _adminFacets.insert(FacetMap::value_type("Metrics", observer->getFacet()));
         
         //
-        // Make sure the admin plugin receives property updates.
+        // Make sure the metrics admin facet receives property updates.
         //
-        propsAdmin->addUpdateCallback(_metricsAdmin);
-    }
-    else
-    {
-        _observer = _initData.observer;
+        if(propsAdmin)
+        {
+            propsAdmin->addUpdateCallback(observer->getFacet());
+        }
     }
-
+        
     //
     // Set observer updater
     //
-    if(_observer)
+    if(_initData.observer)
     {
-        _observer->setObserverUpdater(new ObserverUpdaterI(this));
+        _initData.observer->setObserverUpdater(new ObserverUpdaterI(this));
     }
-        
+
     //
     // Create threads.
     //
@@ -1441,11 +1517,11 @@ IceInternal::Instance::finishSetup(int& argc, char* argv[], const Ice::Communica
         int priority = _initData.properties->getPropertyAsInt("Ice.ThreadPriority");
         if(hasPriority)
         {
-            _timer = new IceUtil::Timer(priority);
+            _timer = new Timer(priority);
         }
         else
         {
-            _timer = new IceUtil::Timer;
+            _timer = new Timer;
         }
     }
     catch(const IceUtil::Exception& ex)
@@ -1590,21 +1666,14 @@ IceInternal::Instance::destroy()
         _retryQueue->destroy();
     }
 
-    if(_metricsAdmin)
+    if(_initData.observer)
     {
-        _metricsAdmin->destroy();
-        _metricsAdmin = 0;
-
-        if(_observer)
-        {
-            // Break cyclic reference counts. Don't clear _observer, it's immutable.
-            CommunicatorObserverIPtr observer = CommunicatorObserverIPtr::dynamicCast(_observer);
-            if(observer)
-            {
-                observer->destroy();
-            }
-            _observer->setObserverUpdater(0); // Break cyclic reference count.
+        CommunicatorObserverIPtr observer = CommunicatorObserverIPtr::dynamicCast(_initData.observer);
+        if(observer)
+        {        
+            observer->destroy(); // Break cyclic reference counts. Don't clear _observer, it's immutable.
         }
+        _initData.observer->setObserverUpdater(0); // Break cyclic reference count.
     }
 
     Ice::LoggerAdminLoggerPtr logger = Ice::LoggerAdminLoggerPtr::dynamicCast(_initData.logger);
@@ -1619,7 +1688,7 @@ IceInternal::Instance::destroy()
     ThreadPoolPtr serverThreadPool;
     ThreadPoolPtr clientThreadPool;
     EndpointHostResolverPtr endpointHostResolver;
-    IceUtil::TimerPtr timer;
+    TimerPtr timer;
     {
         IceUtil::RecMutex::Lock sync(*this);
 
@@ -1766,6 +1835,10 @@ IceInternal::Instance::updateThreadObservers()
         {
             _endpointHostResolver->updateObserver();
         }
+        if(_timer)
+        {
+            _timer->updateObserver(_initData.observer);
+        }
     }
     catch(const Ice::CommunicatorDestroyedException&)
     {
diff --git a/cpp/src/Ice/Instance.h b/cpp/src/Ice/Instance.h
index c774242dfdd..95c4aecb620 100644
--- a/cpp/src/Ice/Instance.h
+++ b/cpp/src/Ice/Instance.h
@@ -54,6 +54,9 @@ class CommunicatorI;
 namespace IceInternal
 {
 
+class Timer;
+typedef IceUtil::Handle<Timer> TimerPtr;
+
 class MetricsAdminI;
 typedef IceUtil::Handle<MetricsAdminI> MetricsAdminIPtr;
 
@@ -96,11 +99,6 @@ public:
     Ice::ObjectPtr removeAdminFacet(const std::string&);
     Ice::ObjectPtr findAdminFacet(const std::string&);
 
-    const Ice::Instrumentation::CommunicatorObserverPtr& getObserver() const
-    {
-        return _observer;
-    }
-
     const Ice::ImplicitContextIPtr& getImplicitContext() const
     {
         return _implicitContext;
@@ -162,7 +160,7 @@ private:
     ThreadPoolPtr _serverThreadPool;
     EndpointHostResolverPtr _endpointHostResolver;
     RetryQueuePtr _retryQueue;
-    IceUtil::TimerPtr _timer;
+    TimerPtr _timer;
     EndpointFactoryManagerPtr _endpointFactoryManager;
     DynamicLibraryListPtr _dynamicLibraryList;
     Ice::PluginManagerPtr _pluginManager;
@@ -175,7 +173,6 @@ private:
     Ice::Identity _adminIdentity;
     std::set<std::string> _adminFacetFilter;
     IceInternal::MetricsAdminIPtr _metricsAdmin;
-    Ice::Instrumentation::CommunicatorObserverPtr _observer;
 };
 
 class ProcessI : public Ice::Process
diff --git a/cpp/src/Ice/InstrumentationI.cpp b/cpp/src/Ice/InstrumentationI.cpp
index b0a06b1cf08..dac18cab9dd 100644
--- a/cpp/src/Ice/InstrumentationI.cpp
+++ b/cpp/src/Ice/InstrumentationI.cpp
@@ -891,17 +891,15 @@ InvocationObserverI::getCollocatedObserver(const Ice::ObjectAdapterPtr& adapter,
     return 0;
 }
 
-CommunicatorObserverI::CommunicatorObserverI(const IceInternal::MetricsAdminIPtr& metrics,
-                                             const Ice::Instrumentation::CommunicatorObserverPtr& delegate) : 
-    _metrics(metrics),
-    _logger(metrics->getLogger()),
-    _delegate(delegate),
-    _connections(metrics, "Connection"),
-    _dispatch(metrics, "Dispatch"),
-    _invocations(metrics, "Invocation"),
-    _threads(metrics, "Thread"),
-    _connects(metrics, "ConnectionEstablishment"),
-    _endpointLookups(metrics, "EndpointLookup")
+CommunicatorObserverI::CommunicatorObserverI(const InitializationData& initData) : 
+    _metrics(new MetricsAdminI(initData.properties, initData.logger)),
+    _delegate(initData.observer),
+    _connections(_metrics, "Connection"),
+    _dispatch(_metrics, "Dispatch"),
+    _invocations(_metrics, "Invocation"),
+    _threads(_metrics, "Thread"),
+    _connects(_metrics, "ConnectionEstablishment"),
+    _endpointLookups(_metrics, "EndpointLookup")
 {
     _invocations.registerSubMap<RemoteMetrics>("Remote", &InvocationMetrics::remotes);
     _invocations.registerSubMap<CollocatedMetrics>("Collocated", &InvocationMetrics::collocated);
@@ -934,7 +932,7 @@ CommunicatorObserverI::getConnectionEstablishmentObserver(const EndpointPtr& end
         }
         catch(const exception& ex)
         {
-            Error error(_logger);
+            Error error(_metrics->getLogger());
             error << "unexpected exception trying to obtain observer:\n" << ex;
         }
     }
@@ -957,7 +955,7 @@ CommunicatorObserverI::getEndpointLookupObserver(const EndpointPtr& endpt)
         }
         catch(const exception& ex)
         {
-            Error error(_logger);
+            Error error(_metrics->getLogger());
             error << "unexpected exception trying to obtain observer:\n" << ex;
         }
     }
@@ -984,7 +982,7 @@ CommunicatorObserverI::getConnectionObserver(const ConnectionInfoPtr& con,
         }
         catch(const exception& ex)
         {
-            Error error(_logger);
+            Error error(_metrics->getLogger());
             error << "unexpected exception trying to obtain observer:\n" << ex;
         }
     }
@@ -1011,7 +1009,7 @@ CommunicatorObserverI::getThreadObserver(const string& parent,
         }
         catch(const exception& ex)
         {
-            Error error(_logger);
+            Error error(_metrics->getLogger());
             error << "unexpected exception trying to obtain observer:\n" << ex;
         }
     }
@@ -1034,7 +1032,7 @@ CommunicatorObserverI::getInvocationObserver(const ObjectPrx& proxy, const strin
         }
         catch(const exception& ex)
         {
-            Error error(_logger);
+            Error error(_metrics->getLogger());
             error << "unexpected exception trying to obtain observer:\n" << ex;
         }
     }
@@ -1057,7 +1055,7 @@ CommunicatorObserverI::getDispatchObserver(const Current& current, int size)
         }
         catch(const exception& ex)
         {
-            Error error(_logger);
+            Error error(_metrics->getLogger());
             error << "unexpected exception trying to obtain observer:\n" << ex;
         }
     }
@@ -1065,7 +1063,7 @@ CommunicatorObserverI::getDispatchObserver(const Current& current, int size)
 }
 
 const IceInternal::MetricsAdminIPtr& 
-CommunicatorObserverI::getMetricsAdmin() const
+CommunicatorObserverI::getFacet() const
 {
     assert(_metrics);
     return _metrics;
@@ -1074,11 +1072,13 @@ CommunicatorObserverI::getMetricsAdmin() const
 void
 CommunicatorObserverI::destroy()
 {
-    _metrics = 0;
     _connections.destroy();
     _dispatch.destroy();
     _invocations.destroy();
     _threads.destroy();
     _connects.destroy();
     _endpointLookups.destroy();
+
+    _metrics->destroy();
+    _metrics = 0;
 }
diff --git a/cpp/src/Ice/InstrumentationI.h b/cpp/src/Ice/InstrumentationI.h
index db515999f23..f1982b765ea 100644
--- a/cpp/src/Ice/InstrumentationI.h
+++ b/cpp/src/Ice/InstrumentationI.h
@@ -214,9 +214,7 @@ class ICE_API CommunicatorObserverI : public Ice::Instrumentation::CommunicatorO
 {
 public:
 
-    CommunicatorObserverI(const IceInternal::MetricsAdminIPtr&,
-                          const Ice::Instrumentation::CommunicatorObserverPtr& = 
-                          Ice::Instrumentation::CommunicatorObserverPtr());
+    CommunicatorObserverI(const Ice::InitializationData&);
 
     virtual void setObserverUpdater(const Ice::Instrumentation::ObserverUpdaterPtr&);
  
@@ -241,14 +239,13 @@ public:
 
     virtual Ice::Instrumentation::DispatchObserverPtr getDispatchObserver(const Ice::Current&, Ice::Int);
 
-    const IceInternal::MetricsAdminIPtr& getMetricsAdmin() const;
+    const IceInternal::MetricsAdminIPtr& getFacet() const;
 
     void destroy();
 
 private:
 
     IceInternal::MetricsAdminIPtr _metrics;
-    Ice::LoggerPtr _logger;
     const Ice::Instrumentation::CommunicatorObserverPtr _delegate;
 
     ObserverFactoryWithDelegateT<ConnectionObserverI> _connections;
diff --git a/cpp/src/Ice/MetricsAdminI.cpp b/cpp/src/Ice/MetricsAdminI.cpp
index 4f5e6ae065c..b661d5e554b 100644
--- a/cpp/src/Ice/MetricsAdminI.cpp
+++ b/cpp/src/Ice/MetricsAdminI.cpp
@@ -387,6 +387,10 @@ MetricsAdminI::MetricsAdminI(const PropertiesPtr& properties, const LoggerPtr& l
     updateViews();
 }
 
+MetricsAdminI::~MetricsAdminI()
+{
+}
+
 void
 MetricsAdminI::destroy()
 {
@@ -605,12 +609,6 @@ MetricsAdminI::getLogger() const
     return _logger;
 }
 
-void
-MetricsAdminI::setProperties(const ::Ice::PropertiesPtr& properties)
-{
-    _properties = properties;
-}
-
 MetricsViewIPtr
 MetricsAdminI::getMetricsView(const std::string& name)
 {
diff --git a/cpp/src/Ice/ObserverHelper.cpp b/cpp/src/Ice/ObserverHelper.cpp
index dcbe0506d86..4e77077bce5 100644
--- a/cpp/src/Ice/ObserverHelper.cpp
+++ b/cpp/src/Ice/ObserverHelper.cpp
@@ -25,7 +25,7 @@ Ice::Context emptyCtx;
 
 IceInternal::InvocationObserver::InvocationObserver(IceProxy::Ice::Object* proxy, const string& op, const Context* ctx)
 {
-    const CommunicatorObserverPtr& obsv = proxy->__reference()->getInstance()->getObserver();
+    const CommunicatorObserverPtr& obsv = proxy->__reference()->getInstance()->initializationData().observer;
     if(!obsv)
     {
         return;
@@ -43,7 +43,7 @@ IceInternal::InvocationObserver::InvocationObserver(IceProxy::Ice::Object* proxy
 
 IceInternal::InvocationObserver::InvocationObserver(IceInternal::Instance* instance, const string& op)
 {
-    const CommunicatorObserverPtr& obsv = instance->getObserver();
+    const CommunicatorObserverPtr& obsv = instance->initializationData().observer;
     if(!obsv)
     {
         return;
@@ -55,7 +55,7 @@ IceInternal::InvocationObserver::InvocationObserver(IceInternal::Instance* insta
 void
 IceInternal::InvocationObserver::attach(IceProxy::Ice::Object* proxy, const string& op, const Context* ctx)
 {
-    const CommunicatorObserverPtr& obsv = proxy->__reference()->getInstance()->getObserver();
+    const CommunicatorObserverPtr& obsv = proxy->__reference()->getInstance()->initializationData().observer;
     if(!obsv)
     {
         return;
@@ -74,7 +74,7 @@ IceInternal::InvocationObserver::attach(IceProxy::Ice::Object* proxy, const stri
 void
 IceInternal::InvocationObserver::attach(IceInternal::Instance* instance, const string& op)
 {
-    const CommunicatorObserverPtr& obsv = instance->getObserver();
+    const CommunicatorObserverPtr& obsv = instance->initializationData().observer;
     if(!obsv)
     {
         return;
diff --git a/cpp/src/Ice/ThreadPool.cpp b/cpp/src/Ice/ThreadPool.cpp
index 1b2907a87c6..a1bb3ab424e 100644
--- a/cpp/src/Ice/ThreadPool.cpp
+++ b/cpp/src/Ice/ThreadPool.cpp
@@ -1261,7 +1261,7 @@ void
 IceInternal::ThreadPool::EventHandlerThread::updateObserver()
 {
     // Must be called with the thread pool mutex locked
-    const CommunicatorObserverPtr& obsv = _pool->_instance->getObserver();
+    const CommunicatorObserverPtr& obsv = _pool->_instance->initializationData().observer;
     if(obsv)
     {
         _observer.attach(obsv->getThreadObserver(_pool->_prefix, name(), _state, _observer.get()));
diff --git a/cpp/src/IceBox/ServiceManagerI.cpp b/cpp/src/IceBox/ServiceManagerI.cpp
index bffc34ae55a..43f183a87c4 100644
--- a/cpp/src/IceBox/ServiceManagerI.cpp
+++ b/cpp/src/IceBox/ServiceManagerI.cpp
@@ -15,8 +15,6 @@
 #include <Ice/Initialize.h>
 #include <Ice/Instance.h>
 #include <Ice/PropertiesAdminI.h>
-#include <Ice/MetricsAdminI.h>
-#include <Ice/InstrumentationI.h>
 #include <Ice/LoggerAdminI.h>
 #include <IceBox/ServiceManagerI.h>
 
@@ -399,27 +397,16 @@ IceBox::ServiceManagerI::start()
             }
 
             //
-            // If Ice metrics are enabled on the IceBox communicator, we also enable them on the 
-            // shared communicator.
+            // If Ice metrics are enabled on the IceBox communicator, we also enable them on
+            // the service communicator.
             // 
-            IceInternal::MetricsAdminIPtr metricsAdmin;
-            if(IceInternal::CommunicatorObserverIPtr::dynamicCast(_communicator->getObserver()))
+            if(_communicator->findAdminFacet("Metrics") && 
+               initData.properties->getProperty("Ice.Admin.Metrics").empty())
             {
-                metricsAdmin = new IceInternal::MetricsAdminI(initData.properties, getProcessLogger());
-                initData.observer = new IceInternal::CommunicatorObserverI(metricsAdmin);
+                initData.properties->setProperty("Ice.Admin.Metrics", "1");
             }
 
             _sharedCommunicator = initialize(initData);
-
-            //
-            // Ensure the metrics admin plugin uses the same property set as the
-            // communicator. This is necessary to correctly deal with runtime 
-            // property updates.
-            //
-            if(metricsAdmin)
-            {
-                metricsAdmin->setProperties(_sharedCommunicator->getProperties());
-            }
         }
 
         //
@@ -551,16 +538,12 @@ IceBox::ServiceManagerI::start(const string& service, const string& entryPoint,
     // property set.
     //
     Ice::CommunicatorPtr communicator;
-    IceInternal::MetricsAdminIPtr metricsAdmin;
+    Ice::ObjectPtr metricsAdmin;
     if(_communicator->getProperties()->getPropertyAsInt("IceBox.UseSharedCommunicator." + service) > 0)
     {
         assert(_sharedCommunicator);
         communicator = _sharedCommunicator;
-        Ice::Instrumentation::CommunicatorObserverPtr o = communicator->getObserver();
-        if(o)
-        {
-            metricsAdmin = IceInternal::CommunicatorObserverIPtr::dynamicCast(o)->getMetricsAdmin();
-        }
+        metricsAdmin = _sharedCommunicator->findAdminFacet("Metrics");
     }
     else
     {
@@ -609,10 +592,10 @@ IceBox::ServiceManagerI::start(const string& service, const string& entryPoint,
             // If Ice metrics are enabled on the IceBox communicator, we also enable them on
             // the service communicator.
             // 
-            if(IceInternal::CommunicatorObserverIPtr::dynamicCast(_communicator->getObserver()))
+            if(_communicator->findAdminFacet("Metrics") && 
+               initData.properties->getProperty("Ice.Admin.Metrics").empty())
             {
-                metricsAdmin = new IceInternal::MetricsAdminI(initData.properties, initData.logger);
-                initData.observer = new IceInternal::CommunicatorObserverI(metricsAdmin);
+                initData.properties->setProperty("Ice.Admin.Metrics", "1");
             }
 
             //
@@ -654,15 +637,7 @@ IceBox::ServiceManagerI::start(const string& service, const string& entryPoint,
                 }
             }
 
-            //
-            // Ensure the metrics admin plugin uses the same property set as the
-            // communicator. This is necessary to correctly deal with runtime 
-            // property updates.
-            //
-            if(metricsAdmin)
-            {
-                metricsAdmin->setProperties(communicator->getProperties());
-            }
+            metricsAdmin = communicator->findAdminFacet("Metrics");
         }
         catch(const Exception& ex)
         {
@@ -698,7 +673,7 @@ IceBox::ServiceManagerI::start(const string& service, const string& entryPoint,
             _communicator->addAdminFacet(metricsAdmin, "IceBox.Service." + info.name + ".Metrics");
 
             // Ensure the metrics admin facet is notified of property updates.
-            propAdmin->addUpdateCallback(metricsAdmin);
+            propAdmin->addUpdateCallback(Ice::PropertiesAdminUpdateCallbackPtr::dynamicCast(metricsAdmin));
         }
 
         //
@@ -796,19 +771,6 @@ IceBox::ServiceManagerI::start(const string& service, const string& entryPoint,
             // Ignored
         }
 
-        try
-        {
-            Ice::ObjectPtr admin = _communicator->removeAdminFacet("IceBox.Service." + info.name + ".Metrics");
-            if(admin && communicator != _sharedCommunicator)
-            {
-                IceInternal::MetricsAdminIPtr::dynamicCast(admin)->destroy();
-            }
-        }
-        catch(const LocalException&)
-        {
-            // Ignored
-        }
-
         if(info.communicator)
         {
             destroyServiceCommunicator(info.name, info.communicator);
@@ -880,19 +842,6 @@ IceBox::ServiceManagerI::stopAll()
             // Ignored
         }
         
-        try
-        {
-            Ice::ObjectPtr admin = _communicator->removeAdminFacet("IceBox.Service." + info.name + ".Metrics");
-            if(admin && info.communicator != _sharedCommunicator)
-            {
-                IceInternal::MetricsAdminIPtr::dynamicCast(admin)->destroy();
-            }
-        }
-        catch(const LocalException&)
-        {
-            // Ignored
-        }
-
         if(info.communicator)
         {
             try
@@ -971,12 +920,6 @@ IceBox::ServiceManagerI::stopAll()
 
     if(_sharedCommunicator)
     {
-        Ice::Instrumentation::CommunicatorObserverPtr o = _sharedCommunicator->getObserver();
-        if(o)
-        {
-            IceInternal::CommunicatorObserverIPtr::dynamicCast(o)->getMetricsAdmin()->destroy();
-        }
-
         try
         {
             _sharedCommunicator->destroy();
diff --git a/cpp/src/IceStorm/Instance.cpp b/cpp/src/IceStorm/Instance.cpp
index 7514beab81b..f3ea3ee134b 100644
--- a/cpp/src/IceStorm/Instance.cpp
+++ b/cpp/src/IceStorm/Instance.cpp
@@ -75,7 +75,7 @@ Instance::Instance(
             IceInternal::CommunicatorObserverIPtr::dynamicCast(communicator->getObserver());
         if(o)
         {
-            _observer = new TopicManagerObserverI(o->getMetricsAdmin());
+            _observer = new TopicManagerObserverI(o->getFacet());
         }
     }
     catch(...)
diff --git a/cpp/src/IceUtil/Timer.cpp b/cpp/src/IceUtil/Timer.cpp
index e18004d06fa..697a0362f2a 100644
--- a/cpp/src/IceUtil/Timer.cpp
+++ b/cpp/src/IceUtil/Timer.cpp
@@ -222,7 +222,7 @@ Timer::run()
         {
             try
             {
-                token.task->runTimerTask();
+                runTimerTask(token.task);
             }
             catch(const IceUtil::Exception& e)
             {
@@ -243,3 +243,9 @@ Timer::run()
         }
     }
 }
+
+void
+Timer::runTimerTask(const TimerTaskPtr& task)
+{
+    task->runTimerTask();
+}
diff --git a/cpp/test/Ice/metrics/AllTests.cpp b/cpp/test/Ice/metrics/AllTests.cpp
index 3205a0b9d53..af37da00016 100644
--- a/cpp/test/Ice/metrics/AllTests.cpp
+++ b/cpp/test/Ice/metrics/AllTests.cpp
@@ -414,9 +414,9 @@ allTests(const Ice::CommunicatorPtr& communicator, const CommunicatorObserverIPt
     updateProps(clientProps, serverProps, update, props);
 
 #ifndef ICE_OS_WINRT
-    int threadCount = 3;
+    int threadCount = 4;
 #else
-    int threadCount = 2; // No endpoint host resolver thread with WinRT.
+    int threadCount = 3; // No endpoint host resolver thread with WinRT.
 #endif
 
     Ice::Long timestamp;
diff --git a/cpp/test/IceBox/admin/AllTests.cpp b/cpp/test/IceBox/admin/AllTests.cpp
index e0828dd8f7b..c8dd1ffd1fc 100644
--- a/cpp/test/IceBox/admin/AllTests.cpp
+++ b/cpp/test/IceBox/admin/AllTests.cpp
@@ -47,13 +47,14 @@ allTests(const Ice::CommunicatorPtr& communicator)
         // Test: PropertiesAdmin::getProperties()
         //
         Ice::PropertyDict pd = pa->getPropertiesForPrefix("");
-        test(pd.size() == 6);
+        test(pd.size() == 7);
         test(pd["Prop1"] == "1");
         test(pd["Prop2"] == "2");
         test(pd["Prop3"] == "3");
         test(pd["Ice.Config"] == "config.service");
         test(pd["Ice.ProgramName"] == "IceBox-TestService");
         test(pd["Ice.Admin.Logger"] == "IceBox.Service.TestService.Logger");
+        test(pd["Ice.Admin.Metrics"] == "1");
 
         Ice::PropertyDict changes;