From 8a73d1f693ab547a91d5a9e0eef817ae816002a5 Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Mon, 5 Feb 2024 21:58:26 +0000 Subject: [PATCH 1/3] fix(tracepoint logging): change tracepoint logger to use plugin system - change default logger to be a plugin - add plugin to default plugin list --- .../agent/api/logger/TracepointLogger.java | 3 ++- .../deep/agent/settings/Settings.java | 22 +++++-------------- ...m.intergral.deep.agent.api.spi.IDeepPlugin | 1 + .../deep/agent/settings/SettingsTest.java | 18 ++++++--------- 4 files changed, 16 insertions(+), 28 deletions(-) diff --git a/agent-api/src/main/java/com/intergral/deep/agent/api/logger/TracepointLogger.java b/agent-api/src/main/java/com/intergral/deep/agent/api/logger/TracepointLogger.java index bc86051..99fedde 100644 --- a/agent-api/src/main/java/com/intergral/deep/agent/api/logger/TracepointLogger.java +++ b/agent-api/src/main/java/com/intergral/deep/agent/api/logger/TracepointLogger.java @@ -17,13 +17,14 @@ package com.intergral.deep.agent.api.logger; +import com.intergral.deep.agent.api.spi.IDeepPlugin; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * This is the default tracepoint logger that will log to the default Deep logger. */ -public class TracepointLogger implements ITracepointLogger { +public class TracepointLogger implements ITracepointLogger, IDeepPlugin { private static final Logger LOGGER = LoggerFactory.getLogger(TracepointLogger.class); diff --git a/agent/src/main/java/com/intergral/deep/agent/settings/Settings.java b/agent/src/main/java/com/intergral/deep/agent/settings/Settings.java index 5f0f3b0..98210e7 100644 --- a/agent/src/main/java/com/intergral/deep/agent/settings/Settings.java +++ b/agent/src/main/java/com/intergral/deep/agent/settings/Settings.java @@ -19,10 +19,10 @@ import com.intergral.deep.agent.api.IRegistration; import com.intergral.deep.agent.api.logger.ITracepointLogger; -import com.intergral.deep.agent.api.logger.TracepointLogger; import com.intergral.deep.agent.api.resource.Resource; import com.intergral.deep.agent.api.settings.ISettings; import com.intergral.deep.agent.api.spi.IDeepPlugin; +import com.intergral.deep.agent.api.spi.Ordered; import com.intergral.deep.agent.types.TracePointConfig.EStage; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -34,6 +34,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -53,8 +54,7 @@ public class Settings implements ISettings { private static final AtomicBoolean IS_ACTIVE = new AtomicBoolean(true); private final Properties properties; private Resource resource; - private final Collection plugins = new ArrayList<>(); - private ITracepointLogger tracepointLogger = new TracepointLogger(); + private final List plugins = new ArrayList<>(); private Settings(Properties properties) { this.properties = properties; @@ -400,7 +400,7 @@ public void setActive(boolean state) { * @param snapshotId the snapshot id */ public void logTracepoint(final String logMsg, final String tracepointId, final String snapshotId) { - this.tracepointLogger.logTracepoint(logMsg, tracepointId, snapshotId); + getTracepointLogger().logTracepoint(logMsg, tracepointId, snapshotId); } /** @@ -409,20 +409,9 @@ public void logTracepoint(final String logMsg, final String tracepointId, final * @return the tracepoint logger */ public ITracepointLogger getTracepointLogger() { - return tracepointLogger; + return getPlugin(ITracepointLogger.class); } - /** - * Set the tracepoint logger to a new logger. - * - * @param tracepointLogger the new logger - */ - public void setTracepointLogger(final ITracepointLogger tracepointLogger) { - if (tracepointLogger == null) { - return; - } - this.tracepointLogger = tracepointLogger; - } /** * Add a plugin to the current config. @@ -432,6 +421,7 @@ public void setTracepointLogger(final ITracepointLogger tracepointLogger) { */ public IRegistration addPlugin(final IDeepPlugin plugin) { this.plugins.add(plugin); + this.plugins.sort(Comparator.comparing(Ordered::order)); return new IRegistration() { @Override public void unregister() { diff --git a/agent/src/main/resources/META-INF/services/com.intergral.deep.agent.api.spi.IDeepPlugin b/agent/src/main/resources/META-INF/services/com.intergral.deep.agent.api.spi.IDeepPlugin index fd5d00b..31fb7e4 100644 --- a/agent/src/main/resources/META-INF/services/com.intergral.deep.agent.api.spi.IDeepPlugin +++ b/agent/src/main/resources/META-INF/services/com.intergral.deep.agent.api.spi.IDeepPlugin @@ -22,3 +22,4 @@ com.intergral.deep.plugin.JavaPlugin com.intergral.deep.agent.api.auth.BasicAuthProvider com.intergral.deep.plugin.PrometheusMetricsPlugin com.intergral.deep.plugin.OtelPlugin +com.intergral.deep.agent.api.logger.TracepointLogger diff --git a/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java b/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java index 35a851d..f5e1d83 100644 --- a/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java +++ b/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java @@ -130,23 +130,19 @@ void setActive() { } @Test - void tracepointLogger_not_null() { + void tracepointLogger_null() { final Settings settings = Settings.build(new HashMap<>()); - assertNotNull(settings.getTracepointLogger()); - } - - @Test - void tracepointLogger_set_null() { - final Settings settings = Settings.build(new HashMap<>()); - settings.setTracepointLogger(null); - assertNotNull(settings.getTracepointLogger()); + assertNull(settings.getTracepointLogger()); } @Test void tracepointLogger_can_log() { final Settings settings = Settings.build(new HashMap<>()); - final ITracepointLogger tracepointLogger = Mockito.mock(ITracepointLogger.class); - settings.setTracepointLogger(tracepointLogger); + abstract class TPLogger implements IDeepPlugin, ITracepointLogger { + + } + final TPLogger tracepointLogger = Mockito.mock(TPLogger.class); + settings.setPlugins(Collections.singletonList(tracepointLogger)); assertNotNull(settings.getTracepointLogger()); settings.logTracepoint("log", "tp_id", "snap_id"); From 770e76e54f6976e53ca58c03e46b8e15998321e4 Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Mon, 5 Feb 2024 22:02:32 +0000 Subject: [PATCH 2/3] fix(lint): fix lint issue --- .../java/com/intergral/deep/agent/settings/SettingsTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java b/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java index f5e1d83..79fbac8 100644 --- a/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java +++ b/agent/src/test/java/com/intergral/deep/agent/settings/SettingsTest.java @@ -141,6 +141,7 @@ void tracepointLogger_can_log() { abstract class TPLogger implements IDeepPlugin, ITracepointLogger { } + final TPLogger tracepointLogger = Mockito.mock(TPLogger.class); settings.setPlugins(Collections.singletonList(tracepointLogger)); assertNotNull(settings.getTracepointLogger()); From 1c041ba957e32a7ee098bafa94d7629d06fceca3 Mon Sep 17 00:00:00 2001 From: Ben Donnelly Date: Tue, 6 Feb 2024 14:54:54 +0000 Subject: [PATCH 3/3] chore(build): update CHANGELOG.md --- CHANGELOG.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1edb433..84f2e31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,11 @@ # main - Unreleased - - **[FEATURE]**: plugin: Add plugin for Otel [#87](https://github.com/intergral/deep/pull/87) [@Umaaz](https://github.com/Umaaz) - - **[FEATURE]**: traces: Add apis for creating traces from tracepoints [#87](https://github.com/intergral/deep/pull/87) [@Umaaz](https://github.com/Umaaz) - - **[ENHANCEMENT]**: make checkstyle use regex for license check [#94](https://github.com/intergral/deep/pull/94) [@Umaaz](https://github.com/Umaaz) - - **[BUGFIX]**: fix issue with SPI loading failing [#92](https://github.com/intergral/deep/pull/92) [@Umaaz](https://github.com/Umaaz) - - **[BUGFIX]**: fix issue with method entry tracepoints [#91](https://github.com/intergral/deep/pull/91) [@Umaaz](https://github.com/Umaaz) +- **[CHANGE]**: change log config to allow better control of logging [#103](https://github.com/intergral/deep/pull/103) [@Umaaz](https://github.com/Umaaz) +- **[CHANGE]**: change tracepoint logger to be a plugin [#106](https://github.com/intergral/deep/pull/106) [@Umaaz](https://github.com/Umaaz) +- **[FEATURE]**: plugin: Add plugin for Otel [#87](https://github.com/intergral/deep/pull/87) [@Umaaz](https://github.com/Umaaz) +- **[FEATURE]**: traces: Add apis for creating traces from tracepoints [#87](https://github.com/intergral/deep/pull/87) [@Umaaz](https://github.com/Umaaz) +- **[ENHANCEMENT]**: make checkstyle use regex for license check [#94](https://github.com/intergral/deep/pull/94) [@Umaaz](https://github.com/Umaaz) +- **[BUGFIX]**: fix issue with SPI loading failing [#92](https://github.com/intergral/deep/pull/92) [@Umaaz](https://github.com/Umaaz) +- **[BUGFIX]**: fix issue with method entry tracepoints [#91](https://github.com/intergral/deep/pull/91) [@Umaaz](https://github.com/Umaaz) # 1.1.4 (15/12/2023) - **[CHANGE]**: plugin: Add new API for registering plugins [#84](https://github.com/intergral/deep/pull/84) [@Umaaz](https://github.com/Umaaz)