Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move linking mbean to jmxservice #65

Merged
merged 2 commits into from
Sep 18, 2020
Merged

Move linking mbean to jmxservice #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5bbc5c
pull up static coupling to ServiceFactory (campsite)
breedx-nr Sep 18, 2020
d3a94e5
move LinkingMetadataMBean registration to JMXService
breedx-nr Sep 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions ...-test/src/main/java/com/newrelic/agent/introspec/internal/IntrospectorServiceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,23 @@

package com.newrelic.agent.introspec.internal;

import com.newrelic.agent.*;
import com.newrelic.agent.ExpirationService;
import com.newrelic.agent.HarvestService;
import com.newrelic.agent.RPMServiceManager;
import com.newrelic.agent.ThreadService;
import com.newrelic.agent.TracerService;
import com.newrelic.agent.TransactionService;
import com.newrelic.agent.attributes.AttributesService;
import com.newrelic.agent.browser.BrowserService;
import com.newrelic.agent.cache.CacheService;
import com.newrelic.agent.circuitbreaker.CircuitBreakerService;
import com.newrelic.agent.commands.CommandParser;
import com.newrelic.agent.config.*;
import com.newrelic.agent.config.AgentConfig;
import com.newrelic.agent.config.AgentConfigFactory;
import com.newrelic.agent.config.AgentConfigImpl;
import com.newrelic.agent.config.ConfigService;
import com.newrelic.agent.config.ConfigServiceFactory;
import com.newrelic.agent.config.DistributedTracingConfig;
import com.newrelic.agent.core.CoreService;
import com.newrelic.agent.database.DatabaseService;
import com.newrelic.agent.environment.EnvironmentService;
Expand All @@ -37,7 +47,13 @@
import com.newrelic.agent.service.Service;
import com.newrelic.agent.service.ServiceFactory;
import com.newrelic.agent.service.ServiceManager;
import com.newrelic.agent.service.analytics.*;
import com.newrelic.agent.service.analytics.CollectorSpanEventReservoirManager;
import com.newrelic.agent.service.analytics.CollectorSpanEventSender;
import com.newrelic.agent.service.analytics.InsightsService;
import com.newrelic.agent.service.analytics.SpanEventCreationDecider;
import com.newrelic.agent.service.analytics.SpanEventsService;
import com.newrelic.agent.service.analytics.TransactionDataToDistributedTraceIntrinsics;
import com.newrelic.agent.service.analytics.TransactionEventsService;
import com.newrelic.agent.service.async.AsyncTransactionService;
import com.newrelic.agent.service.module.JarCollectorService;
import com.newrelic.agent.sql.SqlTraceService;
Expand All @@ -48,7 +64,11 @@
import com.newrelic.agent.tracing.DistributedTraceServiceImpl;
import com.newrelic.agent.utilization.UtilizationService;

import java.util.*;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

class IntrospectorServiceManager extends AbstractService implements ServiceManager {

Expand Down Expand Up @@ -154,7 +174,8 @@ private void setup(Map<String, Object> config) {
remoteInstrumentationService = new RemoteInstrumentationServiceImpl();
sourceLanguageService = new SourceLanguageService();
classTransformerService = new NoOpClassTransformerService();
jmxService = new JmxService();
boolean jmxEnabled = configService.getDefaultAgentConfig().getJmxConfig().isEnabled();
jmxService = new JmxService(jmxEnabled);
attributesService = new AttributesService();
circuitBreakerService = new CircuitBreakerService();
AgentConfig agentConfig = createAgentConfig(config, (Map) config.get("distributed_tracing"));
Expand Down
8 changes: 0 additions & 8 deletions newrelic-agent/src/main/java/com/newrelic/agent/Agent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.newrelic.agent.config.JavaVersionUtils;
import com.newrelic.agent.core.CoreService;
import com.newrelic.agent.core.CoreServiceImpl;
import com.newrelic.agent.jmx.LinkingMetadataRegistration;
import com.newrelic.agent.logging.AgentLogManager;
import com.newrelic.agent.logging.IAgentLogger;
import com.newrelic.agent.service.ServiceFactory;
Expand Down Expand Up @@ -167,8 +166,6 @@ public static void continuePremain(String agentArgs, Instrumentation inst, long

logAnyFilesFoundInEndorsedDirs();

registerAgentMBeans();

if (serviceManager.getConfigService().getDefaultAgentConfig().isStartupTimingEnabled()) {
recordPremainTime(serviceManager.getStatsService(), startTime);
}
Expand Down Expand Up @@ -201,11 +198,6 @@ public static void continuePremain(String agentArgs, Instrumentation inst, long
}
}

private static void registerAgentMBeans() {
// This registers the mbean that exposes linking metadata
new LinkingMetadataRegistration(LOG).registerLinkingMetadata();
}

private static boolean tryToInitializeServiceManager(Instrumentation inst) {
try {
CoreService coreService = new CoreServiceImpl(inst);
Expand Down
15 changes: 8 additions & 7 deletions newrelic-agent/src/main/java/com/newrelic/agent/jmx/JmxService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.newrelic.agent.Agent;
import com.newrelic.agent.HarvestListener;
import com.newrelic.agent.config.AgentConfig;
import com.newrelic.agent.config.JmxConfig;
import com.newrelic.agent.extension.Extension;
import com.newrelic.agent.jmx.create.JmxGet;
import com.newrelic.agent.jmx.create.JmxInvoke;
Expand All @@ -21,7 +19,6 @@
import com.newrelic.agent.service.ServiceFactory;
import com.newrelic.agent.stats.StatsEngine;


import javax.management.Attribute;
import javax.management.AttributeNotFoundException;
import javax.management.MBeanServer;
Expand Down Expand Up @@ -76,11 +73,9 @@ public class JmxService extends AbstractService implements HarvestListener {
*/
private final Set<MBeanServer> toRemoveMBeanServers = new CopyOnWriteArraySet<>();

public JmxService() {
public JmxService(boolean enabled) {
super(JmxService.class.getSimpleName());
AgentConfig config = ServiceFactory.getConfigService().getDefaultAgentConfig();
JmxConfig jmxConfig = config.getJmxConfig();
enabled = jmxConfig.isEnabled();
this.enabled = enabled;
jmxMetricFactory = JmxObjectFactory.createJmxFactory();
}

Expand All @@ -97,6 +92,7 @@ public void addJmxAttributeProcessor(JmxAttributeProcessor attributeProcessor) {
@Override
protected void doStart() {
if (enabled) {
registerAgentMBeans();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we want to have a specific config for this? Not just jmx as a whole

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #62 buddy. We want to do small incremental/progressive changes.

jmxMetricFactory.getStartUpJmxObjects(jmxGets, jmxInvokes);
if (jmxGets.size() > 0) {
ServiceFactory.getHarvestService().addHarvestListener(this);
Expand Down Expand Up @@ -420,6 +416,11 @@ public void reloadExtensions(Set<Extension> oldExtensions, Set<Extension> extens
}
}

private void registerAgentMBeans() {
// This registers the mbean that exposes linking metadata
new LinkingMetadataRegistration(Agent.LOG).registerLinkingMetadata();
}

/*
* These are examples of the built in metric names.
*
Expand Down
2 changes: 1 addition & 1 deletion newrelic-agent/src/main/java/com/newrelic/agent/jmx/LinkingMetadataRegistration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public void registerLinkingMetadata() {
Object bean = new LinkingMetadata();
server.registerMBean(bean, name);
logger.log(Level.INFO, "JMX LinkingMetadata bean registered");
} catch (Exception e) {
} catch (Exception | NoClassDefFoundError e) {
logger.log(Level.INFO, "Error registering JMX LinkingMetadata MBean", e);
}
}
Expand Down
7 changes: 6 additions & 1 deletion newrelic-agent/src/main/java/com/newrelic/agent/service/ServiceManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
import com.newrelic.agent.cache.CacheService;
import com.newrelic.agent.circuitbreaker.CircuitBreakerService;
import com.newrelic.agent.commands.CommandParser;
import com.newrelic.agent.config.AgentConfig;
import com.newrelic.agent.config.ConfigService;
import com.newrelic.agent.config.JmxConfig;
import com.newrelic.agent.core.CoreService;
import com.newrelic.agent.database.DatabaseService;
import com.newrelic.agent.deadlock.DeadlockDetectorService;
Expand Down Expand Up @@ -159,7 +161,10 @@ protected synchronized void doStart() throws Exception {
threadService = new ThreadService();
circuitBreakerService = new CircuitBreakerService();
classTransformerService = new ClassTransformerServiceImpl(coreService.getInstrumentation());
jmxService = new JmxService();

AgentConfig config = configService.getDefaultAgentConfig();
JmxConfig jmxConfig = config.getJmxConfig();
jmxService = new JmxService(jmxConfig.isEnabled());

Logger jarCollectorLogger = Agent.LOG.getChildLogger("com.newrelic.jar_collector");
boolean jarCollectorEnabled = configService.getDefaultAgentConfig().getJarCollectorConfig().isEnabled();
Expand Down
4 changes: 1 addition & 3 deletions newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ public MockServiceManager(ConfigService configService) {
Mockito.when(statsService.getMetricAggregator()).thenReturn(metricAggregator);
harvestService = Mockito.mock(HarvestService.class);
sqlTraceService = Mockito.mock(SqlTraceService.class);
// browserService;
// cacheService;
dbService = new DatabaseService();
extensionService = new ExtensionService(configService, ExtensionsLoadedListener.NOOP);
jarCollectorService = Mockito.mock(JarCollectorService.class);
Expand All @@ -142,7 +140,7 @@ public MockServiceManager(ConfigService configService) {
commandParser = new CommandParser();
remoteInstrumentationService = Mockito.mock(RemoteInstrumentationService.class);
classTransformerService = Mockito.mock(ClassTransformerService.class);
jmxService = new JmxService();
jmxService = new JmxService(true);
circuitBreakerService = new CircuitBreakerService();
spanEventsService = Mockito.mock(SpanEventsService.class);
insights = Mockito.mock(InsightsServiceImpl.class);
Expand Down
37 changes: 31 additions & 6 deletions newrelic-agent/src/test/java/com/newrelic/agent/jmx/LinkingMetadataRegistrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.newrelic.agent.jmx;

import com.newrelic.api.agent.Logger;
import org.junit.Before;
import org.junit.Test;

import javax.management.MBeanServer;
Expand All @@ -16,11 +17,19 @@

public class LinkingMetadataRegistrationTest {

ObjectName name;
Logger logger;
MBeanServer fakeServer;

@Before
public void setup() throws Exception {
name = new ObjectName(MBEAN_NAME);
logger = mock(Logger.class);
fakeServer = mock(MBeanServer.class);
}

@Test
public void testRegisterHappyPath() throws Exception {
ObjectName name = new ObjectName(MBEAN_NAME);
Logger logger = mock(Logger.class);
final MBeanServer fakeServer = mock(MBeanServer.class);

LinkingMetadataRegistration testClass = new LinkingMetadataRegistration(logger) {
@Override
Expand All @@ -35,11 +44,27 @@ protected MBeanServer getMbeanServer() {

@Test
public void testExceptionsAreHandled() throws Exception {
ObjectName name = new ObjectName(MBEAN_NAME);
RuntimeException exception = new RuntimeException("Bad beans exception");

Logger logger = mock(Logger.class);
final MBeanServer fakeServer = mock(MBeanServer.class);
when(fakeServer.registerMBean(isA(LinkingMetadataMBean.class), eq(name))).thenThrow(exception);

LinkingMetadataRegistration testClass = new LinkingMetadataRegistration(logger) {
@Override
protected MBeanServer getMbeanServer() {
return fakeServer;
}
};

testClass.registerLinkingMetadata();
//success
verify(logger).log(eq(Level.INFO), eq("JMX LinkingMetadata started, registering MBean: com.newrelic.jfr:type=LinkingMetadata"));
verify(logger).log(eq(Level.INFO), eq("Error registering JMX LinkingMetadata MBean"), eq(exception));
}

@Test
public void testNoClassDefErrorAlsoHandled() throws Exception {
NoClassDefFoundError exception = new NoClassDefFoundError("I am broken.");

when(fakeServer.registerMBean(isA(LinkingMetadataMBean.class), eq(name))).thenThrow(exception);

LinkingMetadataRegistration testClass = new LinkingMetadataRegistration(logger) {
Expand Down