diff --git a/CHANGELOG.md b/CHANGELOG.md index ac27c4a..180c8c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Changed + +- The vendored trace client is now 0.4.0, and every usage event now carries the plugin version, `command` events included; before, only `startup` did. + ### Added - An automated test suite, run by `mvn test` and by the existing `mvn package` step in CI, so a failing test now fails the build. The first tests lock down the `plugin.yml` contract: every permission node checked in code is registered or on an explicit knowingly-unregistered list (`rp.admin`, `rp.default`, `rp.card.show.others`, `rp.rphelp`), every registered node is checked somewhere, every registered command is dispatched by `CommandService` and vice versa, `rp.card.*` parents exactly the nine player card nodes and not `rp.card.forcesave` or `rp.card.forceload`, and the `USER_GUIDE.md` permission table matches what is registered. The `rp.card.*` guarantee is exercised through Bukkit's own `PermissibleBase` against a stub server, which is the check PR #335 could only establish with a throwaway class. This is the check that would have caught #321, #322 and #329 when they were introduced. `USER_GUIDE.md` gained the missing `rp.rphelp` entry under "Known Permission Discrepancies", which the new suite flagged on its first run. diff --git a/CONFIG.md b/CONFIG.md index 9cfcc96..7a46966 100644 --- a/CONFIG.md +++ b/CONFIG.md @@ -20,7 +20,7 @@ usage-reporting: key: "2LADE-cE7kH1bYHZ0OUQrzEZOrdhpKBg1GKdw3f1FJ0" ``` -**Description:** When the plugin is enabled, and each time one of its commands is used, a small event is sent to the author's [trace](https://github.com/Stephenson-Software/trace-client-java) server so it is known which plugins are actually in use. An event carries the plugin's name, the event name (`startup` or `command`), and either the plugin version or the command name — nothing about players, the world, or the server. Sending happens off the main thread, never delays a tick, and is dropped silently if the server cannot be reached. +**Description:** When the plugin is enabled, and each time one of its commands is used, a small event is sent to the author's [trace](https://github.com/Stephenson-Software/trace-client-java) server so it is known which plugins are actually in use. An event carries the plugin's name, the event name (`startup` or `command`), the plugin version, and for a command the command name — nothing about players, the world, or the server. Sending happens off the main thread, never delays a tick, and is dropped silently if the server cannot be reached. | Option | Default | Description | |--------|---------|-------------| diff --git a/src/main/java/dansplugins/rpsystem/MedievalRoleplayEngine.java b/src/main/java/dansplugins/rpsystem/MedievalRoleplayEngine.java index 1263835..fd977e7 100644 --- a/src/main/java/dansplugins/rpsystem/MedievalRoleplayEngine.java +++ b/src/main/java/dansplugins/rpsystem/MedievalRoleplayEngine.java @@ -78,14 +78,14 @@ public void onEnable() { } // usage reporting: one event now, one per command; see config.yml - trace = TraceClient.builder(configService.getUsageReportingEndpoint(), getName()) + trace = TraceClient.builder(configService.getUsageReportingEndpoint(), getName(), getDescription().getVersion()) .key(configService.getUsageReportingKey()) .enabled(configService.isUsageReportingEnabled()) .serverWideConfig(getDataFolder().getParentFile()) .logger(getLogger()) .build(); logUsageReportingState(); - trace.report("startup", null, Collections.singletonMap("version", getDescription().getVersion())); + trace.report("startup"); } // Said on every startup so an operator can see reporting is on, and why it is off, from diff --git a/src/main/java/dansplugins/rpsystem/trace/TraceClient.java b/src/main/java/dansplugins/rpsystem/trace/TraceClient.java index ad9fec6..476caa4 100644 --- a/src/main/java/dansplugins/rpsystem/trace/TraceClient.java +++ b/src/main/java/dansplugins/rpsystem/trace/TraceClient.java @@ -1,5 +1,5 @@ /* - * trace-client 0.3.0 -- https://github.com/Stephenson-Software/trace-client-java + * trace-client 0.4.0 -- https://github.com/Stephenson-Software/trace-client-java * * One call to report that a program was used. Copy this file into a project as * is, or depend on the artifact; either way there is nothing else to add. @@ -72,12 +72,18 @@ * event's own tag wins over a server-wide one of the same name. See * {@link Builder#serverWideConfig(File)}. * + *

Every event carries the program's own version as the tag + * {@code version} -- the third argument to {@link #builder}, required, so a + * {@code command} event can be tied to a release as well as a + * {@code startup} one. An event's own {@code version} tag wins over it. + * *

A disabled client is a no-op that costs nothing. Programs that run on * other people's machines should expose their own switch in their * configuration and say on startup whether reporting is on. * *

{@code
- * TraceClient trace = TraceClient.builder("https://trace.example.org", "MyPlugin")
+ * TraceClient trace = TraceClient.builder("https://trace.example.org", "MyPlugin",
+ *                 getDescription().getVersion())
  *         .key(config.getString("usage-reporting.key"))
  *         .enabled(config.getBoolean("usage-reporting.enabled", true))
  *         .serverWideConfig(getDataFolder().getParentFile()) // plugins/
@@ -100,7 +106,7 @@
 public final class TraceClient {
 
     /** This client's version, as sent in the User-Agent. */
-    public static final String VERSION = "0.3.0";
+    public static final String VERSION = "0.4.0";
 
     /** How many reports may wait to be sent before new ones are dropped. */
     public static final int QUEUE_CAPACITY = 256;
@@ -158,6 +164,7 @@ public final class TraceClient {
     private final String endpoint;
     private final String key;
     private final String application;
+    private final String version;
     private final Logger logger;
     private final String disabledReason; // null when enabled
     private final Map serverWideTags; // never null; read once, at build()
@@ -167,6 +174,7 @@ private TraceClient(Builder builder) {
         this.endpoint = builder.baseUrl.replaceAll("/+$", "") + "/api/metrics";
         this.key = builder.key;
         this.application = builder.application;
+        this.version = builder.version;
         this.logger = builder.logger;
         ServerWideConfig serverWide = builder.pluginsDirectory == null || environmentDisables()
                 ? ServerWideConfig.NONE
@@ -191,15 +199,18 @@ private TraceClient(Builder builder) {
 
     /**
      * Starts describing a client for the program named {@code application},
-     * reporting to the trace server at {@code baseUrl}.
+     * at {@code version}, reporting to the trace server at {@code baseUrl}.
+     * The version is sent as the tag {@code version} on every event; a blank
+     * one, or one longer than {@value #MAX_TAG_LENGTH} characters, is an
+     * {@link IllegalArgumentException}.
      */
-    public static Builder builder(String baseUrl, String application) {
-        return new Builder(baseUrl, application);
+    public static Builder builder(String baseUrl, String application, String version) {
+        return new Builder(baseUrl, application, version);
     }
 
     /** A client that reports nothing. Useful as a default before configuration is read. */
     public static TraceClient disabled() {
-        return new Builder("http://disabled.invalid", "disabled").enabled(false).build();
+        return new Builder("http://disabled.invalid", "disabled", "disabled").enabled(false).build();
     }
 
     /** Whether {@link #report} will actually send anything. */
@@ -460,6 +471,25 @@ static Map withServerWideTags(Map tags, Map withVersion(Map tags, String version) {
+        Map merged = new LinkedHashMap<>();
+        if (tags != null) {
+            for (Map.Entry tag : new LinkedHashMap<>(tags).entrySet()) {
+                if (tag.getKey() != null && tag.getValue() != null) {
+                    merged.put(tag.getKey(), tag.getValue());
+                }
+            }
+        }
+        if (!merged.containsKey("version")) {
+            merged.put("version", version);
+        }
+        return merged;
+    }
+
     /** Reports that {@code name} happened. */
     public void report(String name) {
         report(name, null, null);
@@ -473,7 +503,8 @@ public void report(String name, Double value, Map tags) {
         if (executor == null || name == null || name.trim().isEmpty()) {
             return;
         }
-        final String body = json(application, name, value, withServerWideTags(tags, serverWideTags));
+        final String body = json(application, name, value,
+                withServerWideTags(withVersion(tags, version), serverWideTags));
         executor.execute(() -> send(body));
     }
 
@@ -606,20 +637,28 @@ static String quote(String text) {
     public static final class Builder {
         private final String baseUrl;
         private final String application;
+        private final String version;
         private String key;
         private boolean enabled = true;
         private File pluginsDirectory;
         private Logger logger;
 
-        private Builder(String baseUrl, String application) {
+        private Builder(String baseUrl, String application, String version) {
             if (baseUrl == null || baseUrl.trim().isEmpty()) {
                 throw new IllegalArgumentException("baseUrl is required");
             }
             if (application == null || application.trim().isEmpty()) {
                 throw new IllegalArgumentException("application is required");
             }
+            if (version == null || version.trim().isEmpty()) {
+                throw new IllegalArgumentException("version is required");
+            }
+            if (version.trim().length() > MAX_TAG_LENGTH) {
+                throw new IllegalArgumentException("version is longer than " + MAX_TAG_LENGTH + " characters");
+            }
             this.baseUrl = baseUrl.trim();
             this.application = application.trim();
+            this.version = version.trim();
         }
 
         /** The program's write key. Without one the client is a no-op. */